Re: [PATCH iwl-net v2 6/6] ixgbe: fix integer overflow and wrong bit position in ixgbe_validate_rtr()

Simon Horman
2026-04-13T14:02:30Z

On Mon, Apr 13, 2026 at 02:43:34PM +0100, Simon Horman wrote:
> On Wed, Apr 08, 2026 at 03:11:54PM +0200, Aleksandr Loktionov wrote:
> > Two bugs in the same loop in ixgbe_validate_rtr():
> >
> > 1. The 3-bit traffic-class field was extracted by shifting a u32 and
> > assigning the result directly to a u8. For user priority 0 this is
> > harmless; for UP[5..7] the shift leaves bits [15..21] in the u32
> > which are then silently truncated when stored in u8. Mask with
> > IXGBE_RTRUP2TC_UP_MASK before the assignment so only the intended
> > 3 bits are kept.
> >
> > 2. When clearing an out-of-bounds entry the mask was always shifted by
> > the fixed constant IXGBE_RTRUP2TC_UP_SHIFT (== 3), regardless of
> > which loop iteration was being processed. This means only UP1 (bit
> > position 3) was ever cleared; UP0,2..7 (positions 0, 6, 9, ..., 21)
> > were left unreset, so invalid TC mappings persisted in hardware and
> > could mis-steer received packets to the wrong traffic class.
> > Use i * IXGBE_RTRUP2TC_UP_SHIFT to target the correct 3-bit field
> > for each iteration.
> >
> > Swap the operand order in the mask expression to place the constant
> > on the right per kernel coding style (noted by David Laight).
> >
> > Fixes: e7589eab9291 ("ixgbe: consolidate, setup for multiple traffic classes")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> > ---
> > v1 -> v2:
> > - Add Fixes: tag; reroute to iwl-net (wrong bit positions cause packet
> > mis-steering); swap to (reg >> ...) & MASK operand order per David
> > Laight.
>
> Reviewed-by: Simon Horman <horms@kernel.org>

Sorry, I was a little too hasty there.

AI generated code review points out that the cited commit
doesn't seem to have introduced the code being fixed,
and that perhaps this Fixes tag would be more appropriate.

Fixes: 8b1c0b24d9af ("ixgbe: configure minimal packet buffers to support TC")

I will also forward on a review from Sashiko, although it
is for an existing bug and thus is strictly FYI.