netdev - Re: [PATCH RFC] r8169: straighten out overlength frame detection

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:	Mon, 28 Dec 2009 18:49:05 -0500
From:	Neil Horman <nhorman@...driver.com>
To:	François romieu <romieu@...eil.com>
Cc:	netdev@...r.kernel.org, davem@...emloft.net,
	eric.dumazet@...il.com, nhorman@...hat.com
Subject: Re: [PATCH RFC] r8169: straighten out overlength frame detection

On Mon, Dec 28, 2009 at 10:31:14PM +0100, François romieu wrote:
> (I'm back)
> 
> The Mon, Dec 28, 2009 at 02:50:53PM -0500, Neil Horman wrote :
> [...]
> > frames were received on NIC's supported by this driver.  This was mentioned in a
> > security conference recently:
> > http://events.ccc.de/congress/2009/Fahrplan//events/3596.en.html
> 
> Is there a paper ?
> 
> > It seems that if we can't enable frame size filtering, then, as Eric correctly
> > noticed, we can find ourselves DMA-ing too much data to a buffer, causing
> > corruption.  As a result is seems that we are forced to allocate a frame which
> > is ready to handle a maximally sized receive.
> 
> Either that or the switch does not allow jumbo frames.
> 
Possible, but regardless, if the result is that we either dma beyond the end of
an allocated skb, or get garbage in the frame size register, it seems we need to
support your initially referenced commit.  Again, I'm not 100% sure here, I'm
soliciting opinion, based on the above presentation.

> > I've not tested the below patch at all, and clearly it stinks to have to do.
> > But I thought it would be worth posting to solicit comments on it.
> [...]
> > diff --git a/drivers/net/r8169.c b/drivers/net/r8169.c
> > index 60f96c4..42e3b22 100644
> > --- a/drivers/net/r8169.c
> > +++ b/drivers/net/r8169.c
> > @@ -3972,7 +3973,7 @@ static struct sk_buff *rtl8169_alloc_rx_skb(struct pci_dev *pdev,
> >  
> >  	pad = align ? align : NET_IP_ALIGN;
> >  
> > -	skb = netdev_alloc_skb(dev, rx_buf_sz + pad);
> > +	skb = netdev_alloc_skb(dev, 16383 + pad);
> 
> I doubt that we will be able to allocate that much memory reliably for long.
> 
I agree, thats why I'm posting this with RFC, because it seems like a terrible
solution.  But at the same time, if we can't rely on the NIC to report frame
sizes reliably, I'm not sure what other choice we have.

> I'd rather go for static buffers + copy (+ src mac address of our new friend).
> 
Thats another choice, yes.  We could probably attain that easily by setting the
copybreak value to 16383 (so as to avoid changing the receive path too much if
there are classes of supported hw that do not encounter this problem).

> Is it enough if I write it in a pair of evening ?
> 
Yeah, thats fine by me, I was just responding to the presentation so we could
get the ball rolling.  If you like, I'd be happy to do it as well.  Just let me
know.


Thanks & regards
Neil

--
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