Re: [PATCH iwl-next] ixgbe: fix integer overflow and wrong bit position in ixgbe_validate_rtr()

David Laight
2026-04-03T10:00:38Z

On Fri, 27 Mar 2026 08:30:44 +0100
Aleksandr Loktionov <aleksandr.loktionov@intel.com> wrote:

> Two bugs in the same loop in ixgbe_validate_rtr():
>
> 1. When extracting 3-bit traffic class values from the IXGBE_RTRUP2TC
> register the shifted value was assigned directly to a u8, silently
> truncating any bits above bit 7. 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 the entry
> at bit position 3 was ever cleared; entries at bit positions 0, 6, 9,
> ..., 21 were left unreset. Use i * IXGBE_RTRUP2TC_UP_SHIFT to target
> the correct field for each iteration.
>
> Also replace the hardcoded 0x7 literal with the IXGBE_RTRUP2TC_UP_MASK
> constant for consistency with other parts of the driver.
>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> index 9aec66c..53b82a5 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
> @@ -9798,11 +9798,12 @@ static void ixgbe_validate_rtr(struct ixgbe_adapter *adapter, u8 tc)
> rsave = reg;
>
> for (i = 0; i < MAX_TRAFFIC_CLASS; i++) {
> - u8 up2tc = reg >> (i * IXGBE_RTRUP2TC_UP_SHIFT);
> + u8 up2tc = IXGBE_RTRUP2TC_UP_MASK &
> + (reg >> (i * IXGBE_RTRUP2TC_UP_SHIFT));

That really ought to be the opposite way round: expr & constant.
Or add a second line to mask the value.

David

>
> /* If up2tc is out of bounds default to zero */
> if (up2tc > tc)
> - reg &= ~(0x7 << IXGBE_RTRUP2TC_UP_SHIFT);
> + reg &= ~(IXGBE_RTRUP2TC_UP_MASK << (i * IXGBE_RTRUP2TC_UP_SHIFT));
> }
>
> if (reg != rsave)