> -----Original Message----- > From: Intel-wired-lan <[email protected]> On Behalf Of Simon > Horman > Sent: Wednesday, April 15, 2026 3:47 PM > To: Loktionov, Aleksandr <[email protected]> > Cc: [email protected]; Nguyen, Anthony L > <[email protected]>; [email protected]; Avinash Dayanand > <[email protected]> > Subject: Re: [Intel-wired-lan] [PATCH iwl-net 4/5] iavf: fix TC boundary > check in > iavf_handle_tclass > > On Mon, Apr 13, 2026 at 09:30:34AM +0200, Aleksandr Loktionov wrote: > > From: Avinash Dayanand <[email protected]> > > > > The condition `tc < adapter->num_tc` admits any tc value equal to or > > greater than num_tc, bypassing the destination-port validation and > > allowing traffic to be steered to a non-existent traffic class. Change > > the comparison to `tc > adapter->num_tc` to correctly reject > > out-of-range TC values. > > > > Fixes: 0075fa0fadd0 ("i40evf: Add support to apply cloud filters") > > Signed-off-by: Avinash Dayanand <[email protected]> > > Signed-off-by: Aleksandr Loktionov <[email protected]> > > I am a bit confused by this logic. > > With this patch applied: > > 1) For tc <= adapter->num_tc, which I assume is valid TCs (other than 0, > in which case the function returns earlier), the filter destination port > is skipped. > > But the failure path for that checks logs: > "Specify destination port to redirect to traffic class other than TC0\n" > > This does not seem consistent. > > 2) For tc > adapter->num_tc, which I assume is invalid TCs, > the function will eventually assign fields of filter->f and succeed > if filter has a valid destination port. > > This doesn't seem to be in keeping with the patch description. > > 3) The above two points aside, is there an out by 1 condition in > the condition tc > adapter->num_tc. It seems to imply > that tc == adapter->num_tc is a valid tc. But I suspect that > is not hte case. > > In short, I'm wondering if the function should look something like this > (completely > untested): > > /** > * iavf_handle_tclass - Forward to a traffic class on the device > * @adapter: board private structure > * @tc: traffic class index on the device > * @filter: pointer to cloud filter structure */ static int > iavf_handle_tclass(struct > iavf_adapter *adapter, u32 tc, > struct iavf_cloud_filter *filter) { > if (tc == 0) > return 0; > > if (tc >= adapter->num_tc) { > // dev_err(...); > return -EINVAL; > } > > if (!filter->f.data.tcp_spec.dst_port) { > dev_err(&adapter->pdev->dev, > "Specify destination port to redirect to traffic > class other than TC0\n"); > return -EINVAL; > } > > /* redirect to a traffic class on the same device */ > filter->f.action = VIRTCHNL_ACTION_TC_REDIRECT; > filter->f.action_meta = tc; > > return 0; > } > > > --- > > drivers/net/ethernet/intel/iavf/iavf_main.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c > > b/drivers/net/ethernet/intel/iavf/iavf_main.c > > index ab5f5adc..5e4035b 100644 > > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c > > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c > > @@ -4062,7 +4062,7 @@ static int iavf_handle_tclass(struct > > iavf_adapter *adapter, u32 tc, { > > if (tc == 0) > > return 0; > > - if (tc < adapter->num_tc) { > > + if (tc > adapter->num_tc) { > > if (!filter->f.data.tcp_spec.dst_port) { > > dev_err(&adapter->pdev->dev, > > "Specify destination port to redirect to traffic > class other than > > TC0\n"); > > -- > > 2.52.0 > >
Tested-by: Rafal Romanowski <[email protected]>
