* Re: [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-14 19:14 [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats() Kohei Enju
@ 2026-02-16 13:19 ` Simon Horman
2026-02-16 14:44 ` Kohei Enju
2026-02-18 5:39 ` Przemek Kitszel
2026-02-18 8:00 ` [Intel-wired-lan] " Paul Menzel
2 siblings, 1 reply; 9+ messages in thread
From: Simon Horman @ 2026-02-16 13:19 UTC (permalink / raw)
To: Kohei Enju
Cc: intel-wired-lan, netdev, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jedrzej Jagielski, Mateusz Palczewski,
Witold Fijalkowski, Przemyslaw Patynowski, kohei.enju
On Sat, Feb 14, 2026 at 07:14:25PM +0000, Kohei Enju wrote:
> iavf incorrectly uses real_num_tx_queues for ETH_SS_STATS. Since the
> value could change in runtime, we should use num_tx_queues instead.
>
> Moreover iavf_get_ethtool_stats() uses num_active_queues while
> iavf_get_sset_count() and iavf_get_stat_strings() use
> real_num_tx_queues, which triggers out-of-bounds writes when we do
> "ethtool -L" and "ethtool -S" simultaneously [1].
>
> For example when we change channels from 1 to 8, Thread 3 could be
> scheduled before Thread 2, and out-of-bounds writes could be triggered
> in Thread 3:
>
> Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
> iavf_set_channels()
> ...
> iavf_alloc_queues()
> -> num_active_queues = 8
> iavf_schedule_finish_config()
> iavf_get_sset_count()
> real_num_tx_queues: 1
> -> buffer for 1 queue
> iavf_get_ethtool_stats()
> num_active_queues: 8
> -> out-of-bounds!
> iavf_finish_config()
> -> real_num_tx_queues = 8
...
> Fixes: 64430f70ba6f ("iavf: Fix displaying queue statistics shown by ethtool")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
...
> @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
> iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
>
> rcu_read_lock();
> - /* As num_active_queues describe both tx and rx queues, we can use
> - * it to iterate over rings' stats.
> + /* Use num_tx_queues to report stats for the maximum number of queues.
> + * Queues beyond num_active_queues will report zero.
> */
> - for (i = 0; i < adapter->num_active_queues; i++) {
> - struct iavf_ring *ring;
> + for (i = 0; i < netdev->num_tx_queues; i++) {
> + struct iavf_ring *tx_ring = NULL, *rx_ring = NULL;
>
> - /* Tx rings stats */
> - ring = &adapter->tx_rings[i];
> - iavf_add_queue_stats(&data, ring);
> + if (i < adapter->num_active_queues) {
Hi Enju-san,
If I understand things correctly, in the scenario described in the patch
description, num_active_queues will be 8 here.
Won't that result in an overflow?
> + tx_ring = &adapter->tx_rings[i];
> + rx_ring = &adapter->rx_rings[i];
> + }
>
> - /* Rx rings stats */
> - ring = &adapter->rx_rings[i];
> - iavf_add_queue_stats(&data, ring);
> + iavf_add_queue_stats(&data, tx_ring);
> + iavf_add_queue_stats(&data, rx_ring);
> }
> rcu_read_unlock();
> }
...
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-16 13:19 ` Simon Horman
@ 2026-02-16 14:44 ` Kohei Enju
2026-02-16 15:17 ` Kohei Enju
2026-02-17 16:02 ` Simon Horman
0 siblings, 2 replies; 9+ messages in thread
From: Kohei Enju @ 2026-02-16 14:44 UTC (permalink / raw)
To: horms
Cc: andrew+netdev, anthony.l.nguyen, davem, edumazet, intel-wired-lan,
jedrzej.jagielski, kohei.enju, kohei, kuba, mateusz.palczewski,
netdev, pabeni, przemyslaw.kitszel, przemyslawx.patynowski,
witoldx.fijalkowski
On Mon, 16 Feb 2026 13:19:09 +0000, Simon Horman wrote:
> > @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
> > iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
> >
> > rcu_read_lock();
> > - /* As num_active_queues describe both tx and rx queues, we can use
> > - * it to iterate over rings' stats.
> > + /* Use num_tx_queues to report stats for the maximum number of queues.
> > + * Queues beyond num_active_queues will report zero.
> > */
> > - for (i = 0; i < adapter->num_active_queues; i++) {
> > - struct iavf_ring *ring;
> > + for (i = 0; i < netdev->num_tx_queues; i++) {
> > + struct iavf_ring *tx_ring = NULL, *rx_ring = NULL;
> >
> > - /* Tx rings stats */
> > - ring = &adapter->tx_rings[i];
> > - iavf_add_queue_stats(&data, ring);
> > + if (i < adapter->num_active_queues) {
>
> Hi Enju-san,
Hi Horman-san, thank you for reviewing!
>
> If I understand things correctly, in the scenario described in the patch
> description, num_active_queues will be 8 here.
Yes.
>
> Won't that result in an overflow?
I think it won't overflow.
In Thread 1, iavf_set_channels(), which allocates {tx,rx}_rings and
updates num_active_queues, is executed under netdev lock. Therefore
Thread 3, which is also executed under the netdev lock, sees updated
num_active_queues and {tx,rx}_rings.
The scenario flow lacked netdev_(un)lock, my bad.
Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
netdev_lock()
iavf_set_channels()
...
iavf_alloc_queues()
-> alloc {tx,rx}_rings
-> num_active_queues = 8
iavf_schedule_finish_config()
netdev_unlock()
netdev_lock()
iavf_get_sset_count()
real_num_tx_queues: 1
-> buffer for 1 queue
iavf_get_ethtool_stats()
num_active_queues: 8
-> out-of-bounds!
netdev_unlock()
iavf_finish_config()
netdev_lock()
-> real_num_tx_queues = 8
netdev_unlock()
>
> > + tx_ring = &adapter->tx_rings[i];
> > + rx_ring = &adapter->rx_rings[i];
> > + }
> >
> > - /* Rx rings stats */
> > - ring = &adapter->rx_rings[i];
> > - iavf_add_queue_stats(&data, ring);
> > + iavf_add_queue_stats(&data, tx_ring);
> > + iavf_add_queue_stats(&data, rx_ring);
> > }
> > rcu_read_unlock();
> > }
>
> ...
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-16 14:44 ` Kohei Enju
@ 2026-02-16 15:17 ` Kohei Enju
2026-02-17 16:02 ` Simon Horman
1 sibling, 0 replies; 9+ messages in thread
From: Kohei Enju @ 2026-02-16 15:17 UTC (permalink / raw)
To: kohei
Cc: andrew+netdev, anthony.l.nguyen, davem, edumazet, horms,
intel-wired-lan, jedrzej.jagielski, kohei.enju, kuba,
mateusz.palczewski, netdev, pabeni, przemyslaw.kitszel,
przemyslawx.patynowski, witoldx.fijalkowski
On Mon, 16 Feb 2026 14:44:57 +0000, Kohei Enju wrote:
> The scenario flow lacked netdev_(un)lock, my bad.
>
> Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
> netdev_lock()
> iavf_set_channels()
> ...
> iavf_alloc_queues()
> -> alloc {tx,rx}_rings
> -> num_active_queues = 8
> iavf_schedule_finish_config()
> netdev_unlock()
> netdev_lock()
> iavf_get_sset_count()
> real_num_tx_queues: 1
> -> buffer for 1 queue
> iavf_get_ethtool_stats()
> num_active_queues: 8
> -> out-of-bounds!
> netdev_unlock()
> iavf_finish_config()
> netdev_lock()
> -> real_num_tx_queues = 8
> netdev_unlock()
Note that the flow of Thread 1 is based on a recent commit
b542fa9dd830 ("iavf: fix incorrect reset handling in callbacks") in
tnguy/net-queue.git.
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-16 14:44 ` Kohei Enju
2026-02-16 15:17 ` Kohei Enju
@ 2026-02-17 16:02 ` Simon Horman
1 sibling, 0 replies; 9+ messages in thread
From: Simon Horman @ 2026-02-17 16:02 UTC (permalink / raw)
To: Kohei Enju
Cc: andrew+netdev, anthony.l.nguyen, davem, edumazet, intel-wired-lan,
jedrzej.jagielski, kohei.enju, kuba, mateusz.palczewski, netdev,
pabeni, przemyslaw.kitszel, przemyslawx.patynowski,
witoldx.fijalkowski
On Mon, Feb 16, 2026 at 02:44:57PM +0000, Kohei Enju wrote:
> On Mon, 16 Feb 2026 13:19:09 +0000, Simon Horman wrote:
>
> > > @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
> > > iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
> > >
> > > rcu_read_lock();
> > > - /* As num_active_queues describe both tx and rx queues, we can use
> > > - * it to iterate over rings' stats.
> > > + /* Use num_tx_queues to report stats for the maximum number of queues.
> > > + * Queues beyond num_active_queues will report zero.
> > > */
> > > - for (i = 0; i < adapter->num_active_queues; i++) {
> > > - struct iavf_ring *ring;
> > > + for (i = 0; i < netdev->num_tx_queues; i++) {
> > > + struct iavf_ring *tx_ring = NULL, *rx_ring = NULL;
> > >
> > > - /* Tx rings stats */
> > > - ring = &adapter->tx_rings[i];
> > > - iavf_add_queue_stats(&data, ring);
> > > + if (i < adapter->num_active_queues) {
> >
> > Hi Enju-san,
>
> Hi Horman-san, thank you for reviewing!
>
> >
> > If I understand things correctly, in the scenario described in the patch
> > description, num_active_queues will be 8 here.
>
> Yes.
>
> >
> > Won't that result in an overflow?
>
> I think it won't overflow.
>
> In Thread 1, iavf_set_channels(), which allocates {tx,rx}_rings and
> updates num_active_queues, is executed under netdev lock. Therefore
> Thread 3, which is also executed under the netdev lock, sees updated
> num_active_queues and {tx,rx}_rings.
>
> The scenario flow lacked netdev_(un)lock, my bad.
>
> Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
> netdev_lock()
> iavf_set_channels()
> ...
> iavf_alloc_queues()
> -> alloc {tx,rx}_rings
> -> num_active_queues = 8
> iavf_schedule_finish_config()
> netdev_unlock()
> netdev_lock()
> iavf_get_sset_count()
> real_num_tx_queues: 1
> -> buffer for 1 queue
> iavf_get_ethtool_stats()
> num_active_queues: 8
> -> out-of-bounds!
> netdev_unlock()
> iavf_finish_config()
> netdev_lock()
> -> real_num_tx_queues = 8
> netdev_unlock()
Thanks, and sorry for missing that the first time around.
With that clarified in my mind this looks good to me.
Reviewed-by: Simon Horman <horms@kernel.org>
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-14 19:14 [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats() Kohei Enju
2026-02-16 13:19 ` Simon Horman
@ 2026-02-18 5:39 ` Przemek Kitszel
2026-02-18 6:31 ` Kohei Enju
2026-02-18 8:00 ` [Intel-wired-lan] " Paul Menzel
2 siblings, 1 reply; 9+ messages in thread
From: Przemek Kitszel @ 2026-02-18 5:39 UTC (permalink / raw)
To: Kohei Enju, intel-wired-lan, netdev
Cc: Tony Nguyen, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Jedrzej Jagielski, kohei.enju
On 2/14/26 20:14, Kohei Enju wrote:
> iavf incorrectly uses real_num_tx_queues for ETH_SS_STATS. Since the
> value could change in runtime, we should use num_tx_queues instead.
>
[...]
> Use immutable num_tx_queues in all related functions to avoid the issue.
>
> [1]
> BUG: KASAN: vmalloc-out-of-bounds in iavf_add_one_ethtool_stat+0x200/0x270
[...]
Thank you very much Enju-san,
this is a nice improvement!
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Side note: I'm working on enabling 256 queues for iavf, what means
changing the reported max number of queues, what finally means screens
of zero stats printed, unfortunate, but better be correct that pretty ;)
>
> Fixes: 64430f70ba6f ("iavf: Fix displaying queue statistics shown by ethtool")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> ---
> .../net/ethernet/intel/iavf/iavf_ethtool.c | 31 +++++++++----------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> index 6ff3842a1ff1..98bec3afc200 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> @@ -313,14 +313,13 @@ static int iavf_get_sset_count(struct net_device *netdev, int sset)
> {
> /* Report the maximum number queues, even if not every queue is
> * currently configured. Since allocation of queues is in pairs,
> - * use netdev->real_num_tx_queues * 2. The real_num_tx_queues is set
> - * at device creation and never changes.
> + * use netdev->num_tx_queues * 2. The num_tx_queues is set at
> + * device creation and never changes.
> */
>
> if (sset == ETH_SS_STATS)
> return IAVF_STATS_LEN +
> - (IAVF_QUEUE_STATS_LEN * 2 *
> - netdev->real_num_tx_queues);
> + (IAVF_QUEUE_STATS_LEN * 2 * netdev->num_tx_queues);
> else
> return -EINVAL;
> }
> @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
> iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
>
> rcu_read_lock();
> - /* As num_active_queues describe both tx and rx queues, we can use
> - * it to iterate over rings' stats.
> + /* Use num_tx_queues to report stats for the maximum number of queues.
> + * Queues beyond num_active_queues will report zero.
<rambling>
zeroing inactive stats is bad behavior, not introduced by you of course,
but worth to fix (as a followup, not necessarily by you)
unrelated to the prev paragraph,
even if we remove explicit = 0 from the driver, ethtool userspace will
still print (lots of) zeroes for "under-reported" stats, thanks to
multi-syscall stats retrieval, that could be "fixed" in ethtool, but
perhaps does not matter, as users use wrappers anyway
</rambling>
> */
> - for (i = 0; i < adapter->num_active_queues; i++) {
> - struct iavf_ring *ring;
> + for (i = 0; i < netdev->num_tx_queues; i++) {
> + struct iavf_ring *tx_ring = NULL, *rx_ring = NULL;
>
> - /* Tx rings stats */
> - ring = &adapter->tx_rings[i];
> - iavf_add_queue_stats(&data, ring);
> + if (i < adapter->num_active_queues) {
> + tx_ring = &adapter->tx_rings[i];
> + rx_ring = &adapter->rx_rings[i];
> + }
>
> - /* Rx rings stats */
> - ring = &adapter->rx_rings[i];
> - iavf_add_queue_stats(&data, ring);
> + iavf_add_queue_stats(&data, tx_ring);
> + iavf_add_queue_stats(&data, rx_ring);
> }
> rcu_read_unlock();
> }
> @@ -376,9 +375,9 @@ static void iavf_get_stat_strings(struct net_device *netdev, u8 *data)
> iavf_add_stat_strings(&data, iavf_gstrings_stats);
>
> /* Queues are always allocated in pairs, so we just use
> - * real_num_tx_queues for both Tx and Rx queues.
> + * num_tx_queues for both Tx and Rx queues.
> */
> - for (i = 0; i < netdev->real_num_tx_queues; i++) {
> + for (i = 0; i < netdev->num_tx_queues; i++) {
> iavf_add_stat_strings(&data, iavf_gstrings_queue_stats,
> "tx", i);
> iavf_add_stat_strings(&data, iavf_gstrings_queue_stats,
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-18 5:39 ` Przemek Kitszel
@ 2026-02-18 6:31 ` Kohei Enju
0 siblings, 0 replies; 9+ messages in thread
From: Kohei Enju @ 2026-02-18 6:31 UTC (permalink / raw)
To: przemyslaw.kitszel
Cc: andrew+netdev, anthony.l.nguyen, davem, edumazet, intel-wired-lan,
jedrzej.jagielski, kohei.enju, kohei, kuba, netdev, pabeni
On Wed, 18 Feb 2026 06:39:43 +0100, Przemek Kitszel wrote:
> On 2/14/26 20:14, Kohei Enju wrote:
> > iavf incorrectly uses real_num_tx_queues for ETH_SS_STATS. Since the
> > value could change in runtime, we should use num_tx_queues instead.
> >
>
> [...]
>
> > Use immutable num_tx_queues in all related functions to avoid the issue.
> >
> > [1]
> > BUG: KASAN: vmalloc-out-of-bounds in iavf_add_one_ethtool_stat+0x200/0x270
>
> [...]
>
> Thank you very much Enju-san,
> this is a nice improvement!
> Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
>
> Side note: I'm working on enabling 256 queues for iavf, what means
Cool, and thank you for reviewing!
> changing the reported max number of queues, what finally means screens
> of zero stats printed, unfortunate, but better be correct that pretty ;)
Indeed.
>
> >
> > Fixes: 64430f70ba6f ("iavf: Fix displaying queue statistics shown by ethtool")
> > Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> > ---
> > .../net/ethernet/intel/iavf/iavf_ethtool.c | 31 +++++++++----------
> > 1 file changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > index 6ff3842a1ff1..98bec3afc200 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > @@ -313,14 +313,13 @@ static int iavf_get_sset_count(struct net_device *netdev, int sset)
> > {
> > /* Report the maximum number queues, even if not every queue is
> > * currently configured. Since allocation of queues is in pairs,
> > - * use netdev->real_num_tx_queues * 2. The real_num_tx_queues is set
> > - * at device creation and never changes.
> > + * use netdev->num_tx_queues * 2. The num_tx_queues is set at
> > + * device creation and never changes.
> > */
> >
> > if (sset == ETH_SS_STATS)
> > return IAVF_STATS_LEN +
> > - (IAVF_QUEUE_STATS_LEN * 2 *
> > - netdev->real_num_tx_queues);
> > + (IAVF_QUEUE_STATS_LEN * 2 * netdev->num_tx_queues);
> > else
> > return -EINVAL;
> > }
> > @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
> > iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
> >
> > rcu_read_lock();
> > - /* As num_active_queues describe both tx and rx queues, we can use
> > - * it to iterate over rings' stats.
> > + /* Use num_tx_queues to report stats for the maximum number of queues.
> > + * Queues beyond num_active_queues will report zero.
>
> <rambling>
> zeroing inactive stats is bad behavior, not introduced by you of course,
> but worth to fix (as a followup, not necessarily by you)
I agree.
>
> unrelated to the prev paragraph,
> even if we remove explicit = 0 from the driver, ethtool userspace will
> still print (lots of) zeroes for "under-reported" stats, thanks to
> multi-syscall stats retrieval, that could be "fixed" in ethtool, but
> perhaps does not matter, as users use wrappers anyway
> </rambling>
The root cause of the OOB this time is that iavf_get_sset_count() uses
real_num_tx_queues for allocating buffers, and iavf_get_ethtool_stats()
uses num_active_queues for writing [1].
This may warrant more thought, but I was thinking that using
num_active_queues in all related functions would also be acceptable.
That's because, thanks to netdev_lock(), we can ensure num_active_queues
is unchanged within each ioctl call (ETHTOOL_GSSET_INFO,
ETHTOOL_GSTRINGS, ETHTOOL_GSTATS).
In other words, num_active_queues is effectively immutable for the
duration of a single ioctl, avoiding OOB.
[1]
Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
netdev_lock()
iavf_set_channels()
...
iavf_alloc_queues()
-> alloc {tx,rx}_rings
-> num_active_queues = 8
iavf_schedule_finish_config()
netdev_unlock()
netdev_lock()
iavf_get_sset_count()
real_num_tx_queues: 1
-> buffer for 1 queue
iavf_get_ethtool_stats()
num_active_queues: 8
-> out-of-bounds!
netdev_unlock()
iavf_finish_config()
netdev_lock()
-> real_num_tx_queues = 8
netdev_unlock()
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [Intel-wired-lan] [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-14 19:14 [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats() Kohei Enju
2026-02-16 13:19 ` Simon Horman
2026-02-18 5:39 ` Przemek Kitszel
@ 2026-02-18 8:00 ` Paul Menzel
2026-03-19 9:14 ` Romanowski, Rafal
2 siblings, 1 reply; 9+ messages in thread
From: Paul Menzel @ 2026-02-18 8:00 UTC (permalink / raw)
To: Kohei Enju
Cc: intel-wired-lan, netdev, Tony Nguyen, Przemek Kitszel,
Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jedrzej Jagielski, Mateusz Palczewski,
Witold Fijalkowski, Przemyslaw Patynowski, kohei.enju
Dear Kohei,
Thank you for your patch.
Am 14.02.26 um 20:14 schrieb Kohei Enju:
> iavf incorrectly uses real_num_tx_queues for ETH_SS_STATS. Since the
> value could change in runtime, we should use num_tx_queues instead.
>
> Moreover iavf_get_ethtool_stats() uses num_active_queues while
> iavf_get_sset_count() and iavf_get_stat_strings() use
> real_num_tx_queues, which triggers out-of-bounds writes when we do
> "ethtool -L" and "ethtool -S" simultaneously [1].
>
> For example when we change channels from 1 to 8, Thread 3 could be
> scheduled before Thread 2, and out-of-bounds writes could be triggered
> in Thread 3:
>
> Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
> iavf_set_channels()
> ...
> iavf_alloc_queues()
> -> num_active_queues = 8
> iavf_schedule_finish_config()
> iavf_get_sset_count()
> real_num_tx_queues: 1
> -> buffer for 1 queue
> iavf_get_ethtool_stats()
> num_active_queues: 8
> -> out-of-bounds!
> iavf_finish_config()
> -> real_num_tx_queues = 8
>
> Use immutable num_tx_queues in all related functions to avoid the issue.
>
> [1]
> BUG: KASAN: vmalloc-out-of-bounds in iavf_add_one_ethtool_stat+0x200/0x270
> Write of size 8 at addr ffffc900031c9080 by task ethtool/5800
>
> CPU: 1 UID: 0 PID: 5800 Comm: ethtool Not tainted 6.19.0-enjuk-08403-g8137e3db7f1c #241 PREEMPT(full)
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
> Call Trace:
> <TASK>
> dump_stack_lvl+0x6f/0xb0
> print_report+0x170/0x4f3
> kasan_report+0xe1/0x180
> iavf_add_one_ethtool_stat+0x200/0x270
> iavf_get_ethtool_stats+0x14c/0x2e0
> __dev_ethtool+0x3d0c/0x5830
> dev_ethtool+0x12d/0x270
> dev_ioctl+0x53c/0xe30
> sock_do_ioctl+0x1a9/0x270
> sock_ioctl+0x3d4/0x5e0
> __x64_sys_ioctl+0x137/0x1c0
> do_syscall_64+0xf3/0x690
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f7da0e6e36d
> ...
> </TASK>
>
> The buggy address belongs to a 1-page vmalloc region starting at 0xffffc900031c9000 allocated at __dev_ethtool+0x3cc9/0x5830
> The buggy address belongs to the physical page: page: refcount:1 mapcount:0 mapping:0000000000000000
> index:0xffff88813a013de0 pfn:0x13a013
> flags: 0x200000000000000(node=0|zone=2)
> raw: 0200000000000000 0000000000000000 dead000000000122 0000000000000000
> raw: ffff88813a013de0 0000000000000000 00000001ffffffff 0000000000000000
> page dumped because: kasan: bad access detected
>
> Memory state around the buggy address:
> ffffc900031c8f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> ffffc900031c9000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> >ffffc900031c9080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> ^
> ffffc900031c9100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> ffffc900031c9180: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
>
> Fixes: 64430f70ba6f ("iavf: Fix displaying queue statistics shown by ethtool")
> Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> ---
> .../net/ethernet/intel/iavf/iavf_ethtool.c | 31 +++++++++----------
> 1 file changed, 15 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> index 6ff3842a1ff1..98bec3afc200 100644
> --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> @@ -313,14 +313,13 @@ static int iavf_get_sset_count(struct net_device *netdev, int sset)
> {
> /* Report the maximum number queues, even if not every queue is
> * currently configured. Since allocation of queues is in pairs,
> - * use netdev->real_num_tx_queues * 2. The real_num_tx_queues is set
> - * at device creation and never changes.
> + * use netdev->num_tx_queues * 2. The num_tx_queues is set at
> + * device creation and never changes.
> */
>
> if (sset == ETH_SS_STATS)
> return IAVF_STATS_LEN +
> - (IAVF_QUEUE_STATS_LEN * 2 *
> - netdev->real_num_tx_queues);
> + (IAVF_QUEUE_STATS_LEN * 2 * netdev->num_tx_queues);
> else
> return -EINVAL;
> }
> @@ -345,19 +344,19 @@ static void iavf_get_ethtool_stats(struct net_device *netdev,
> iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats);
>
> rcu_read_lock();
> - /* As num_active_queues describe both tx and rx queues, we can use
> - * it to iterate over rings' stats.
> + /* Use num_tx_queues to report stats for the maximum number of queues.
> + * Queues beyond num_active_queues will report zero.
> */
> - for (i = 0; i < adapter->num_active_queues; i++) {
> - struct iavf_ring *ring;
> + for (i = 0; i < netdev->num_tx_queues; i++) {
> + struct iavf_ring *tx_ring = NULL, *rx_ring = NULL;
>
> - /* Tx rings stats */
> - ring = &adapter->tx_rings[i];
> - iavf_add_queue_stats(&data, ring);
> + if (i < adapter->num_active_queues) {
> + tx_ring = &adapter->tx_rings[i];
> + rx_ring = &adapter->rx_rings[i];
> + }
>
> - /* Rx rings stats */
> - ring = &adapter->rx_rings[i];
> - iavf_add_queue_stats(&data, ring);
> + iavf_add_queue_stats(&data, tx_ring);
> + iavf_add_queue_stats(&data, rx_ring);
> }
> rcu_read_unlock();
> }
> @@ -376,9 +375,9 @@ static void iavf_get_stat_strings(struct net_device *netdev, u8 *data)
> iavf_add_stat_strings(&data, iavf_gstrings_stats);
>
> /* Queues are always allocated in pairs, so we just use
> - * real_num_tx_queues for both Tx and Rx queues.
> + * num_tx_queues for both Tx and Rx queues.
> */
> - for (i = 0; i < netdev->real_num_tx_queues; i++) {
> + for (i = 0; i < netdev->num_tx_queues; i++) {
> iavf_add_stat_strings(&data, iavf_gstrings_queue_stats,
> "tx", i);
> iavf_add_stat_strings(&data, iavf_gstrings_queue_stats,
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply [flat|nested] 9+ messages in thread* RE: [Intel-wired-lan] [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in iavf_get_ethtool_stats()
2026-02-18 8:00 ` [Intel-wired-lan] " Paul Menzel
@ 2026-03-19 9:14 ` Romanowski, Rafal
0 siblings, 0 replies; 9+ messages in thread
From: Romanowski, Rafal @ 2026-03-19 9:14 UTC (permalink / raw)
To: Paul Menzel, Kohei Enju
Cc: intel-wired-lan@lists.osuosl.org, netdev@vger.kernel.org,
Nguyen, Anthony L, Kitszel, Przemyslaw, Andrew Lunn,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Jagielski, Jedrzej, Mateusz Palczewski, Witold Fijalkowski,
Przemyslaw Patynowski, kohei.enju@gmail.com
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Paul
> Menzel
> Sent: Wednesday, February 18, 2026 9:01 AM
> To: Kohei Enju <kohei@enjuk.jp>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; 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>; Jagielski, Jedrzej <jedrzej.jagielski@intel.com>; Mateusz
> Palczewski <mateusz.palczewski@intel.com>; Witold Fijalkowski
> <witoldx.fijalkowski@intel.com>; Przemyslaw Patynowski
> <przemyslawx.patynowski@intel.com>; kohei.enju@gmail.com
> Subject: Re: [Intel-wired-lan] [PATCH v1 iwl-net] iavf: fix out-of-bounds writes in
> iavf_get_ethtool_stats()
>
> Dear Kohei,
>
>
> Thank you for your patch.
>
> Am 14.02.26 um 20:14 schrieb Kohei Enju:
> > iavf incorrectly uses real_num_tx_queues for ETH_SS_STATS. Since the
> > value could change in runtime, we should use num_tx_queues instead.
> >
> > Moreover iavf_get_ethtool_stats() uses num_active_queues while
> > iavf_get_sset_count() and iavf_get_stat_strings() use
> > real_num_tx_queues, which triggers out-of-bounds writes when we do
> > "ethtool -L" and "ethtool -S" simultaneously [1].
> >
> > For example when we change channels from 1 to 8, Thread 3 could be
> > scheduled before Thread 2, and out-of-bounds writes could be triggered
> > in Thread 3:
> >
> > Thread 1 (ethtool -L) Thread 2 (work) Thread 3 (ethtool -S)
> > iavf_set_channels()
> > ...
> > iavf_alloc_queues()
> > -> num_active_queues = 8
> > iavf_schedule_finish_config()
> > iavf_get_sset_count()
> > real_num_tx_queues: 1
> > -> buffer for 1 queue
> > iavf_get_ethtool_stats()
> > num_active_queues: 8
> > -> out-of-bounds!
> > iavf_finish_config()
> > -> real_num_tx_queues = 8
> >
> > Use immutable num_tx_queues in all related functions to avoid the issue.
> >
> > [1]
> > BUG: KASAN: vmalloc-out-of-bounds in
> iavf_add_one_ethtool_stat+0x200/0x270
> > Write of size 8 at addr ffffc900031c9080 by task ethtool/5800
> >
> > CPU: 1 UID: 0 PID: 5800 Comm: ethtool Not tainted 6.19.0-enjuk-08403-
> g8137e3db7f1c #241 PREEMPT(full)
> > Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-
> 1.16.3-2 04/01/2014
> > Call Trace:
> > <TASK>
> > dump_stack_lvl+0x6f/0xb0
> > print_report+0x170/0x4f3
> > kasan_report+0xe1/0x180
> > iavf_add_one_ethtool_stat+0x200/0x270
> > iavf_get_ethtool_stats+0x14c/0x2e0
> > __dev_ethtool+0x3d0c/0x5830
> > dev_ethtool+0x12d/0x270
> > dev_ioctl+0x53c/0xe30
> > sock_do_ioctl+0x1a9/0x270
> > sock_ioctl+0x3d4/0x5e0
> > __x64_sys_ioctl+0x137/0x1c0
> > do_syscall_64+0xf3/0x690
> > entry_SYSCALL_64_after_hwframe+0x77/0x7f
> > RIP: 0033:0x7f7da0e6e36d
> > ...
> > </TASK>
> >
> > The buggy address belongs to a 1-page vmalloc region starting at
> 0xffffc900031c9000 allocated at __dev_ethtool+0x3cc9/0x5830
> > The buggy address belongs to the physical page: page: refcount:1 mapcount:0
> mapping:0000000000000000
> > index:0xffff88813a013de0 pfn:0x13a013
> > flags: 0x200000000000000(node=0|zone=2)
> > raw: 0200000000000000 0000000000000000 dead000000000122
> 0000000000000000
> > raw: ffff88813a013de0 0000000000000000 00000001ffffffff
> 0000000000000000
> > page dumped because: kasan: bad access detected
> >
> > Memory state around the buggy address:
> > ffffc900031c8f80: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> > ffffc900031c9000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
> > >ffffc900031c9080: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> > ^
> > ffffc900031c9100: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> > ffffc900031c9180: f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8 f8
> >
> > Fixes: 64430f70ba6f ("iavf: Fix displaying queue statistics shown by
> > ethtool")
> > Signed-off-by: Kohei Enju <kohei@enjuk.jp>
> > ---
> > .../net/ethernet/intel/iavf/iavf_ethtool.c | 31 +++++++++----------
> > 1 file changed, 15 insertions(+), 16 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > index 6ff3842a1ff1..98bec3afc200 100644
> > --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c
> > @@ -313,14 +313,13 @@ static int iavf_get_sset_count(struct net_device
Tested-by: Rafal Romanowski <rafal.romanowski@intel.com>
^ permalink raw reply [flat|nested] 9+ messages in thread