* [BUG net-next-2.6] dst_release() crash
@ 2011-03-03 16:28 Eric Dumazet
2011-03-03 17:07 ` [PATCH net-next-2.6] udp: udp_sendmsg() fix Eric Dumazet
2011-03-03 18:38 ` [BUG net-next-2.6] dst_release() crash David Miller
0 siblings, 2 replies; 5+ messages in thread
From: Eric Dumazet @ 2011-03-03 16:28 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Just had this while working on latest net-next-2.6
[11483.697210] BUG: unable to handle kernel NULL pointer dereference at
0000002a
[11483.697233] IP: [<c12b0638>] dst_release+0x18/0x60
[11483.697252] *pdpt = 0000000034917001 *pde = 0000000000000000
[11483.697270] Oops: 0002 [#1] SMP
[11483.697283] last sysfs
file: /sys/devices/virtual/net/bond0/bonding/active_slave
[11483.697299] Modules linked in: pktgen ipmi_si ipmi_msghandler hpilo
tg3 bonding ipv6
[11483.697335]
[11483.697340] Pid: 5490, comm: ntpd Not tainted
2.6.38-rc5-02884-g0853e6c-dirty #974 HP ProLiant BL460c G1
[11483.697366] EIP: 0060:[<c12b0638>] EFLAGS: 00010282 CPU: 4
[11483.697392] EIP is at dst_release+0x18/0x60
[11483.697416] EAX: ffffffea EBX: ffffffea ECX: 00000000 EDX: 6e14a8c0
[11483.697444] ESI: ffffffff EDI: ffffffea EBP: f445fd18 ESP: f445fd10
[11483.697471] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
[11483.697497] Process ntpd (pid: 5490, ti=f445e000 task=f3542490
task.ti=f445e000)
[11483.697540] Stack:
[11483.697562] f3401400 ffffffea f445fdc0 c12fc9d2 00000030 f445fd3c
c12a1c01 00000282
[11483.697619] 00000008 00000030 0dfeaa0a 6e14a8c0 7b001000 00000000
0dfeaa0a f445fda0
[11483.697680] 00000000 00000000 c12dbd90 00000000 f445ff38 00000000
00000000 00000000
[11483.697741] Call Trace:
[11483.697764] [<c12fc9d2>] udp_sendmsg+0x282/0x6e0
[11483.697790] [<c12a1c01>] ? memcpy_toiovec+0x51/0x70
[11483.697818] [<c12dbd90>] ? ip_generic_getfrag+0x0/0xb0
[11483.697848] [<c1184e00>] ? timerqueue_add+0x30/0xc0
[11483.697874] [<c1304ae5>] inet_sendmsg+0x45/0xa0
[11483.697901] [<c10617ab>] ? __hrtimer_start_range_ns+0x1ab/0x500
[11483.697930] [<c1298543>] sock_sendmsg+0xc3/0xf0
[11483.697957] [<c100b2b4>] ? save_i387_fxsave+0x74/0x80
[11483.697984] [<c1189022>] ? _copy_from_user+0x32/0x50
[11483.698010] [<c1299702>] sys_sendto+0xb2/0xe0
[11483.698035] [<c100bd50>] ? restore_i387_xstate+0xd0/0x1f0
[11483.698062] [<c129a07d>] sys_socketcall+0x18d/0x270
[11483.698088] [<c1002cd0>] sysenter_do_call+0x12/0x26
[11483.698123] Code: 53 c1 e8 ac 0b 0a 00 5b c9 c3 89 f6 8d bc 27 00 00
00 00 55 89 e5 83 ec 08 85 c0 89 1c 24 89 74 24 04 89 c3 74 11 be ff ff
ff ff <f0> 0f c1 70 40 4e 78 2f 85 f6 74 0c 8b 1c 24 8b 74 24 04 c9 c3
[11483.698319] EIP: [<c12b0638>] dst_release+0x18/0x60 SS:ESP
0068:f445fd10
[11483.698350] CR2: 000000000000002a
[11483.698594] ---[ end trace cb654a6638801e35 ]---
crash in
c12b0638: f0 0f c1 70 40 lock xadd %esi,0x40(%eax)
EAX = -EINVAL
commit b23dd4fe42b455af (ipv4: Make output route lookup return rtable
directly) introduced a regression ...
We could add a sanity test in dst_release(), or fix callers ?
What do you think ?
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH net-next-2.6] udp: udp_sendmsg() fix
2011-03-03 16:28 [BUG net-next-2.6] dst_release() crash Eric Dumazet
@ 2011-03-03 17:07 ` Eric Dumazet
2011-03-03 18:42 ` David Miller
2011-03-03 18:38 ` [BUG net-next-2.6] dst_release() crash David Miller
1 sibling, 1 reply; 5+ messages in thread
From: Eric Dumazet @ 2011-03-03 17:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Le jeudi 03 mars 2011 à 17:28 +0100, Eric Dumazet a écrit :
> Just had this while working on latest net-next-2.6
>
> [11483.697210] BUG: unable to handle kernel NULL pointer dereference at
> 0000002a
> [11483.697233] IP: [<c12b0638>] dst_release+0x18/0x60
> [11483.697252] *pdpt = 0000000034917001 *pde = 0000000000000000
> [11483.697270] Oops: 0002 [#1] SMP
> [11483.697283] last sysfs
> file: /sys/devices/virtual/net/bond0/bonding/active_slave
> [11483.697299] Modules linked in: pktgen ipmi_si ipmi_msghandler hpilo
> tg3 bonding ipv6
> [11483.697335]
> [11483.697340] Pid: 5490, comm: ntpd Not tainted
> 2.6.38-rc5-02884-g0853e6c-dirty #974 HP ProLiant BL460c G1
> [11483.697366] EIP: 0060:[<c12b0638>] EFLAGS: 00010282 CPU: 4
> [11483.697392] EIP is at dst_release+0x18/0x60
> [11483.697416] EAX: ffffffea EBX: ffffffea ECX: 00000000 EDX: 6e14a8c0
> [11483.697444] ESI: ffffffff EDI: ffffffea EBP: f445fd18 ESP: f445fd10
> [11483.697471] DS: 007b ES: 007b FS: 00d8 GS: 0033 SS: 0068
> [11483.697497] Process ntpd (pid: 5490, ti=f445e000 task=f3542490
> task.ti=f445e000)
> [11483.697540] Stack:
> [11483.697562] f3401400 ffffffea f445fdc0 c12fc9d2 00000030 f445fd3c
> c12a1c01 00000282
> [11483.697619] 00000008 00000030 0dfeaa0a 6e14a8c0 7b001000 00000000
> 0dfeaa0a f445fda0
> [11483.697680] 00000000 00000000 c12dbd90 00000000 f445ff38 00000000
> 00000000 00000000
> [11483.697741] Call Trace:
> [11483.697764] [<c12fc9d2>] udp_sendmsg+0x282/0x6e0
> [11483.697790] [<c12a1c01>] ? memcpy_toiovec+0x51/0x70
> [11483.697818] [<c12dbd90>] ? ip_generic_getfrag+0x0/0xb0
> [11483.697848] [<c1184e00>] ? timerqueue_add+0x30/0xc0
> [11483.697874] [<c1304ae5>] inet_sendmsg+0x45/0xa0
> [11483.697901] [<c10617ab>] ? __hrtimer_start_range_ns+0x1ab/0x500
> [11483.697930] [<c1298543>] sock_sendmsg+0xc3/0xf0
> [11483.697957] [<c100b2b4>] ? save_i387_fxsave+0x74/0x80
> [11483.697984] [<c1189022>] ? _copy_from_user+0x32/0x50
> [11483.698010] [<c1299702>] sys_sendto+0xb2/0xe0
> [11483.698035] [<c100bd50>] ? restore_i387_xstate+0xd0/0x1f0
> [11483.698062] [<c129a07d>] sys_socketcall+0x18d/0x270
> [11483.698088] [<c1002cd0>] sysenter_do_call+0x12/0x26
> [11483.698123] Code: 53 c1 e8 ac 0b 0a 00 5b c9 c3 89 f6 8d bc 27 00 00
> 00 00 55 89 e5 83 ec 08 85 c0 89 1c 24 89 74 24 04 89 c3 74 11 be ff ff
> ff ff <f0> 0f c1 70 40 4e 78 2f 85 f6 74 0c 8b 1c 24 8b 74 24 04 c9 c3
> [11483.698319] EIP: [<c12b0638>] dst_release+0x18/0x60 SS:ESP
> 0068:f445fd10
> [11483.698350] CR2: 000000000000002a
> [11483.698594] ---[ end trace cb654a6638801e35 ]---
>
>
> crash in
>
> c12b0638: f0 0f c1 70 40 lock xadd %esi,0x40(%eax)
>
> EAX = -EINVAL
>
>
>
> commit b23dd4fe42b455af (ipv4: Make output route lookup return rtable
> directly) introduced a regression ...
>
> We could add a sanity test in dst_release(), or fix callers ?
>
> What do you think ?
>
>
Here is the patch I cooked after an audit
[PATCH net-next-2.6] udp: udp_sendmsg() fix
commit b23dd4fe42b455af (ipv4: Make output route lookup return rtable
directly) introduced a regression in udp_sendmsg()
If IP route lookup fails, we should not try to release an non existent
route.
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 95e0c2c..f6c0c00 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -927,7 +927,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
err = PTR_ERR(rt);
if (err == -ENETUNREACH)
IP_INC_STATS_BH(net, IPSTATS_MIB_OUTNOROUTES);
- goto out;
+ goto out2;
}
err = -EACCES;
@@ -991,6 +991,7 @@ do_append_data:
out:
ip_rt_put(rt);
+out2:
if (free)
kfree(ipc.opt);
if (!err)
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH net-next-2.6] udp: udp_sendmsg() fix
2011-03-03 17:07 ` [PATCH net-next-2.6] udp: udp_sendmsg() fix Eric Dumazet
@ 2011-03-03 18:42 ` David Miller
2011-03-03 19:02 ` Eric Dumazet
0 siblings, 1 reply; 5+ messages in thread
From: David Miller @ 2011-03-03 18:42 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 03 Mar 2011 18:07:23 +0100
> [PATCH net-next-2.6] udp: udp_sendmsg() fix
>
> commit b23dd4fe42b455af (ipv4: Make output route lookup return rtable
> directly) introduced a regression in udp_sendmsg()
>
> If IP route lookup fails, we should not try to release an non existent
> route.
>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Ok, this works too.
My version NULL's out rt, so that if we had any changes in code paths
here it would be unlikely we'd reintroduce this bug.
I'm fine either way, although my version is in net-next-2.6 at the
moment :-)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH net-next-2.6] udp: udp_sendmsg() fix
2011-03-03 18:42 ` David Miller
@ 2011-03-03 19:02 ` Eric Dumazet
0 siblings, 0 replies; 5+ messages in thread
From: Eric Dumazet @ 2011-03-03 19:02 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Le jeudi 03 mars 2011 à 10:42 -0800, David Miller a écrit :
> My version NULL's out rt, so that if we had any changes in code paths
> here it would be unlikely we'd reintroduce this bug.
>
> I'm fine either way, although my version is in net-next-2.6 at the
> moment :-)
No problem, your version pleases me as well ;)
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [BUG net-next-2.6] dst_release() crash
2011-03-03 16:28 [BUG net-next-2.6] dst_release() crash Eric Dumazet
2011-03-03 17:07 ` [PATCH net-next-2.6] udp: udp_sendmsg() fix Eric Dumazet
@ 2011-03-03 18:38 ` David Miller
1 sibling, 0 replies; 5+ messages in thread
From: David Miller @ 2011-03-03 18:38 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 03 Mar 2011 17:28:06 +0100
> We could add a sanity test in dst_release(), or fix callers ?
Callers are responsible for not passing garbage pointers around
:-)
Thanks for the report.
I just checked in the following fix.
--------------------
ipv4: Fix crash in dst_release when udp_sendmsg route lookup fails.
As reported by Eric:
[11483.697233] IP: [<c12b0638>] dst_release+0x18/0x60
...
[11483.697741] Call Trace:
[11483.697764] [<c12fc9d2>] udp_sendmsg+0x282/0x6e0
[11483.697790] [<c12a1c01>] ? memcpy_toiovec+0x51/0x70
[11483.697818] [<c12dbd90>] ? ip_generic_getfrag+0x0/0xb0
The pointer passed to dst_release() is -EINVAL, that's because
we leave an error pointer in the local variable "rt" by accident.
NULL it out to fix the bug.
Reported-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
net/ipv4/udp.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
index 95e0c2c..c9a73e5 100644
--- a/net/ipv4/udp.c
+++ b/net/ipv4/udp.c
@@ -925,6 +925,7 @@ int udp_sendmsg(struct kiocb *iocb, struct sock *sk, struct msghdr *msg,
rt = ip_route_output_flow(net, &fl, sk);
if (IS_ERR(rt)) {
err = PTR_ERR(rt);
+ rt = NULL;
if (err == -ENETUNREACH)
IP_INC_STATS_BH(net, IPSTATS_MIB_OUTNOROUTES);
goto out;
--
1.7.4.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2011-03-03 19:02 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-03-03 16:28 [BUG net-next-2.6] dst_release() crash Eric Dumazet
2011-03-03 17:07 ` [PATCH net-next-2.6] udp: udp_sendmsg() fix Eric Dumazet
2011-03-03 18:42 ` David Miller
2011-03-03 19:02 ` Eric Dumazet
2011-03-03 18:38 ` [BUG net-next-2.6] dst_release() crash 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).