From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtpout-03.galae.net (smtpout-03.galae.net [185.246.85.4]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 388DF378D87 for ; Fri, 7 Aug 2026 16:11:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.246.85.4 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786119081; cv=none; b=tZgZlAUlNYy0Cw2eNyA7ciUowswvEiubcGZAuPZQm4vAuz3nk4jtqij+ZLBo4zKQ///Wdux8a2wyF1J7hiWiEYcKMg2bA6TSZKMTAwnScSwFCaZjdWmSSqJstpVPsMwghgjY0wcbKDTaBulKn+xvXy9g76QPdp1oHGnKk9fuIa0= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786119081; c=relaxed/simple; bh=1scJK/kSKrc7d5AvdTdAGwnyVyMDz/ioBzZqmGZkV5c=; h=Content-Type:Date:Message-Id:Subject:Cc:To:From:In-Reply-To: References:MIME-Version; b=j9cBiaRSl3+VDkIFIqo487do54Us60/vzAr/46KMSpwQQykLZQldiPeEqEO6BSukX303Ll9704UgYytJmJrAKEcebmPm8vks8aXgy8hv+BP2CK/H6lvs7EITuQkgiDzZql2bZTj5uP7ETTpOHVpprha/4QWSMH6w8MDOI4aX2Ow= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com; spf=pass smtp.mailfrom=bootlin.com; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b=Ow/j/6hA; arc=none smtp.client-ip=185.246.85.4 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=bootlin.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=bootlin.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=bootlin.com header.i=@bootlin.com header.b="Ow/j/6hA" Received: from smtpout-01.galae.net (smtpout-01.galae.net [212.83.139.233]) by smtpout-03.galae.net (Postfix) with ESMTPS id 6537C4E4113F; Fri, 7 Aug 2026 16:11:17 +0000 (UTC) Received: from mail.galae.net (mail.galae.net [212.83.136.155]) by smtpout-01.galae.net (Postfix) with ESMTPS id 1FD64603F0; Fri, 7 Aug 2026 16:11:17 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by localhost (Mailerdaemon) with ESMTPSA id D3A2B11C37B34; Fri, 7 Aug 2026 18:11:06 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bootlin.com; s=dkim; t=1786119072; h=from:subject:date:message-id:to:cc:mime-version:content-type: content-transfer-encoding:in-reply-to:references; bh=9fu+cUvJlXgYu6yEmBzntFpysMJ3dt/4KQZXu2Enl48=; b=Ow/j/6hAdgBRhKZJHAHEhFiAW7IIc6Lm9uuH6bIytQ+3UTjRPqRE2eaZU9cdcbXmPPlOsI YZMUarHUOh9GdQ6LDW8rXJBB4pmPvRogb1xq0VGKAM4yJ3lZZwbxSAiJDUDIou0g69yiGR 1a/N+oIfmJmHNGXJxiNC5i4V0U36v6Pp5+c7SRpuBBlIOlbgWCeTEINqZiPbDPDbOkc/yR CebMV9mDcGj5grUrw8OtvE92lEoAE5hmBoHe6/1ZRXapygBarhJmd3vtCKQzEYknWGTn2M D8dmdCoHTfnWxez4wp8ZIBCY/MYECvDVLqbYiDOMTeLecjEz0NpbqKe9s9Ar+A== Content-Type: text/plain; charset=UTF-8 Date: Fri, 07 Aug 2026 18:11:06 +0200 Message-Id: Subject: Re: [PATCH net-next v8 17/17] net: macb: use context swapping in .ndo_change_mtu() Cc: , , "Nicolas Ferre" , "Claudiu Beznea" , "Paolo Valerio" , "Nicolai Buchwitz" , "Vladimir Kondratiev" , "Gregory CLEMENT" , =?utf-8?q?Beno=C3=AEt_Monin?= , "Tawfik Bayouk" , "Thomas Petazzoni" , "Maxime Chevallier" To: =?utf-8?q?Th=C3=A9o_Lebrun?= , "Conor Dooley" , "Andrew Lunn" , "David S. Miller" , "Eric Dumazet" , "Jakub Kicinski" , "Paolo Abeni" , "Richard Cochran" , "Russell King" From: =?utf-8?q?Th=C3=A9o_Lebrun?= In-Reply-To: <20260805-macb-context-v8-17-bc302ffd1174@bootlin.com> References: <20260805-macb-context-v8-17-bc302ffd1174@bootlin.com> Content-Transfer-Encoding: quoted-printable Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-TUID: t2rQwsuV9FQi X-Last-TLS-Session-Version: TLSv1.3 Replying to sashiko https://sashiko.dev/#/patchset/20260805-macb-context-v8-0-bc302ffd1174%40bo= otlin.com > commit 370786abcd6adf66f9c2032c12fec77c99f4d627 > Author: Th=C3=A9o Lebrun > > net: macb: use context swapping in .ndo_change_mtu() > > Use newly introduced context buffer management to implement > .ndo_change_mtu() as a context swap: allocate new context -> > reconfigure HW -> free old context. > > This resists memory pressure well by failing without closing the > interface and it is much faster by avoiding PHY reinit. > > > diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/eth= ernet/cadence/macb_main.c > > index 4e004a5345b39..b3d84a65078f8 100644 > > --- a/drivers/net/ethernet/cadence/macb_main.c > > +++ b/drivers/net/ethernet/cadence/macb_main.c > > @@ -3533,11 +3533,29 @@ static int macb_close(struct net_device *netdev= ) > > =20 > > static int macb_change_mtu(struct net_device *netdev, int new_mtu) > > { > > - if (netif_running(netdev)) > > - return -EBUSY; > > + struct macb *bp =3D netdev_priv(netdev); > > + bool running =3D netif_running(netdev); > > + struct macb_context *new_ctx; > > + > > + if (running) { > > + /* Context swapping is not supported for AT91. */ > > + if (bp->caps & MACB_CAPS_MACB_IS_EMAC) > > + return -EBUSY; > > + > > + new_ctx =3D macb_context_alloc(bp, new_mtu, > > + bp->configured_rx_ring_size, > > + bp->configured_tx_ring_size); > > [...] > > > + if (IS_ERR(new_ctx)) > > + return PTR_ERR(new_ctx); > > + > > + macb_context_swap_start(bp); > > + } > > =20 > > WRITE_ONCE(netdev->mtu, new_mtu); > > =20 > > + if (running) > > + macb_context_swap_end(bp, new_ctx); > > [Severity: High] > Can the watchdog timer cause a use-after-free by concurrently accessing t= he > old context pointer while macb_context_swap_end() is freeing it? > > If the interface is idle when the MTU is changed, the trans_start timesta= mp > may be older than the watchdog timeout. macb_context_swap_start() calls > netif_tx_disable() to stop the tx queues, but does not update trans_start= . > > This can allow the asynchronous dev_watchdog timer to conclude a tx timeo= ut > has occurred and trigger macb_tx_timeout() -> macb_tx_restart(), which re= ads > the context: > > static void macb_tx_restart(struct macb_queue *queue) > { > struct macb_context *ctx =3D queue->bp->ctx; > struct macb_txq *txq =3D macb_txq(queue); > > At the same time, macb_context_swap_end() replaces the context pointer > and frees the old context: > > macb_free(old_ctx); > kfree(old_ctx); > > Could this race cause memory corruption if macb_tx_restart() accesses the= old > context after it is freed? Well, this is a fresh report for once! But it is completely unrelated to our series at hand. We implement context swapping. Let's investigate anyway. - First I checked and nothing serialises all those call together at the subsystem layer. ndo_tx_timeout is only under netdev->tx_global_lock. - ndo_tx_timeout is in softirq context so no bp->mac_cfg_lock mutex to save us, that would have been the easy solution. - The proper solution is therefore bp->lock with a bp->ctx_swap check inside. As queue->tx_ptr_lock is also involved we must make sure to respect the ordering done elsewhere in the driver, so something like the following in macb_tx_timeout() should fix it: spin_lock_irqsave(&queue->tx_ptr_lock, flags); spin_lock(&bp->lock); if (bp->ctx_swap) goto out; ... spin_unlock(&bp->lock); spin_unlock_irqrestore(&queue->tx_ptr_lock, flags); This is all out of scope and series is way too large already, it'll have to wait. Thanks, --=20 Th=C3=A9o Lebrun, Bootlin Embedded Linux and Kernel engineering https://bootlin.com