[PATCH net] net: ag71xx: reallocate rx buffers when changing the MTU

[PATCH net] net: ag71xx: reallocate rx buffers when changing the MTU

[Rosen Penev]

The rx buffers are sized from the MTU in ag71xx_open(), but
ag71xx_change_mtu() writes the new maximum frame length straight to
the MAC while the interface is running. The rx descriptors carry no
buffer length, so after raising the MTU the MAC accepts and DMAs
frames larger than the mapped buffer, overwriting the skb_shared_info
at the end of the fragment and whatever follows it.

This is reachable without user action: DSA raises the MTU of a
running conduit to make room for the switch tag.

Restart the interface when the MTU changes while it is up, so the
rings are refilled with buffers sized for the new MTU before the MAC
is programmed with the new frame length.

Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
---
 drivers/net/ethernet/atheros/ag71xx.c | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
index 8a3c689f6825..3c9de8c2ba64 100644
--- a/drivers/net/ethernet/atheros/ag71xx.c
+++ b/drivers/net/ethernet/atheros/ag71xx.c
@@ -1761,11 +1761,19 @@ static irqreturn_t ag71xx_interrupt(int irq, void *dev_id)
 
 static int ag71xx_change_mtu(struct net_device *ndev, int new_mtu)
 {
-	struct ag71xx *ag = netdev_priv(ndev);
+	bool running = netif_running(ndev);
+
+	/* The rx buffers are sized for the MTU at open time and the rx
+	 * descriptors carry no buffer length, so a larger MTU must not
+	 * reach the MAC before the rings have been refilled.
+	 */
+	if (running)
+		ag71xx_stop(ndev);
 
 	WRITE_ONCE(ndev->mtu, new_mtu);
-	ag71xx_wr(ag, AG71XX_REG_MAC_MFL,
-		  ag71xx_max_frame_len(ndev->mtu));
+
+	if (running)
+		return ag71xx_open(ndev);
 
 	return 0;
 }
-- 
2.56.0

Re: [PATCH net] net: ag71xx: reallocate rx buffers when changing the MTU

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net] net: ag71xx: reallocate rx buffers when changing the MTU

[Rosen Penev]

On Sun, Oct 18, 2026 at 6:48 PM <netdev-bot+sinfo@kernel.org> wrote:
>
> Hi!
>
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
>
>  - How the issue was discovered, e.g. hit in production, hit during
>    development, syzbot report, manual code inspection, LLM or static
>    analysis tool scan.
>
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
Sashiko flagged this on a performance improvement for net-next.
>
>  - What hardware the change was tested on. For driver fixes please
>    mention the device (and if relevant firmware version) used for
>    testing, or say that the change was not tested on real hardware.
>
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
>
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.

Re: [PATCH net] net: ag71xx: reallocate rx buffers when changing the MTU

[Andrew Lunn]

On Sun, Oct 18, 2026 at 06:44:38PM -0700, Rosen Penev wrote:
> The rx buffers are sized from the MTU in ag71xx_open(), but
> ag71xx_change_mtu() writes the new maximum frame length straight to
> the MAC while the interface is running. The rx descriptors carry no
> buffer length, so after raising the MTU the MAC accepts and DMAs
> frames larger than the mapped buffer, overwriting the skb_shared_info
> at the end of the fragment and whatever follows it.
> 
> This is reachable without user action: DSA raises the MTU of a
> running conduit to make room for the switch tag.
> 
> Restart the interface when the MTU changes while it is up, so the
> rings are refilled with buffers sized for the new MTU before the MAC
> is programmed with the new frame length.
> 
> Fixes: d51b6ce441d3 ("net: ethernet: add ag71xx driver")
> Signed-off-by: Rosen Penev <rosenp@gmail.com>
> ---
>  drivers/net/ethernet/atheros/ag71xx.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/atheros/ag71xx.c b/drivers/net/ethernet/atheros/ag71xx.c
> index 8a3c689f6825..3c9de8c2ba64 100644
> --- a/drivers/net/ethernet/atheros/ag71xx.c
> +++ b/drivers/net/ethernet/atheros/ag71xx.c
> @@ -1761,11 +1761,19 @@ static irqreturn_t ag71xx_interrupt(int irq, void *dev_id)
>  
>  static int ag71xx_change_mtu(struct net_device *ndev, int new_mtu)
>  {
> -	struct ag71xx *ag = netdev_priv(ndev);
> +	bool running = netif_running(ndev);
> +
> +	/* The rx buffers are sized for the MTU at open time and the rx
> +	 * descriptors carry no buffer length, so a larger MTU must not
> +	 * reach the MAC before the rings have been refilled.
> +	 */
> +	if (running)
> +		ag71xx_stop(ndev);

This appears to free the rings.

>  
>  	WRITE_ONCE(ndev->mtu, new_mtu);
> -	ag71xx_wr(ag, AG71XX_REG_MAC_MFL,
> -		  ag71xx_max_frame_len(ndev->mtu));
> +
> +	if (running)
> +		return ag71xx_open(ndev);

And this tries to allocate them again. If this fails, there is no way
back, the interface is dead.

This is not how MTU change should work. If it fails, it should leave
the old MTU in place.

    Andrew

---
pw-bot: cr