netdev - RE: [Intel-wired-lan] [PATCH net-next 1/3] virtchnl: fix fake 1-elem arrays in structs allocated as `nents + 1`

Open Source and information security mailing list archives

Message-ID: <BL0PR11MB3521F65964AA8F824211B8528F15A@BL0PR11MB3521.namprd11.prod.outlook.com>
Date: Wed, 16 Aug 2023 12:48:37 +0000
From: "Romanowski, Rafal" <rafal.romanowski@...el.com>
To: "Lobakin, Aleksander" <aleksander.lobakin@...el.com>, Kees Cook
	<keescook@...omium.org>
CC: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>, "Zaremba, Larysa"
	<larysa.zaremba@...el.com>, "netdev@...r.kernel.org"
	<netdev@...r.kernel.org>, "Gustavo A. R. Silva" <gustavoars@...nel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, Eric Dumazet
	<edumazet@...gle.com>, "intel-wired-lan@...ts.osuosl.org"
	<intel-wired-lan@...ts.osuosl.org>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, Jakub Kicinski <kuba@...nel.org>, "Paolo
 Abeni" <pabeni@...hat.com>, "David S. Miller" <davem@...emloft.net>
Subject: RE: [Intel-wired-lan] [PATCH net-next 1/3] virtchnl: fix fake 1-elem
 arrays in structs allocated as `nents + 1` - 1

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@...osl.org> On Behalf Of
> Alexander Lobakin
> Sent: piątek, 4 sierpnia 2023 19:34
> To: Kees Cook <keescook@...omium.org>
> Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>; Zaremba,
> Larysa <larysa.zaremba@...el.com>; netdev@...r.kernel.org; Gustavo A. R.
> Silva <gustavoars@...nel.org>; linux-kernel@...r.kernel.org; Eric Dumazet
> <edumazet@...gle.com>; intel-wired-lan@...ts.osuosl.org; linux-
> hardening@...r.kernel.org; Jakub Kicinski <kuba@...nel.org>; Paolo Abeni
> <pabeni@...hat.com>; David S. Miller <davem@...emloft.net>
> Subject: Re: [Intel-wired-lan] [PATCH net-next 1/3] virtchnl: fix fake 1-elem
> arrays in structs allocated as `nents + 1` - 1
> 
> From: Kees Cook <keescook@...omium.org>
> Date: Fri, 4 Aug 2023 10:29:48 -0700
> 
> > On Fri, Aug 04, 2023 at 05:42:19PM +0200, Alexander Lobakin wrote:
> >> From: Kees Cook <keescook@...omium.org>
> >> Date: Fri, 4 Aug 2023 01:27:02 -0700
> >>
> >>> On Fri, Jul 28, 2023 at 05:52:05PM +0200, Alexander Lobakin wrote:
> >>>> The two most problematic virtchnl structures are virtchnl_rss_key
> >>>> and virtchnl_rss_lut. Their "flex" arrays have the type of u8,
> >>>> thus, when allocating / checking, the actual size is calculated as
> >>>> `sizeof + nents - 1 byte`. But their sizeof() is not 1 byte larger
> >>>> than the size of such structure with proper flex array, it's two
> >>>> bytes larger due to the padding. That said, their size is always 1
> >>>> byte larger unless there are no tail elements -- then it's +2 bytes.
> >>>> Add virtchnl_struct_size() macro which will handle this case (and
> >>>> later other cases as well). Make its calling conv the same as we
> >>>> call
> >>>> struct_size() to allow it to be drop-in, even though it's unlikely
> >>>> to become possible to switch to generic API. The macro will
> >>>> calculate a proper size of a structure with a flex array at the
> >>>> end, so that it becomes transparent for the compilers, but add the
> >>>> difference from the old values, so that the real size of sorta-ABI-
> messages doesn't change.
> >>>> Use it on the allocation side in IAVF and the receiving side
> >>>> (defined as static inline in virtchnl.h) for the mentioned two structures.
> >>>>
> >>>> Signed-off-by: Alexander Lobakin <aleksander.lobakin@...el.com>
> >>>
> >>> This is a novel approach to solving the ABI issues for a 1-elem
> >>> conversion, but I have been convinced it's a workable approach here.
> >>> :) Thanks for doing this conversion!
> >>>
> >>> Reviewed-by: Kees Cook <keescook@...omium.org>
> >>>
> >>
> >> Thanks a lot!
> >> You gave Reviewed-by for patches #1 and #3, does it mean the whole
> >> series or something is wrong with the patch #2? :D
> >
> > Hm, maybe delivery was delayed? I see it on lore:
> > https://lore.kernel.org/all/202308040128.667940394B@keescook/
> >
> 
> Nevermind, my mail rules did put it in the folder other than the one where
> the main thread was, sorry :s Much thanks, I'm now a fan of _Generic() too
> :D
> 
> Olek
> _______________________________________________
> Intel-wired-lan mailing list
> Intel-wired-lan@...osl.org
> https://lists.osuosl.org/mailman/listinfo/intel-wired-lan


Tested-by: Rafal Romanowski <rafal.romanowski@...el.com>

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives