Re: [Intel-wired-lan] [PATCH net] ice: fix access-beyond-end in the switch code

[Tony Nguyen <anthony.l.nguyen@intel.com>]

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>


_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan