Messages in this thread | | | From | "Romanowski, Rafal" <> | Subject | RE: [Intel-wired-lan] [PATCH net-next 1/3] virtchnl: fix fake 1-elem arrays in structs allocated as `nents + 1` - 1 | Date | Wed, 16 Aug 2023 12:48:37 +0000 |
| |
> -----Original Message----- > From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of > Alexander Lobakin > Sent: piątek, 4 sierpnia 2023 19:34 > To: Kees Cook <keescook@chromium.org> > Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>; Zaremba, > Larysa <larysa.zaremba@intel.com>; netdev@vger.kernel.org; Gustavo A. R. > Silva <gustavoars@kernel.org>; linux-kernel@vger.kernel.org; Eric Dumazet > <edumazet@google.com>; intel-wired-lan@lists.osuosl.org; linux- > hardening@vger.kernel.org; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni > <pabeni@redhat.com>; David S. Miller <davem@davemloft.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@chromium.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@chromium.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@intel.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@chromium.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@osuosl.org > https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
| |