Re: [PATCH iwl-net 3/3] e1000e: fix NETIF_F_RXALL buffer overrun

[Matt Vollrath <tactii@gmail.com>]

On 9/1/26 23:29, Matt Vollrath wrote:
> When SBP is set, the card may deliver frames which would otherwise be
> filtered out by LPE being unset. This would allow the device to write
> up to 526 bytes beyond the skb's data allocation: over its own shinfo,
> and beyond. This bug is reachable only when MTU <= 1500 and
> NETIF_F_RXALL is set ("ethtool -K <dev> rx-all on").
> 
> Ensure that buffers are large enough for an entire 2048 byte chunk when
> NETIF_F_RXALL is set. Do this by moving final rx_buffer_len
> determination to one place, right before RCTL.BSIZE is determined. This
> will correctly re-evaluate every time the adapter is configured, not
> just on MTU change.
> 
> Signed-off-by: Matt Vollrath <tactii@gmail.com>
> Suggested-by: Jakub Kicinski <kuba@kernel.org>
> Assisted-by: Claude:claude-5-fable
> Fixes: cf955e6c96cb ("e1000e: Support RXALL feature flag.")
> Cc: stable@vger.kernel.org
> ---
>   drivers/net/ethernet/intel/e1000e/netdev.c | 53 +++++++++++++---------
>   1 file changed, 32 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 063fc8cd2673..80d5a0010df8 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -3036,6 +3036,33 @@ static void e1000_configure_tx(struct e1000_adapter *adapter)
>   #define PAGE_USE_COUNT(S) (((S) >> PAGE_SHIFT) + \
>   			   (((S) & (PAGE_SIZE - 1)) ? 1 : 0))
>   
> +/**
> + * e1000_set_rx_buffer_len - determine the Rx buffer size
> + * @adapter: Board private structure
> + **/
> +static void e1000_set_rx_buffer_len(struct e1000_adapter *adapter)
> +{
> +	struct net_device *netdev = adapter->netdev;
> +	u32 max_frame = adapter->max_frame_size;
> +
> +	/* NOTE: netdev_alloc_skb reserves 16 bytes, and typically NET_IP_ALIGN
> +	 * means we reserve 2 more, this pushes us to allocate from the next
> +	 * larger slab size.
> +	 * i.e. RXBUFFER_2048 --> size-4096 slab
> +	 * However with the new *_jumbo_rx* routines, jumbo receives will use
> +	 * fragmented skbs
> +	 */
> +	if (max_frame <= 2048)
> +		adapter->rx_buffer_len = 2048;
> +	else
> +		adapter->rx_buffer_len = 4096;
> +
> +	/* adjust allocation if LPE protects us, and we aren't using SBP */
> +	if (max_frame <= (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN) &&
> +	    !(netdev->features & NETIF_F_RXALL))
> +		adapter->rx_buffer_len = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN;
> +}
> +
>   /**
>    * e1000_setup_rctl - configure the receive control registers
>    * @adapter: Board private structure
> @@ -3102,6 +3129,8 @@ static void e1000_setup_rctl(struct e1000_adapter *adapter)
>   		e1e_wphy(hw, 22, phy_data);
>   	}
>   
> +	e1000_set_rx_buffer_len(adapter);
> +
>   	/* Setup buffer sizes */
>   	rctl &= ~E1000_RCTL_SZ_4096;
>   	rctl |= E1000_RCTL_BSEX;
> @@ -6087,30 +6116,12 @@ static int e1000_change_mtu(struct net_device *netdev, int new_mtu)
>   
>   	pm_runtime_get_sync(netdev->dev.parent);
>   
> -	if (netif_running(netdev))
> +	if (netif_running(netdev)) {

I see now that I should not have collapsed this to one netif_running check.

>   		e1000e_down(adapter, true);
> -
> -	/* NOTE: netdev_alloc_skb reserves 16 bytes, and typically NET_IP_ALIGN
> -	 * means we reserve 2 more, this pushes us to allocate from the next
> -	 * larger slab size.
> -	 * i.e. RXBUFFER_2048 --> size-4096 slab
> -	 * However with the new *_jumbo_rx* routines, jumbo receives will use
> -	 * fragmented skbs
> -	 */
> -
> -	if (max_frame <= 2048)
> -		adapter->rx_buffer_len = 2048;
> -	else
> -		adapter->rx_buffer_len = 4096;
> -
> -	/* adjust allocation if LPE protects us, and we aren't using SBP */
> -	if (max_frame <= (VLAN_ETH_FRAME_LEN + ETH_FCS_LEN))
> -		adapter->rx_buffer_len = VLAN_ETH_FRAME_LEN + ETH_FCS_LEN;
> -
> -	if (netif_running(netdev))
>   		e1000e_up(adapter);
> -	else
> +	} else {
>   		e1000e_reset(adapter);
> +	}
>   
>   	pm_runtime_put_sync(netdev->dev.parent);
>