Messages in this thread | | | Date | Mon, 12 Feb 2024 10:30:17 -0800 | From | Kees Cook <> | Subject | Re: [PATCH] RDMA/uverbs: Remove flexible arrays from struct *_filter |
| |
On Sun, Feb 11, 2024 at 12:58:56PM +0100, Erick Archer wrote: > When a struct containing a flexible array is included in another struct, > and there is a member after the struct-with-flex-array, there is a > possibility of memory overlap. These cases must be audited [1]. See: > > struct inner { > ... > int flex[]; > }; > > struct outer { > ... > struct inner header; > int overlap; > ... > }; > > This is the scenario for all the "struct *_filter" structures that are > included in the following "struct ib_flow_spec_*" structures: > > struct ib_flow_spec_eth > struct ib_flow_spec_ib > struct ib_flow_spec_ipv4 > struct ib_flow_spec_ipv6 > struct ib_flow_spec_tcp_udp > struct ib_flow_spec_tunnel > struct ib_flow_spec_esp > struct ib_flow_spec_gre > struct ib_flow_spec_mpls > > The pattern is like the one shown below: > > struct *_filter { > ... > u8 real_sz[]; > }; > > struct ib_flow_spec_mpls { > ... > struct *_filter val; > struct *_filter mask; > }; > > In this case, the trailing flexible array "real_sz" is never allocated > and is only used to calculate the size of the structures. Here the use > of the "offsetof" helper can be changed by the "sizeof" operator because > the goal is to get the size of these structures. Therefore, the trailing > flexible arrays can also be removed. > > Link: https://github.com/KSPP/linux/issues/202 [1] > Signed-off-by: Erick Archer <erick.archer@gmx.com> > --- > Hi everyone, > > This patch has not been tested. This has only been built-tested.
I might suggest doing a binary difference comparison[1], as it's possible that "real_sz" is being used to try to avoid trailing padding on structs. I wasn't able to trivially construct an example, so maybe I'm not understanding its purpose correctly.
If, however, there are cases where offsetof(..., real_sz) != sizeof(...), then I would check two alternatives:
struct { } real_sz;
but that may induce padding still, or:
u8 real_sz[0];
which would be a literally zero-sized array, used only for addressing.
Or, these can be left as-is, and the "flex array not at end of struct" warnings can be disabled for these targets.
-Kees
-- Kees Cook
| |