Re: [Bugme-new] [Bug 9179] New: 2.6.23.1 / USB_ZD1201: Kernel panic with zd1201 driver

[Dan Williams <dcbw-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>]

On Wed, 2007-10-17 at 13:27 -0700, Andrew Morton wrote:
> On Wed, 17 Oct 2007 11:34:57 -0700 (PDT)
> bugme-daemon-590EEB7GvNiWaY/ihj7yzEB+6BGkLq7r@public.gmane.org wrote:
> 
> > http://bugzilla.kernel.org/show_bug.cgi?id=9179
> > 
> >            Summary: 2.6.23.1 / USB_ZD1201: Kernel panic with zd1201 driver
> >            Product: Drivers
> >            Version: 2.5
> >      KernelVersion: 2.6.23.1
> >           Platform: All
> >         OS/Version: Linux
> >               Tree: Mainline
> >             Status: NEW
> >           Severity: normal
> >           Priority: P1
> >          Component: network-wireless
> >         AssignedTo: drivers_network-wireless-ztI5WcYan/vQLgFONoPN62D2FQJk+8+b@public.gmane.org
> >         ReportedBy: zairasai-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org
> > 
> > 
> > [1.] One line summary of the problem:
> > 
> > 2.6.23.1 / USB_ZD1201: Kernel panic with zd1201 driver
> > 
> > 
> > 
> > 
> > [2.] Full description of the problem:
> > 
> > The zd1201-driver (symbol: USB_ZD1201) triggers a kernel panic during
> > initialization of the WLAN device, showing the following message:
> > 
> > EIP: [<e095e1d1>] zd1201_usbrx+0x6e1/0xbb0 [zd1201] SS:ESP 0068:c0469d7c
> > Kernel panic - not syncing: Fatal exception in interrupt
> > 
> > According to the init output during bootup, the panic seems to occur right when
> > the WLAN device receives an IP address from the DHCP-Server of the
> > WLAN/DSL-Router. The WLAN device is (in my case) a 'Belkin F5D6051' based on
> > the ZyDAS 1201 chip.
> > 
> > As far as i know, the only recent change in 'drivers/net/wireless/zd1201.c' was
> > done in patch-2.6.22, so the bug probably affects all kernel versions later
> > than 2.6.21.7, but at least the ones i've tested (which are listed in the
> > summary below). It also recently came up in some different
> > distribution-specific forums/bugtrackers, so it does not seem to be specific to
> > my machine/setup. A link to another report on this problem is included at the
> > end of this report.
> > 
> > Below is an extract of patch-2.6.22, showing that the lines 330 and 388 have
> > been removed from 'drivers/net/wireless/zd1201.c'. I put those two lines back,
> > which made things work as expected again; however, that is only meant as a
> > hint, since i don't know why they were taken out or what other implications my
> > change might have.
> > 
> > patch-2.6.22, lines 586509-586528:
> > {{{
> > diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
> > index 6cb66a3..935b144 100644
> > --- a/drivers/net/wireless/zd1201.c
> > +++ b/drivers/net/wireless/zd1201.c
> > @@ -327,7 +327,6 @@ static void zd1201_usbrx(struct urb *urb)
> >                         memcpy(skb_put(skb, 6), &data[datalen-8], 6);
> >                         memcpy(skb_put(skb, 2), &data[datalen-24], 2);
> >                         memcpy(skb_put(skb, len), data, len);
> > -                       skb->dev = zd->dev;
> >                         skb->dev->last_rx = jiffies;
> >                         skb->protocol = eth_type_trans(skb, zd->dev);
> >                         zd->stats.rx_packets++;
> > @@ -385,7 +384,6 @@ static void zd1201_usbrx(struct urb *urb)
> >                         memcpy(skb_put(skb, 2), &data[6], 2);
> >                         memcpy(skb_put(skb, len), data+8, len);
> >                 }
> > -               skb->dev = zd->dev;
> >                 skb->dev->last_rx = jiffies;
> >                 skb->protocol = eth_type_trans(skb, zd->dev);
> >                 zd->stats.rx_packets++;
> > }}}
> > 
> 
> Arnaldo, we have a pretty solid report here that your
> 4c13eb6657fe9ef7b4dc8f1a405c902e9e5234e0 made this driver go crash.

In 2.6.22 and later, eth_type_trans() sets skb->dev.  It looks like the
lines tha tset last_rx in the patch above should be moved below the
eth_type_trans() lines, otherwise they'll likely oops.

Something like this is probably in order?

diff --git a/drivers/net/wireless/zd1201.c b/drivers/net/wireless/zd1201.c
index 6cb66a3..935b144 100644
--- a/drivers/net/wireless/zd1201.c
+++ b/drivers/net/wireless/zd1201.c
@@ -327,7 +327,6 @@ static void zd1201_usbrx(struct urb *urb)
                        memcpy(skb_put(skb, 6), &data[datalen-8], 6);
                        memcpy(skb_put(skb, 2), &data[datalen-24], 2);
                        memcpy(skb_put(skb, len), data, len);
-                       skb->dev = zd->dev;
-                       skb->dev->last_rx = jiffies;
                        skb->protocol = eth_type_trans(skb, zd->dev);
+                       skb->dev->last_rx = jiffies;
                        zd->stats.rx_packets++;
@@ -385,7 +384,6 @@ static void zd1201_usbrx(struct urb *urb)
                        memcpy(skb_put(skb, 2), &data[6], 2);
                        memcpy(skb_put(skb, len), data+8, len);
                }
-               skb->dev = zd->dev;
-               skb->dev->last_rx = jiffies;
                skb->protocol = eth_type_trans(skb, zd->dev);
+               skb->dev->last_rx = jiffies;
                zd->stats.rx_packets++;

Dan