* Probably a flaw in Linux rtl8139 driver FYI [not found] ` <528f811a0904170010w2b2c8b34j698b255331f78765@mail.gmail.com> @ 2009-04-18 12:40 ` Tzungder Lin 2009-04-18 13:01 ` Eric Dumazet 0 siblings, 1 reply; 5+ messages in thread From: Tzungder Lin @ 2009-04-18 12:40 UTC (permalink / raw) To: netdev, davem Dear Sirs, Hello, I am jon from Taiwan. First I want to thank you for your great contributions of open source. Thank you for your 8139 driver to make our world better. Here is what happens: While I am debugging our Embedded Linux SoC I found a flaw in Linux 8139 driver (8139too.c) ,probably. If I attack (interval < 10ms) the driver with broadcast packets (mac.destaddr == ff:ff:ff:ff:ff:ff) when network interface up (ifconfig eth0 up) at the first time, the kernel space memory will be corrupted (overwrited with packet content) start from 0xc03e8800. Then kernel panics. Here is what I discovered: While ifconfig eth0 up kernel calls open() of 8139 driver(8139too.c). In rtl8139_hw_start() of rtl8139_open(), 8139 driver enable RX before setting up the DMA buffer address. Therefore in this interval where RX was enabled and DMA buffer address is not yet set up, any incoming broadcast packet would be send to a strange physical address: 0x003e8800 which is the default value of DMA buffer address. Unfortunately, this address is in Linux kernel used by kmem_cache functions. So, kernel panics. I have tried to fix the driver by setting up DMA buffer address before RX enabled and everything is fine. I have checked 8139too.c in both 2.4.x kernel and 2.6.x kernel, they both have the same initial flow. Here is a simple patch to show you what I found. --- 8139too.c 2007-12-13 15:54:26.000000000 +0800 +++ 8139too_patch.c 2009-04-17 14:56:27.000000000 +0800 @@ -1382,6 +1382,10 @@ RTL_W32_F (MAC0 + 0, cpu_to_le32 (*(u32 *) (dev->dev_addr + 0))); RTL_W32_F (MAC0 + 4, cpu_to_le32 (*(u32 *) (dev->dev_addr + 4))); + /* init Rx ring buffer DMA address */ + /* init before Rx enabled to avoid broadcast packet attack in this interval */ + RTL_W32_F (RxBuf, tp->rx_ring_dma); + /* Must enable Tx/Rx before setting transfer thresholds! */ RTL_W8 (ChipCmd, CmdRxEnb | CmdTxEnb); @@ -1405,8 +1409,6 @@ /* Lock Config[01234] and BMCR register writes */ RTL_W8 (Cfg9346, Cfg9346_Lock); - /* init Rx ring buffer DMA address */ - RTL_W32_F (RxBuf, tp->rx_ring_dma); /* init Tx buffer DMA addresses */ for (i = 0; i < NUM_TX_DESC; i++) Hope this can make the driver more robust. FYR Thanks a lot! Regards Jonathan Lin @Taiwan 2009.4.18 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Probably a flaw in Linux rtl8139 driver FYI 2009-04-18 12:40 ` Probably a flaw in Linux rtl8139 driver FYI Tzungder Lin @ 2009-04-18 13:01 ` Eric Dumazet 2009-04-18 15:28 ` Tzungder Lin 0 siblings, 1 reply; 5+ messages in thread From: Eric Dumazet @ 2009-04-18 13:01 UTC (permalink / raw) To: Tzungder Lin; +Cc: netdev, davem Tzungder Lin a écrit : > Dear Sirs, > > Hello, I am jon from Taiwan. > First I want to thank you for your great contributions of open source. > Thank you for your 8139 driver to make our world better. > > Here is what happens: > While I am debugging our Embedded Linux SoC I found a flaw in Linux > 8139 driver (8139too.c) ,probably. > If I attack (interval < 10ms) the driver with broadcast packets > (mac.destaddr == ff:ff:ff:ff:ff:ff) when network interface up (ifconfig > eth0 up) at the first time, the kernel space memory will be corrupted > (overwrited with packet content) start from 0xc03e8800. > Then kernel panics. > > Here is what I discovered: > While ifconfig eth0 up kernel calls open() of 8139 driver(8139too.c). > In rtl8139_hw_start() of rtl8139_open(), 8139 driver enable RX before > setting up the DMA buffer > address. > Therefore in this interval where RX was enabled and DMA buffer address > is not yet set up, any incoming broadcast packet would be send to a strange > physical address: 0x003e8800 which is the default value of DMA buffer address. > Unfortunately, this address is in Linux kernel used by kmem_cache functions. > So, kernel panics. I have tried to fix the driver by setting up DMA > buffer address > before RX enabled and everything is fine. > > I have checked 8139too.c in both 2.4.x kernel and 2.6.x kernel, they > both have the same initial flow. > Here is a simple patch to show you what I found. > > --- 8139too.c 2007-12-13 15:54:26.000000000 +0800 > +++ 8139too_patch.c 2009-04-17 14:56:27.000000000 +0800 > @@ -1382,6 +1382,10 @@ > RTL_W32_F (MAC0 + 0, cpu_to_le32 (*(u32 *) (dev->dev_addr + 0))); > RTL_W32_F (MAC0 + 4, cpu_to_le32 (*(u32 *) (dev->dev_addr + 4))); > > + /* init Rx ring buffer DMA address */ > + /* init before Rx enabled to avoid broadcast packet attack in > this interval */ > + RTL_W32_F (RxBuf, tp->rx_ring_dma); > + > /* Must enable Tx/Rx before setting transfer thresholds! */ > RTL_W8 (ChipCmd, CmdRxEnb | CmdTxEnb); > > @@ -1405,8 +1409,6 @@ > /* Lock Config[01234] and BMCR register writes */ > RTL_W8 (Cfg9346, Cfg9346_Lock); > > - /* init Rx ring buffer DMA address */ > - RTL_W32_F (RxBuf, tp->rx_ring_dma); > > /* init Tx buffer DMA addresses */ > for (i = 0; i < NUM_TX_DESC; i++) > > Hope this can make the driver more robust. > FYR > Thanks a lot! > > Regards > Jonathan Lin @Taiwan > 2009.4.18 Hello Jonathan This seems a nice catch ! What about also initializing tp->cur_rx = 0 *before* enabling RX too ? So after patch we should have : tp->cur_rx = 0; RTL_W32_F (RxBuf, tp->rx_ring_dma); /* Must enable Tx/Rx before setting transfer thresholds! */ RTL_W8 (ChipCmd, CmdRxEnb | CmdTxEnb); Everything should be setup before enable RX interrupts coming... ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Probably a flaw in Linux rtl8139 driver FYI 2009-04-18 13:01 ` Eric Dumazet @ 2009-04-18 15:28 ` Tzungder Lin 2009-04-18 19:43 ` Eric Dumazet 0 siblings, 1 reply; 5+ messages in thread From: Tzungder Lin @ 2009-04-18 15:28 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, davem Dear Eric, I agree with you at first. But after I checked the driver code, I think tp->cur_rx has been set to zero in rtl8139_init_ring() which has been executed before rtl8139_hw_start(). So, it should be safe already. Thanks for your advice still. FYR Regards jonathan On Sat, Apr 18, 2009 at 9:01 PM, Eric Dumazet <dada1@cosmosbay.com> wrote: > Tzungder Lin a écrit : >> Dear Sirs, >> >> Hello, I am jon from Taiwan. >> First I want to thank you for your great contributions of open source. >> Thank you for your 8139 driver to make our world better. >> >> Here is what happens: >> While I am debugging our Embedded Linux SoC I found a flaw in Linux >> 8139 driver (8139too.c) ,probably. >> If I attack (interval < 10ms) the driver with broadcast packets >> (mac.destaddr == ff:ff:ff:ff:ff:ff) when network interface up (ifconfig >> eth0 up) at the first time, the kernel space memory will be corrupted >> (overwrited with packet content) start from 0xc03e8800. >> Then kernel panics. >> >> Here is what I discovered: >> While ifconfig eth0 up kernel calls open() of 8139 driver(8139too.c). >> In rtl8139_hw_start() of rtl8139_open(), 8139 driver enable RX before >> setting up the DMA buffer >> address. >> Therefore in this interval where RX was enabled and DMA buffer address >> is not yet set up, any incoming broadcast packet would be send to a strange >> physical address: 0x003e8800 which is the default value of DMA buffer address. >> Unfortunately, this address is in Linux kernel used by kmem_cache functions. >> So, kernel panics. I have tried to fix the driver by setting up DMA >> buffer address >> before RX enabled and everything is fine. >> >> I have checked 8139too.c in both 2.4.x kernel and 2.6.x kernel, they >> both have the same initial flow. >> Here is a simple patch to show you what I found. >> >> --- 8139too.c 2007-12-13 15:54:26.000000000 +0800 >> +++ 8139too_patch.c 2009-04-17 14:56:27.000000000 +0800 >> @@ -1382,6 +1382,10 @@ >> RTL_W32_F (MAC0 + 0, cpu_to_le32 (*(u32 *) (dev->dev_addr + 0))); >> RTL_W32_F (MAC0 + 4, cpu_to_le32 (*(u32 *) (dev->dev_addr + 4))); >> >> + /* init Rx ring buffer DMA address */ >> + /* init before Rx enabled to avoid broadcast packet attack in >> this interval */ >> + RTL_W32_F (RxBuf, tp->rx_ring_dma); >> + >> /* Must enable Tx/Rx before setting transfer thresholds! */ >> RTL_W8 (ChipCmd, CmdRxEnb | CmdTxEnb); >> >> @@ -1405,8 +1409,6 @@ >> /* Lock Config[01234] and BMCR register writes */ >> RTL_W8 (Cfg9346, Cfg9346_Lock); >> >> - /* init Rx ring buffer DMA address */ >> - RTL_W32_F (RxBuf, tp->rx_ring_dma); >> >> /* init Tx buffer DMA addresses */ >> for (i = 0; i < NUM_TX_DESC; i++) >> >> Hope this can make the driver more robust. >> FYR >> Thanks a lot! >> >> Regards >> Jonathan Lin @Taiwan >> 2009.4.18 > > Hello Jonathan > > This seems a nice catch ! > > What about also initializing tp->cur_rx = 0 *before* enabling RX too ? > > So after patch we should have : > > tp->cur_rx = 0; > RTL_W32_F (RxBuf, tp->rx_ring_dma); > /* Must enable Tx/Rx before setting transfer thresholds! */ > RTL_W8 (ChipCmd, CmdRxEnb | CmdTxEnb); > > Everything should be setup before enable RX interrupts coming... > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Probably a flaw in Linux rtl8139 driver FYI 2009-04-18 15:28 ` Tzungder Lin @ 2009-04-18 19:43 ` Eric Dumazet 2009-04-19 6:52 ` Tzungder Lin 0 siblings, 1 reply; 5+ messages in thread From: Eric Dumazet @ 2009-04-18 19:43 UTC (permalink / raw) To: Tzungder Lin; +Cc: netdev, davem Tzungder Lin a écrit : > Dear Eric, > > I agree with you at first. > But after I checked the driver code, I think tp->cur_rx has been set > to zero in rtl8139_init_ring() which has been executed before > rtl8139_hw_start(). > So, it should be safe already. > Thanks for your advice still. > FYR Very good ! Could you formally post the patch with your Signoff, and make it applicable with "patch -p1" ? (Please read Documentation/SubmittingPatches if you need some info) Thanks ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: Probably a flaw in Linux rtl8139 driver FYI 2009-04-18 19:43 ` Eric Dumazet @ 2009-04-19 6:52 ` Tzungder Lin 0 siblings, 0 replies; 5+ messages in thread From: Tzungder Lin @ 2009-04-19 6:52 UTC (permalink / raw) To: Eric Dumazet; +Cc: netdev, davem Dear Sirs, Thank you very much. I will follow it tonight as soon as possible. Regards jonathan On 4/19/09, Eric Dumazet <dada1@cosmosbay.com> wrote: > Tzungder Lin a écrit : > > Dear Eric, > > > > I agree with you at first. > > But after I checked the driver code, I think tp->cur_rx has been set > > to zero in rtl8139_init_ring() which has been executed before > > rtl8139_hw_start(). > > So, it should be safe already. > > Thanks for your advice still. > > FYR > > Very good ! Could you formally post the patch with your Signoff, > and make it applicable with "patch -p1" ? > > (Please read Documentation/SubmittingPatches if you need some info) > > Thanks > > > ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-04-19 6:52 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <528f811a0904170008q34b5cde1l810fb22f78eefaf7@mail.gmail.com>
[not found] ` <528f811a0904170010w2b2c8b34j698b255331f78765@mail.gmail.com>
2009-04-18 12:40 ` Probably a flaw in Linux rtl8139 driver FYI Tzungder Lin
2009-04-18 13:01 ` Eric Dumazet
2009-04-18 15:28 ` Tzungder Lin
2009-04-18 19:43 ` Eric Dumazet
2009-04-19 6:52 ` Tzungder Lin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).