All of lore.kernel.org
 help / color / mirror / Atom feed
From: Nicolai Buchwitz <nb@tipi-net.de>
To: Paolo Valerio <pvalerio@redhat.com>
Cc: netdev@vger.kernel.org,
	"Nicolas Ferre" <nicolas.ferre@microchip.com>,
	"Claudiu Beznea" <claudiu.beznea@tuxon.dev>,
	"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>,
	"Lorenzo Bianconi" <lorenzo@kernel.org>,
	"Théo Lebrun" <theo.lebrun@bootlin.com>
Subject: Re: [PATCH net-next v5 4/8] net: macb: use the current queue number for stats
Date: Mon, 16 Mar 2026 17:30:48 +0100	[thread overview]
Message-ID: <500d698680fb51285f78fa68b6e41875@tipi-net.de> (raw)
In-Reply-To: <20260313201433.2346119-5-pvalerio@redhat.com>

On 13.3.2026 21:14, Paolo Valerio wrote:
> gem_get_ethtool_stats calculates the size of the statistics
> data to copy always considering maximum number of queues.
> 
> The patch makes sure the statistics are copied only for the
> active queues as returned in the string set count op.
> 
> Signed-off-by: Paolo Valerio <pvalerio@redhat.com>
> ---
>  drivers/net/ethernet/cadence/macb_main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 06ad8c8ec036..fbeaa85b4a9c 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -3528,7 +3528,7 @@ static void gem_get_ethtool_stats(struct 
> net_device *dev,
>  	spin_lock_irq(&bp->stats_lock);
>  	gem_update_stats(bp);
>  	memcpy(data, &bp->ethtool_stats, sizeof(u64)
> -			* (GEM_STATS_LEN + QUEUE_STATS_LEN * MACB_MAX_QUEUES));
> +			* (GEM_STATS_LEN + QUEUE_STATS_LEN * bp->num_queues));

This is an out-of-bounds write, not just a cosmetic change.
gem_get_sset_count() returns GEM_STATS_LEN + QUEUE_STATS_LEN *
bp->num_queues, and ethtool allocates the data buffer based on that 
count.
The old memcpy with MACB_MAX_QUEUES (8) writes past the end of the
buffer on any GEM instance with fewer than 8 hardware queues.

KASAN confirms on RP1 (1 queue) without this patch applied:

   BUG: KASAN: vmalloc-out-of-bounds in gem_get_ethtool_stats+0x50/0x78
   Write of size 760 at addr ffffffc0822e7000 by task ethtool/922

The overflow stays within the vzalloc page slack, so the practical
impact is low - but it's still an out-of-bounds write that exists in
the current upstream code. Might be worth splitting this out as a
standalone fix targeting net with a Fixes: tag, and updating the commit
message accordingly?

>  	spin_unlock_irq(&bp->stats_lock);
>  }

Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>

Thanks
Nicolai

  reply	other threads:[~2026-03-16 16:30 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-13 20:14 [PATCH net-next v5 0/8] net: macb: Add XDP support and page pool integration Paolo Valerio
2026-03-13 20:14 ` [PATCH net-next v5 1/8] net: macb: move Rx buffers alloc from link up to open Paolo Valerio
2026-03-16 12:19   ` Nicolai Buchwitz
2026-03-13 20:14 ` [PATCH net-next v5 2/8] net: macb: rename rx_skbuff into rx_buff Paolo Valerio
2026-03-16 12:20   ` Nicolai Buchwitz
2026-03-13 20:14 ` [PATCH net-next v5 3/8] net: macb: Add page pool support handle multi-descriptor frame rx Paolo Valerio
2026-03-13 20:14 ` [PATCH net-next v5 4/8] net: macb: use the current queue number for stats Paolo Valerio
2026-03-16 16:30   ` Nicolai Buchwitz [this message]
2026-03-18 21:27     ` Paolo Valerio
2026-03-13 20:14 ` [PATCH net-next v5 5/8] net: macb: make macb_tx_skb generic Paolo Valerio
2026-03-16 12:21   ` Nicolai Buchwitz
2026-03-13 20:14 ` [PATCH net-next v5 6/8] net: macb: generalize tx buffer handling Paolo Valerio
2026-03-16 12:18   ` Nicolai Buchwitz
2026-03-13 20:14 ` [PATCH net-next v5 7/8] net: macb: add XDP support for gem Paolo Valerio
2026-03-13 20:14 ` [PATCH net-next v5 8/8] net: macb: introduce ndo_xdp_xmit support Paolo Valerio
2026-03-16 12:12 ` [PATCH net-next v5 0/8] net: macb: Add XDP support and page pool integration Nicolai Buchwitz

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=500d698680fb51285f78fa68b6e41875@tipi-net.de \
    --to=nb@tipi-net.de \
    --cc=andrew+netdev@lunn.ch \
    --cc=claudiu.beznea@tuxon.dev \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=lorenzo@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=nicolas.ferre@microchip.com \
    --cc=pabeni@redhat.com \
    --cc=pvalerio@redhat.com \
    --cc=theo.lebrun@bootlin.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.