[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20240221174930.GF13491@ziepe.ca>
Date: Wed, 21 Feb 2024 13:49:30 -0400
From: Jason Gunthorpe <jgg@...pe.ca>
To: Erick Archer <erick.archer@....com>
Cc: Leon Romanovsky <leon@...nel.org>, Edward Srouji <edwards@...dia.com>,
Patrisious Haddad <phaddad@...dia.com>,
"Gustavo A. R. Silva" <gustavoars@...nel.org>,
Kees Cook <keescook@...omium.org>, linux-rdma@...r.kernel.org,
linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2] RDMA/uverbs: Remove flexible arrays from struct
*_filter
On Sat, Feb 17, 2024 at 03:29:13PM +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_* {
> ...
> 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.
>
> However, due to the trailing padding that can be induced in structs it
> is possible that the:
>
> offsetof(struct *_filter, real_sz) != sizeof(struct *_filter)
>
> This situation happens with the "struct ib_flow_ipv6_filter" and to
> avoid it the "__packed" macro is used in this structure. But now, the
> "sizeof(struct ib_flow_ipv6_filter)" has changed. This is not a problem
> since this size is not used in the code.
>
> The situation now is that "sizeof(struct ib_flow_spec_ipv6)" has also
> changed (this struct contains the struct ib_flow_ipv6_filter). This is
> also not a problem since it is only used to set the size of the "union
> ib_flow_spec", which can store all the "ib_flow_spec_*" structures.
>
> Link: https://github.com/KSPP/linux/issues/202 [1]
> Signed-off-by: Erick Archer <erick.archer@....com>
> ---
> Changes in v2:
> - Add the "__packed" macro to the "struct ib_flow_ipv6_filter".
> - Update the commit message to explain why the "__packed" macro is used.
Applied to for-next, thanks
Jason
Powered by blists - more mailing lists