netdev - Re: [PATCH] skge: fix broken driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 19 Sep 2013 14:29:20 -0400 (EDT)
From:	Mikulas Patocka <mpatocka@...hat.com>
To:	Igor Gnatenko <i.gnatenko.brain@...il.com>
cc:	David Miller <davem@...emloft.net>, stephen@...workplumber.org,
	netdev@...r.kernel.org
Subject: Re: [PATCH] skge: fix broken driver



On Thu, 19 Sep 2013, Igor Gnatenko wrote:

> On Thu, 2013-09-19 at 14:04 -0400, Mikulas Patocka wrote:
> > 
> > 
> > On Thu, 19 Sep 2013, David Miller wrote:
> > 
> > > From: Mikulas Patocka <mpatocka@...hat.com>
> > > Date: Thu, 19 Sep 2013 12:33:30 -0400 (EDT)
> > > 
> > > > Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
> > > > Cc: stable@...nel.org # 3.11
> > > 
> > > First, this is missing the reported-by and tested-by tags provided
> > > by people who tested this patch.
> > 
> > I noticed the problem and tested the patch on my own machine. So I added 
> > myself to these tags.
> > 
> > > Secondly, CC:'ing stable is not the correct way to submit networking
> > > patches for -stable inclusion.  You simply ask me to queue them up
> > > for -stable explicitly instead.
> > > 
> > > Please re-submit this with the currect signoffs, thank you.
> > 
> > Here I'm re-submitting it.
> > 
> > ---
> > 
> > skge: fix broken driver
> > 
> > The patch 136d8f377e1575463b47840bc5f1b22d94bf8f63 broke the skge driver.
> > Note this part of the patch:
> > +               if (skge_rx_setup(skge, e, nskb, skge->rx_buf_size) < 0) {
> > +                       dev_kfree_skb(nskb);
> > +                       goto resubmit;
> > +               }
> > +
> >                 pci_unmap_single(skge->hw->pdev,
> >                                  dma_unmap_addr(e, mapaddr),
> >                                  dma_unmap_len(e, maplen),
> >                                  PCI_DMA_FROMDEVICE);
> >                 skb = e->skb;
> >                 prefetch(skb->data);
> > -               skge_rx_setup(skge, e, nskb, skge->rx_buf_size);
> > 
> > The function skge_rx_setup modifies e->skb to point to the new skb. Thus,
> > after this change, the new buffer, not the old, is returned to the
> > networking stack.
> > 
> > This bug is present in kernels 3.11, 3.11.1 and 3.12-rc1. The patch should 
> > be queued for 3.11-stable.
> > 
> > Signed-off-by: Mikulas Patocka <mpatocka@...hat.com>
> > Reported-by: Mikulas Patocka <mpatocka@...hat.com>
> > Tested-by: Mikulas Patocka <mpatocka@...hat.com>
> > 
> > ---
> >  drivers/net/ethernet/marvell/skge.c |    5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > Index: linux-3.11.1-fast/drivers/net/ethernet/marvell/skge.c
> > ===================================================================
> > --- linux-3.11.1-fast.orig/drivers/net/ethernet/marvell/skge.c  2013-09-10 19:46:58.000000000 +0200
> > +++ linux-3.11.1-fast/drivers/net/ethernet/marvell/skge.c       2013-09-19 18:20:43.000000000 +0200
> > @@ -3092,6 +3092,9 @@ static struct sk_buff *skge_rx_get(struc
> >                 if (!nskb)
> >                         goto resubmit;
> >  
> > +               skb = e->skb;
> > +               prefetch(skb->data);
> > +
> >                 if (skge_rx_setup(skge, e, nskb, skge->rx_buf_size) < 0) {
> >                         dev_kfree_skb(nskb);
> >                         goto resubmit;
> > @@ -3101,8 +3104,6 @@ static struct sk_buff *skge_rx_get(struc
> >                                  dma_unmap_addr(e, mapaddr),
> >                                  dma_unmap_len(e, maplen),
> >                                  PCI_DMA_FROMDEVICE);
> > -               skb = e->skb;
> > -               prefetch(skb->data);
> >         }
> >  
> >         skb_put(skb, len);
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@...r.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> Hey Mikulas! See https://lkml.org/lkml/2013/9/19/38 , plz.
> -- 
> Igor Gnatenko
> Fedora release 20 (Heisenbug)
> Linux 3.11.1-300.fc20.x86_64

I see. My patch is a bit simpler - it doesn't allocate the structure 
skge_element on the stack.

Mikulas
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html