Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
Hi Sergei, On Fri, 20 Nov 2015 02:53:39 +0900, Sergei Shtylyov wrote: > >Shoji-san, can I push this patch to net.git? I doubt that it has > ill effects in itself -- the reason of the slowdown you're seeing > should be somewhere else... Sure. I've tested and the null access problem is gone for sure. I'm pretty sure that the fix won't break anything. It's going to take, however, some more time to pin down the slow down problem. I'll report when I find the cause. Thanks, -- yashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
Hello. On 10/27/2015 01:52 AM, Yasushi SHOJI wrote: In a low memory situation the following kernel oops occurs: Unable to handle kernel NULL pointer dereference at virtual address 0050 pgd = 8490c000 [0050] *pgd=4651e831, *pte=, *ppte= Internal error: Oops: 17 [#1] PREEMPT ARM Modules linked in: CPU: 0Not tainted (3.4-at16 #9) PC is at skb_put+0x10/0x98 LR is at sh_eth_poll+0x2c8/0xa10 pc : [<8035f780>]lr : [<8028bf50>]psr: 6113 sp : 84eb1a90 ip : 84eb1ac8 fp : 84eb1ac4 r10: 003f r9 : 05ea r8 : r7 : r6 : 940453b0 r5 : 0003 r4 : 9381b180 r3 : r2 : r1 : 05ea r0 : Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user Control: 10c53c7d Table: 4248c059 DAC: 0015 Process klogd (pid: 2046, stack limit = 0x84eb02e8) [...] This is because netdev_alloc_skb() fails and 'mdp->rx_skbuff[entry]' is left NULL but sh_eth_rx() later uses it without checking. Add such check... Reported-by: Yasushi SHOJISigned-off-by: Sergei Shtylyov --- This patch is against DaveM's 'net.git' repo. drivers/net/ethernet/renesas/sh_eth.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) Index: net/drivers/net/ethernet/renesas/sh_eth.c === --- net.orig/drivers/net/ethernet/renesas/sh_eth.c +++ net/drivers/net/ethernet/renesas/sh_eth.c @@ -1481,6 +1481,7 @@ static int sh_eth_rx(struct net_device * if (mdp->cd->shift_rd0) desc_status >>= 16; + skb = mdp->rx_skbuff[entry]; if (desc_status & (RD_RFS1 | RD_RFS2 | RD_RFS3 | RD_RFS4 | RD_RFS5 | RD_RFS6 | RD_RFS10)) { ndev->stats.rx_errors++; @@ -1496,12 +1497,11 @@ static int sh_eth_rx(struct net_device * ndev->stats.rx_missed_errors++; if (desc_status & RD_RFS10) ndev->stats.rx_over_errors++; - } else { + } else if (skb) { if (!mdp->cd->hw_swap) sh_eth_soft_swap( phys_to_virt(ALIGN(rxdesc->addr, 4)), pkt_len + 2); - skb = mdp->rx_skbuff[entry]; mdp->rx_skbuff[entry] = NULL; if (mdp->cd->rpadir) skb_reserve(skb, NET_IP_ALIGN); This certainly prevents from a bad access, however, some odd thing is going on. Once I hit a low memory situation with this patch, network thorough-put and response is very bad. telnet, ping, wget takes loong time. PING 172.16.2.13 (172.16.2.13) 56(84) bytes of data. 64 bytes from 172.16.2.13: icmp_seq=5 ttl=64 time=0.223 ms 64 bytes from 172.16.2.13: icmp_seq=6 ttl=64 time=0.195 ms 64 bytes from 172.16.2.13: icmp_seq=7 ttl=64 time=0.203 ms 64 bytes from 172.16.2.13: icmp_seq=8 ttl=64 time=0.219 ms 64 bytes from 172.16.2.13: icmp_seq=9 ttl=64 time=0.165 ms 64 bytes from 172.16.2.13: icmp_seq=10 ttl=64 time=0.171 ms 64 bytes from 172.16.2.13: icmp_seq=1 ttl=64 time=9023 ms 64 bytes from 172.16.2.13: icmp_seq=2 ttl=64 time=8022 ms 64 bytes from 172.16.2.13: icmp_seq=3 ttl=64 time=7014 ms 64 bytes from 172.16.2.13: icmp_seq=4 ttl=64 time=6006 ms I'll investigate it. Shoji-san, can I push this patch to net.git? I doubt that it has ill effects in itself -- the reason of the slowdown you're seeing should be somewhere else... MBR, Sergei -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
From: Sergei ShtylyovDate: Sun, 25 Oct 2015 01:42:33 +0300 > In a low memory situation the following kernel oops occurs: ... > This is because netdev_alloc_skb() fails and 'mdp->rx_skbuff[entry]' is left > NULL but sh_eth_rx() later uses it without checking. Add such check... > > Reported-by: Yasushi SHOJI > Signed-off-by: Sergei Shtylyov This seems to need more investigation and work, therefore I am not applying this at this time. When a final version is available (or even if this one is deemed appropriate as-is) please resubmit it withut RFT in the Subject. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
Hi Sergei, Thank you for your patch! On Sun, 25 Oct 2015 07:42:33 +0900, Sergei Shtylyov wrote: > > In a low memory situation the following kernel oops occurs: > > Unable to handle kernel NULL pointer dereference at virtual address 0050 > pgd = 8490c000 > [0050] *pgd=4651e831, *pte=, *ppte= > Internal error: Oops: 17 [#1] PREEMPT ARM > Modules linked in: > CPU: 0Not tainted (3.4-at16 #9) > PC is at skb_put+0x10/0x98 > LR is at sh_eth_poll+0x2c8/0xa10 > pc : [<8035f780>]lr : [<8028bf50>]psr: 6113 > sp : 84eb1a90 ip : 84eb1ac8 fp : 84eb1ac4 > r10: 003f r9 : 05ea r8 : > r7 : r6 : 940453b0 r5 : 0003 r4 : 9381b180 > r3 : r2 : r1 : 05ea r0 : > Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user > Control: 10c53c7d Table: 4248c059 DAC: 0015 > Process klogd (pid: 2046, stack limit = 0x84eb02e8) > [...] > > This is because netdev_alloc_skb() fails and 'mdp->rx_skbuff[entry]' is left > NULL but sh_eth_rx() later uses it without checking. Add such check... > > Reported-by: Yasushi SHOJI> Signed-off-by: Sergei Shtylyov > > --- > This patch is against DaveM's 'net.git' repo. > > drivers/net/ethernet/renesas/sh_eth.c |4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > Index: net/drivers/net/ethernet/renesas/sh_eth.c > === > --- net.orig/drivers/net/ethernet/renesas/sh_eth.c > +++ net/drivers/net/ethernet/renesas/sh_eth.c > @@ -1481,6 +1481,7 @@ static int sh_eth_rx(struct net_device * > if (mdp->cd->shift_rd0) > desc_status >>= 16; > > + skb = mdp->rx_skbuff[entry]; > if (desc_status & (RD_RFS1 | RD_RFS2 | RD_RFS3 | RD_RFS4 | > RD_RFS5 | RD_RFS6 | RD_RFS10)) { > ndev->stats.rx_errors++; > @@ -1496,12 +1497,11 @@ static int sh_eth_rx(struct net_device * > ndev->stats.rx_missed_errors++; > if (desc_status & RD_RFS10) > ndev->stats.rx_over_errors++; > - } else { > + } else if (skb) { > if (!mdp->cd->hw_swap) > sh_eth_soft_swap( > phys_to_virt(ALIGN(rxdesc->addr, 4)), > pkt_len + 2); > - skb = mdp->rx_skbuff[entry]; > mdp->rx_skbuff[entry] = NULL; > if (mdp->cd->rpadir) > skb_reserve(skb, NET_IP_ALIGN); > This certainly prevents from a bad access, however, some odd thing is going on. Once I hit a low memory situation with this patch, network thorough-put and response is very bad. telnet, ping, wget takes loong time. PING 172.16.2.13 (172.16.2.13) 56(84) bytes of data. 64 bytes from 172.16.2.13: icmp_seq=5 ttl=64 time=0.223 ms 64 bytes from 172.16.2.13: icmp_seq=6 ttl=64 time=0.195 ms 64 bytes from 172.16.2.13: icmp_seq=7 ttl=64 time=0.203 ms 64 bytes from 172.16.2.13: icmp_seq=8 ttl=64 time=0.219 ms 64 bytes from 172.16.2.13: icmp_seq=9 ttl=64 time=0.165 ms 64 bytes from 172.16.2.13: icmp_seq=10 ttl=64 time=0.171 ms 64 bytes from 172.16.2.13: icmp_seq=1 ttl=64 time=9023 ms 64 bytes from 172.16.2.13: icmp_seq=2 ttl=64 time=8022 ms 64 bytes from 172.16.2.13: icmp_seq=3 ttl=64 time=7014 ms 64 bytes from 172.16.2.13: icmp_seq=4 ttl=64 time=6006 ms I'll investigate it. -- yashi -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html