Netdev List
 help / color / mirror / Atom feed
From: Michael Bommarito <michael.bommarito@gmail.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Tonghao Zhang <xiangxia.m.yue@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	stable@vger.kernel.org
Subject: [PATCH net] net: ifb: clamp ethtool stats loops to num_tx_queues
Date: Mon, 11 May 2026 08:28:35 -0400	[thread overview]
Message-ID: <20260511122835.441911-1-michael.bommarito@gmail.com> (raw)

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

                 reply	other threads:[~2026-05-11 12:28 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260511122835.441911-1-michael.bommarito@gmail.com \
    --to=michael.bommarito@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=xiangxia.m.yue@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox