[regression] Kernel panic on resume from sleep

[regression] Kernel panic on resume from sleep

[Zbynek Michl]

Hello,

Can anybody help me with the following kernel issue?
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983595

Do I understand it correctly that the kernel crashes due to the bug in
the alx driver?

Thank you
Zbynek

Re: [regression] Kernel panic on resume from sleep

[Jakub Kicinski]

On Mon, 1 Mar 2021 23:11:05 +0100 Zbynek Michl wrote:
> Hello,
> 
> Can anybody help me with the following kernel issue?
> https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983595
> 
> Do I understand it correctly that the kernel crashes due to the bug in
> the alx driver?

Order of calls on resume looks suspicious, can you give this a try?

diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
index 9b7f1af5f574..9e02f8864593 100644
--- a/drivers/net/ethernet/atheros/alx/main.c
+++ b/drivers/net/ethernet/atheros/alx/main.c
@@ -1894,13 +1894,16 @@ static int alx_resume(struct device *dev)
 
        if (!netif_running(alx->dev))
                return 0;
-       netif_device_attach(alx->dev);
 
        rtnl_lock();
        err = __alx_open(alx, true);
        rtnl_unlock();
+       if (err)
+               return err;
 
-       return err;
+       netif_device_attach(alx->dev);
+
+       return 0;
 }
 
 static SIMPLE_DEV_PM_OPS(alx_pm_ops, alx_suspend, alx_resume);

Re: [regression] Kernel panic on resume from sleep

[Zbynek Michl]

Looks good so far, but need to wait some more time as the issue was irregular.

Do you have any explanation why the calls disorder caused the panic
just occasionally?

Also, the same (wrong) order I can see in the 3.16 kernel code, but it
has worked fine with this kernel in all cases. So what is different in
5.10?

Thanks
Zbynek

On Wed, Mar 3, 2021 at 2:44 AM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Mon, 1 Mar 2021 23:11:05 +0100 Zbynek Michl wrote:
> > Hello,
> >
> > Can anybody help me with the following kernel issue?
> > https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=983595
> >
> > Do I understand it correctly that the kernel crashes due to the bug in
> > the alx driver?
>
> Order of calls on resume looks suspicious, can you give this a try?
>
> diff --git a/drivers/net/ethernet/atheros/alx/main.c b/drivers/net/ethernet/atheros/alx/main.c
> index 9b7f1af5f574..9e02f8864593 100644
> --- a/drivers/net/ethernet/atheros/alx/main.c
> +++ b/drivers/net/ethernet/atheros/alx/main.c
> @@ -1894,13 +1894,16 @@ static int alx_resume(struct device *dev)
>
>         if (!netif_running(alx->dev))
>                 return 0;
> -       netif_device_attach(alx->dev);
>
>         rtnl_lock();
>         err = __alx_open(alx, true);
>         rtnl_unlock();
> +       if (err)
> +               return err;
>
> -       return err;
> +       netif_device_attach(alx->dev);
> +
> +       return 0;
>  }
>
>  static SIMPLE_DEV_PM_OPS(alx_pm_ops, alx_suspend, alx_resume);

Re: [regression] Kernel panic on resume from sleep

[Jakub Kicinski]

On Thu, 4 Mar 2021 13:50:01 +0100 Zbynek Michl wrote:
> Looks good so far, but need to wait some more time as the issue was irregular.
> 
> Do you have any explanation why the calls disorder caused the panic
> just occasionally?

Depends if kernel attempts to try to send a packet before __alx_open()
finishes. You can probably make it more likely by running trafgen, iperf
or such while suspending and resuming?

> Also, the same (wrong) order I can see in the 3.16 kernel code, but it
> has worked fine with this kernel in all cases. So what is different in
> 5.10?

At some point in between those versions the driver got modified to
allocate and free the NAPI structures dynamically.

I didn't look too closely to find out if things indeed worked 100%
correctly before, but now they will reliably crash on a NULL pointer
dereference if transmission comes before open is done.

Re: [regression] Kernel panic on resume from sleep

[Zbynek Michl]

On Thu, Mar 4, 2021 at 6:51 PM Jakub Kicinski <kuba@kernel.org> wrote:

> Depends if kernel attempts to try to send a packet before __alx_open()
> finishes. You can probably make it more likely by running trafgen, iperf
> or such while suspending and resuming?

I've tried "ping -f <GW>" first, but there was no effect - PC woke up
successfully.

Then I've tried TCP connection, like "wget -O /dev/null
https://cdn.kernel.org/pub/linux/kernel/v5.x/linux-5.11.3.tar.xz" and
this kills the kernel on resume reliably.

So perphaps ICMP and TCP behave differently?

Anyway, I'm happy to be able to finally enforce the kernel crash this way :)

> I didn't look too closely to find out if things indeed worked 100%
> correctly before, but now they will reliably crash on a NULL pointer
> dereference if transmission comes before open is done.

I can confirm that the fix really works and I cannot enforce the
kernel crash anymore.

So can we merge the patch to the mainline, and ideally also to the 5.10?

Thanks
Zbynek

Re: [regression] Kernel panic on resume from sleep

[Jakub Kicinski]

On Fri, 5 Mar 2021 13:50:28 +0100 Zbynek Michl wrote:
> On Thu, Mar 4, 2021 at 6:51 PM Jakub Kicinski <kuba@kernel.org> wrote:
> 
> > Depends if kernel attempts to try to send a packet before __alx_open()
> > finishes. You can probably make it more likely by running trafgen, iperf
> > or such while suspending and resuming?  
> 
> I've tried "ping -f <GW>" first, but there was no effect - PC woke up
> successfully.
> 
> Then I've tried TCP connection, like "wget -O /dev/null
> https://cdn.kernel.org/pub/linux/kernel/v5.x/linux-5.11.3.tar.xz" and
> this kills the kernel on resume reliably.
> 
> So perphaps ICMP and TCP behave differently?
> 
> Anyway, I'm happy to be able to finally enforce the kernel crash this way :)
> 
> > I didn't look too closely to find out if things indeed worked 100%
> > correctly before, but now they will reliably crash on a NULL pointer
> > dereference if transmission comes before open is done.  
> 
> I can confirm that the fix really works and I cannot enforce the
> kernel crash anymore.
> 
> So can we merge the patch to the mainline, and ideally also to the 5.10?

Great, I submitted the patch and marked it for stable inclusion!

It should percolate through the trees in a couple of weeks.

Re: [regression] Kernel panic on resume from sleep

[Zbynek Michl]

> Great, I submitted the patch and marked it for stable inclusion!
>
> It should percolate through the trees in a couple of weeks.

Cool. Jakub, thank you very much for your awesome assistance!

Zbynek