[PATCH net] net: ifb: clamp ethtool stats loops to num_tx_queues

[PATCH net] net: ifb: clamp ethtool stats loops to num_tx_queues

[Michael Bommarito]

ifb_dev_init() allocates dp->tx_private to dev->num_tx_queues entries
via kzalloc_objs(*txp, dev->num_tx_queues), but ifb_get_ethtool_stats()
walks the array up to dev->real_num_rx_queues and reads each slot's
rx_stats. When userspace creates an ifb device with asymmetric queue
counts where the rx count exceeds the tx count, e.g.

    ip link add name ifb10 numtxqueues 1 numrxqueues 8 type ifb
    ethtool -S ifb10

every iteration past dev->num_tx_queues reads (real_num_rx_queues -
num_tx_queues) * sizeof(struct ifb_q_private) bytes past the end of
the allocation. Because struct ifb_q_private is
____cacheline_aligned_in_smp (about 256 bytes on x86_64), an attacker
can sample 14 u64 values from a roughly 1.5 KB out-of-bounds window
with a 1+8 device. The sampled bytes are copied to userspace through
the ETHTOOL_GSTATS
ioctl, which sits in the privilege-exempt arm of ethtool_ioctl() so
any user with netns visibility to the ifb device can trigger it.

The TX stats loop is currently safe by construction
(netif_set_real_num_tx_queues() rejects txq > num_tx_queues), but
apply the same clamp to both loops so the contract is symmetric and
robust against future churn around real_num_tx_queues semantics.

Zero-pad the per-queue stats slots that no longer have a backing
ifb_q_private so the output buffer length still matches
ifb_get_sset_count() (which uses real_num_{rx,tx}_queues unmodified).

Reproduced under UML+KASAN at v7.1-rc2:

  BUG: KASAN: slab-out-of-bounds in ifb_fill_stats_data+0x3c/0xae
  Read of size 8 at addr 0000000062dbd228 by task ethtool/36
  ifb_fill_stats_data+0x3c/0xae
  ifb_get_ethtool_stats+0xc0/0x129
  __dev_ethtool+0x1ca5/0x363c
  dev_ethtool+0x123/0x1b3
  dev_ioctl+0x56c/0x744
  sock_do_ioctl+0x15f/0x1b2
  sock_ioctl+0x4d5/0x50a
  sys_ioctl+0xd8b/0xde9

  The buggy address belongs to the object at 0000000062dbd000
   which belongs to the cache kmalloc-512 of size 512
  The buggy address is located 232 bytes to the right of
   allocated 320-byte region [0000000062dbd000, 0000000062dbd140)

With the patch applied, the same UML+KASAN repro is silent and the
ethtool -S output reports zero stats for the out-of-range rx slots.

Fixes: a21ee5b2fcb8 ("net: ifb: support ethtools stats")
Cc: stable@vger.kernel.org
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
Assisted-by: Claude:claude-opus-4-7
---
 drivers/net/ifb.c | 25 +++++++++++++++++++++----
 1 file changed, 21 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ifb.c b/drivers/net/ifb.c
index 5407d2ed71b3..66323de24ba9 100644
--- a/drivers/net/ifb.c
+++ b/drivers/net/ifb.c
@@ -255,21 +255,38 @@ static void ifb_fill_stats_data(u64 **data,
 	*data += IFB_Q_STATS_LEN;
 }
 
+static void ifb_fill_empty_stats_data(u64 **data)
+{
+	memset(*data, 0, IFB_Q_STATS_LEN * sizeof(**data));
+	*data += IFB_Q_STATS_LEN;
+}
+
 static void ifb_get_ethtool_stats(struct net_device *dev,
 				  struct ethtool_stats *stats, u64 *data)
 {
 	struct ifb_dev_private *dp = netdev_priv(dev);
 	struct ifb_q_private *txp;
+	unsigned int n_queues = dev->num_tx_queues;
 	int i;
 
 	for (i = 0; i < dev->real_num_rx_queues; i++) {
-		txp = dp->tx_private + i;
-		ifb_fill_stats_data(&data, &txp->rx_stats);
+		if (i >= n_queues) {
+			ifb_fill_empty_stats_data(&data);
+			continue;
+		}
+
+		txp = dp->tx_private + i;
+		ifb_fill_stats_data(&data, &txp->rx_stats);
 	}
 
 	for (i = 0; i < dev->real_num_tx_queues; i++) {
-		txp = dp->tx_private + i;
-		ifb_fill_stats_data(&data, &txp->tx_stats);
+		if (i >= n_queues) {
+			ifb_fill_empty_stats_data(&data);
+			continue;
+		}
+
+		txp = dp->tx_private + i;
+		ifb_fill_stats_data(&data, &txp->tx_stats);
 	}
 }
 
-- 
2.53.0

Re: [PATCH net] net: ifb: clamp ethtool stats loops to num_tx_queues

[Jakub Kicinski]

On Mon, 11 May 2026 08:28:35 -0400 Michael Bommarito wrote:
>  static void ifb_get_ethtool_stats(struct net_device *dev,
>  				  struct ethtool_stats *stats, u64 *data)
>  {
>  	struct ifb_dev_private *dp = netdev_priv(dev);
>  	struct ifb_q_private *txp;
> +	unsigned int n_queues = dev->num_tx_queues;
>  	int i;
>  
>  	for (i = 0; i < dev->real_num_rx_queues; i++) {
> -		txp = dp->tx_private + i;
> -		ifb_fill_stats_data(&data, &txp->rx_stats);
> +		if (i >= n_queues) {
> +			ifb_fill_empty_stats_data(&data);
> +			continue;
> +		}
> +
> +		txp = dp->tx_private + i;
> +		ifb_fill_stats_data(&data, &txp->rx_stats);
>  	}
>  
>  	for (i = 0; i < dev->real_num_tx_queues; i++) {
> -		txp = dp->tx_private + i;
> -		ifb_fill_stats_data(&data, &txp->tx_stats);
> +		if (i >= n_queues) {
> +			ifb_fill_empty_stats_data(&data);
> +			continue;
> +		}
> +
> +		txp = dp->tx_private + i;
> +		ifb_fill_stats_data(&data, &txp->tx_stats);
>  	}

Take a look at ifb_stats64(), queues without Tx can't Rx here.
The use of real_* members is also not correct, the device
should report stats for all queues it has, not just active ones.
So please rewrite the ethtool stats code to dump rx and tx based
purely on dev->num_tx_queues
-- 
pw-bot: cr