Messages in this thread | | | Date | Wed, 1 Jun 2022 08:26:02 -0700 | Subject | Re: [PATCH net] ice: fix access-beyond-end in the switch code | From | Tony Nguyen <> |
| |
On 6/1/2022 3:59 AM, Alexander Lobakin wrote: > Global `-Warray-bounds` enablement revealed some problems, one of > which is the way we define and use AQC rules messages. > In fact, they have a shared header, followed by the actual message, > which can be of one of several different formats. So it is > straightforward enough to define that header as a separate struct > and then embed it into message structures as needed, but currently > all the formats reside in one union coupled with the header. Then, > the code allocates only the memory needed for a particular message > format, leaving the union potentially incomplete. > There are no actual reads or writes beyond the end of an allocated > chunk, but at the same time, the whole implementation is fragile and > backed by an equilibrium rather than strong type and memory checks. > > Define the structures the other way around: one for the common > header and the rest for the actual formats with the header embedded. > There are no places where several union members would be used at the > same time anyway. This allows to use proper struct_size() and let > the compiler know what is going to be done. > Finally, unsilence `-Warray-bounds` back for ice_switch.c. > > Other little things worth mentioning: > * &ice_sw_rule_vsi_list_query is not used anywhere, remove it. It's > weird anyway to talk to hardware with purely kernel types > (bitmaps); > * expand the ICE_SW_RULE_*_SIZE() macros to pass a structure > variable name to struct_size() to let it do strict typechecking; > * rename ice_sw_rule_lkup_rx_tx::hdr to ::hdr_data to keep ::hdr > for the header structure to have the same name for it constistenly > everywhere; > * drop the duplicate of %ICE_SW_RULE_RX_TX_NO_HDR_SIZE residing in > ice_switch.h. > > Fixes: 9daf8208dd4d ("ice: Add support for switch filter programming") > Fixes: 66486d8943ba ("ice: replace single-element array used for C struct hack") > Signed-off-by: Alexander Lobakin <alexandr.lobakin@intel.com> > Reviewed-by: Marcin Szycik <marcin.szycik@linux.intel.com> > --- > To Tony: I'd like this to hit RC1 or RC2, so would it be okay to pass > through -net directly? Or via some quick pull request would work too > I guess :)
LGTM. I'm okay with it going to net directly.
Acked-by: Tony Nguyen <anthony.l.nguyen@intel.com>
| |