[PATCH net] net: b44: set pause params only when interface is up

[PATCH net] net: b44: set pause params only when interface is up

[Peter Münster]

[-- Attachment #1.1: Type: text/plain, Size: 151 bytes --]

Hi,

This patch fixes a kernel panic when using netifd.
Could you please apply it to linux-5.15.y?

TIA and kind regards,
-- 
           Peter

[-- Attachment #1.2: 0001-net-b44-set-pause-params-only-when-interface-is-up.patch --]
[-- Type: text/x-patch, Size: 1641 bytes --]

b44_free_rings() accesses b44::rx_buffers (and ::tx_buffers)
unconditionally, but b44::rx_buffers is only valid when the
device is up (they get allocated in b44_open(), and deallocated
again in b44_close()), any other time these is just a NULL pointers.

So if you try to change the pause params while the network interface
is disabled/administratively down, everything explodes (which likely
netifd tries to do).

Link: https://github.com/openwrt/openwrt/issues/13789
Reported-by: Peter Münster <pm@a16n.net>
Suggested-by: Jonas Gorski <jonas.gorski@gmail.com>
Signed-off-by: Vaclav Svoboda <svoboda@neng.cz>
Tested-by: Peter Münster <pm@a16n.net>
Signed-off-by: Peter Münster <pm@a16n.net>
---
 drivers/net/ethernet/broadcom/b44.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/broadcom/b44.c b/drivers/net/ethernet/broadcom/b44.c
index 485d32dda56f..ce370ef641f0 100644
--- a/drivers/net/ethernet/broadcom/b44.c
+++ b/drivers/net/ethernet/broadcom/b44.c
@@ -2029,12 +2029,14 @@ static int b44_set_pauseparam(struct net_device *dev,
 		bp->flags |= B44_FLAG_TX_PAUSE;
 	else
 		bp->flags &= ~B44_FLAG_TX_PAUSE;
-	if (bp->flags & B44_FLAG_PAUSE_AUTO) {
-		b44_halt(bp);
-		b44_init_rings(bp);
-		b44_init_hw(bp, B44_FULL_RESET);
-	} else {
-		__b44_set_flow_ctrl(bp, bp->flags);
+	if (netif_running(dev)) {
+		if (bp->flags & B44_FLAG_PAUSE_AUTO) {
+			b44_halt(bp);
+			b44_init_rings(bp);
+			b44_init_hw(bp, B44_FULL_RESET);
+		} else {
+			__b44_set_flow_ctrl(bp, bp->flags);
+		}
 	}
 	spin_unlock_irq(&bp->lock);
 
-- 
2.35.3


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: [PATCH net] net: b44: set pause params only when interface is up

[Andrew Lunn]

On Thu, Apr 18, 2024 at 04:12:34PM +0200, Peter Münster wrote:
> Hi,
> 
> This patch fixes a kernel panic when using netifd.
> Could you please apply it to linux-5.15.y?

Hi Peter

Please take a read of:

https://docs.kernel.org/process/submitting-patches.html

https://www.kernel.org/doc/html/latest/process/maintainer-netdev.html#netdev-faq

This is a networking fix, so please base the patch on net:

https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git

Please include a Fixed: tag indicating when the problem was added.

Also, add a Cc: stable@vger.kernel.org, which will cause the patch to
be back ported to stable trees, including linux-5.15.

You can use the script scripts/get_maintainer.pl to get a list of
Maintainers you should Cc: the patch to.

The code change itself looks reasonable, although Florian should
review it.

   Thanks
	Andrew

Re: [PATCH net] net: b44: set pause params only when interface is up

[Peter Münster]

[-- Attachment #1: Type: text/plain, Size: 558 bytes --]

On Thu, Apr 18 2024, Andrew Lunn wrote:

> Please include a Fixed: tag indicating when the problem was added.

Hi Andrew,

I’m sorry, I don’t know, when the problem was added. I only know, that
there was no problem with OpenWrt < 23.X. But I don’t know why. Perhaps
the behaviour of netifd has changed from 22.X to 23.X.

So I guess, that the problem is there since the creation of
b44_set_pauseparam(), but it has never been triggered before.

So what should I do please with the Fixed: tag?

TIA for any hints,
-- 
           Peter

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 211 bytes --]

Re: [PATCH net] net: b44: set pause params only when interface is up

[Michael Chan]

On Thu, Apr 18, 2024 at 1:27 PM Peter Münster <pm@a16n.net> wrote:
>
> On Thu, Apr 18 2024, Andrew Lunn wrote:
>
> > Please include a Fixed: tag indicating when the problem was added.
>
> Hi Andrew,
>
> I’m sorry, I don’t know, when the problem was added. I only know, that
> there was no problem with OpenWrt < 23.X. But I don’t know why. Perhaps
> the behaviour of netifd has changed from 22.X to 23.X.
>
> So I guess, that the problem is there since the creation of
> b44_set_pauseparam(), but it has never been triggered before.
>
> So what should I do please with the Fixed: tag?
>

It looks like this dates back to the beginning of git.  So I guess it should be:

Fixes: 1da177e4c3f4 (Linux-2.6.12-rc2)

Re: [PATCH net] net: b44: set pause params only when interface is up

[Andrew Lunn]

On Thu, Apr 18, 2024 at 01:40:00PM -0700, Michael Chan wrote:
> On Thu, Apr 18, 2024 at 1:27 PM Peter Münster <pm@a16n.net> wrote:
> >
> > On Thu, Apr 18 2024, Andrew Lunn wrote:
> >
> > > Please include a Fixed: tag indicating when the problem was added.
> >
> > Hi Andrew,
> >
> > I’m sorry, I don’t know, when the problem was added. I only know, that
> > there was no problem with OpenWrt < 23.X. But I don’t know why. Perhaps
> > the behaviour of netifd has changed from 22.X to 23.X.
> >
> > So I guess, that the problem is there since the creation of
> > b44_set_pauseparam(), but it has never been triggered before.
> >
> > So what should I do please with the Fixed: tag?
> >
> 
> It looks like this dates back to the beginning of git.  So I guess it should be:

Yes, i came to the same conclusion.

> Fixes: 1da177e4c3f4 (Linux-2.6.12-rc2)

Agreed.

	Andrew