On Thu, Feb 9, 2023 at 8:36 AM Kees Cook keescook@chromium.org wrote:
On Mon, Feb 06, 2023 at 11:17:06AM -0800, Stanislav Fomichev wrote:
It's my understanding that it's the intended use-case. Users are expected to use this struct as a header; at least we've been using it that way :-)
For me, both return the same: sizeof(struct { __u32 prefix; __u8 data[0]; }) sizeof(struct { __u32 prefix; __u8 data[]; })
So let's do s/data[0]/data[]/ in the UAPI only? What's wrong with using this struct as a header?
For the whole struct, yup, the above sizeof()s are correct. However:
sizeof(foo->data) == 0 // when data[0] sizeof(foo->data) == compile error // when data[]
The [0]-array GNU extension doesn't have consistent behavior, so it's being removed from the kernel in favor of the proper C99 [] flexible arrays, so we can enable -fstrict-flex-arrays=3 to remove all the ambiguities with array bounds: https://docs.kernel.org/process/deprecated.html#zero-length-and-one-element-... https://people.kernel.org/kees/bounded-flexible-arrays-in-c
As a header, this kind of overlap isn't well supported. Clang already warns, and GCC is going to be removing support for overlapping composite structs with a flex array in the middle (and also warns under -pedantic): https://godbolt.org/z/vWzqs41h6
I talk about dealing with these specific cases in my recent write-up on array bounds checking -- see "Overlapping composite structure members" in the people.kernel.org post above.
Perhaps better might be:
struct bpf_lpm_trie_key { __u32 prefixlen; /* up to 32 for AF_INET, 128 for AF_INET6 */ };
struct bpf_lpm_trie_key_raw { struct bpf_lpm_trie_key_prefix prefix; u8 data[]; };
struct my_key { struct bpf_lpm_trie_key_prefix prefix; int a, b, c; };
This approach is, perhaps, the best way to go? Besides the selftest, what things in userspace consumes struct bpf_lpm_trie_key?
Plenty of bpf progs use it: https://github.com/cilium/cilium/blob/master/bpf/lib/common.h#L352