On Wed, Mar 25, 2026 at 12:12:07PM +0000, Loftus, Ciara wrote:
> > Subject: Re: [PATCH] net/i40e: fix raw flow item validation
> >
> > On Tue, Mar 24, 2026 at 03:56:27PM +0000, Ciara Loftus wrote:
> > > Commit c138a6c3bd0a ("net/i40e: validate raw flow items before
> > > dereferencing") rejected raw flow items when spec and mask length
> > > fields differ, intending to prevent out-of-bounds pattern access.
> > >
> > > This breaks flows using the default rte_flow_item_raw_mask, which
> > > sets mask.length to 0xffff regardless of the spec pattern size. Remove
> > > the erroneous check.
> > >
> > > Fixes: c138a6c3bd0a ("net/i40e: validate raw flow items before
> > dereferencing")
> > > Cc: [email protected]
> > >
> > > Signed-off-by: Ciara Loftus <[email protected]>
> > > ---
> > > drivers/net/intel/i40e/i40e_flow.c | 7 -------
> > > 1 file changed, 7 deletions(-)
> > >
> > > diff --git a/drivers/net/intel/i40e/i40e_flow.c
> > b/drivers/net/intel/i40e/i40e_flow.c
> > > index 84cfddb92d..78191a2d22 100644
> > > --- a/drivers/net/intel/i40e/i40e_flow.c
> > > +++ b/drivers/net/intel/i40e/i40e_flow.c
> > > @@ -2360,13 +2360,6 @@ i40e_flow_parse_fdir_pattern(struct
> > rte_eth_dev *dev,
> > > "NULL RAW mask
> > pattern");
> > > return -rte_errno;
> > > }
> > > - if (raw_spec->length != raw_mask->length) {
> > > - rte_flow_error_set(error, EINVAL,
> > > -
> > RTE_FLOW_ERROR_TYPE_ITEM,
> > > - item,
> > > - "RAW spec and
> > mask length mismatch");
> > > - return -rte_errno;
> > > - }
> > > }
> >
> > This looks a correct check to have. Maybe we need to check how the default
> > mask is generated for this case and fix things there?
>
> It is unclear to me, whether the length field of the mask should hold
> a bytecount or a bitmask. For example in testpmd if a raw flow is
> created without an explicit mask, the length appears to hold a bitmask
> all set to 1s.
> If a raw flow is created with an explicit mask, the length appears to
> hold the bytecount of the mask.
> Modifying that behaviour so that it is consistent appears to be
> non-trivial, I would be reluctant to change it this late in the release.
> If we wish to continue to permit the ambiguity in the driver for now,
> this patch is needed. Otherwise, we are accepting that the field should
> always hold a bytecount.
>
Ok, so safest option for this release is to revert this part of the code.
Will take the original patch.
/Bruce
