* [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
@ 2015-10-24 22:42 Sergei Shtylyov
2015-10-26 22:52 ` Yasushi SHOJI
2015-10-30 9:02 ` David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Sergei Shtylyov @ 2015-10-24 22:42 UTC (permalink / raw)
To: netdev, yashi; +Cc: linux-sh
In a low memory situation the following kernel oops occurs:
Unable to handle kernel NULL pointer dereference at virtual address 00000050
pgd = 8490c000
[00000050] *pgd=4651e831, *pte=00000000, *ppte=00000000
Internal error: Oops: 17 [#1] PREEMPT ARM
Modules linked in:
CPU: 0 Not 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: 60000113
sp : 84eb1a90 ip : 84eb1ac8 fp : 84eb1ac4
r10: 0000003f r9 : 000005ea r8 : 00000000
r7 : 00000000 r6 : 940453b0 r5 : 00030000 r4 : 9381b180
r3 : 00000000 r2 : 00000000 r1 : 000005ea r0 : 00000000
Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
Control: 10c53c7d Table: 4248c059 DAC: 00000015
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 <yashi@atmark-techno.com>
Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
---
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);
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
2015-10-24 22:42 [PATCH RFT v2] sh_eth: fix kernel oops in skb_put() Sergei Shtylyov
@ 2015-10-26 22:52 ` Yasushi SHOJI
2015-11-19 17:53 ` Sergei Shtylyov
2015-10-30 9:02 ` David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Yasushi SHOJI @ 2015-10-26 22:52 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, linux-sh
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 00000050
> pgd = 8490c000
> [00000050] *pgd=4651e831, *pte=00000000, *ppte=00000000
> Internal error: Oops: 17 [#1] PREEMPT ARM
> Modules linked in:
> CPU: 0 Not 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: 60000113
> sp : 84eb1a90 ip : 84eb1ac8 fp : 84eb1ac4
> r10: 0000003f r9 : 000005ea r8 : 00000000
> r7 : 00000000 r6 : 940453b0 r5 : 00030000 r4 : 9381b180
> r3 : 00000000 r2 : 00000000 r1 : 000005ea r0 : 00000000
> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> Control: 10c53c7d Table: 4248c059 DAC: 00000015
> 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 <yashi@atmark-techno.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>
> ---
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
2015-10-26 22:52 ` Yasushi SHOJI
@ 2015-11-19 17:53 ` Sergei Shtylyov
2015-11-20 11:18 ` Yasushi SHOJI
0 siblings, 1 reply; 5+ messages in thread
From: Sergei Shtylyov @ 2015-11-19 17:53 UTC (permalink / raw)
To: Yasushi SHOJI; +Cc: netdev, linux-sh
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 00000050
>> pgd = 8490c000
>> [00000050] *pgd=4651e831, *pte=00000000, *ppte=00000000
>> Internal error: Oops: 17 [#1] PREEMPT ARM
>> Modules linked in:
>> CPU: 0 Not 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: 60000113
>> sp : 84eb1a90 ip : 84eb1ac8 fp : 84eb1ac4
>> r10: 0000003f r9 : 000005ea r8 : 00000000
>> r7 : 00000000 r6 : 940453b0 r5 : 00030000 r4 : 9381b180
>> r3 : 00000000 r2 : 00000000 r1 : 000005ea r0 : 00000000
>> Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>> Control: 10c53c7d Table: 4248c059 DAC: 00000015
>> 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 <yashi@atmark-techno.com>
>> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
>>
>> ---
>> 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
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
2015-11-19 17:53 ` Sergei Shtylyov
@ 2015-11-20 11:18 ` Yasushi SHOJI
0 siblings, 0 replies; 5+ messages in thread
From: Yasushi SHOJI @ 2015-11-20 11:18 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, linux-sh
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH RFT v2] sh_eth: fix kernel oops in skb_put()
2015-10-24 22:42 [PATCH RFT v2] sh_eth: fix kernel oops in skb_put() Sergei Shtylyov
2015-10-26 22:52 ` Yasushi SHOJI
@ 2015-10-30 9:02 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2015-10-30 9:02 UTC (permalink / raw)
To: sergei.shtylyov; +Cc: netdev, yashi, linux-sh
From: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Date: 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 <yashi@atmark-techno.com>
> Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2015-11-20 11:18 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-10-24 22:42 [PATCH RFT v2] sh_eth: fix kernel oops in skb_put() Sergei Shtylyov
2015-10-26 22:52 ` Yasushi SHOJI
2015-11-19 17:53 ` Sergei Shtylyov
2015-11-20 11:18 ` Yasushi SHOJI
2015-10-30 9:02 ` David Miller
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).