Commit fdc0bde90a689b9145f2b6f271c03f4c99d09667
Committed by
David S. Miller
1 parent
f6e0b239a2
Exists in
master
and in
7 other branches
icmp: icmp_sk() should not use smp_processor_id() in preemptible code
Pass namespace into icmp_xmit_lock, obtain socket inside and return it as a result for caller. Thanks Alexey Dobryan for this report: Steps to reproduce: CONFIG_PREEMPT=y CONFIG_DEBUG_PREEMPT=y tracepath <something> BUG: using smp_processor_id() in preemptible [00000000] code: tracepath/3205 caller is icmp_sk+0x15/0x30 Pid: 3205, comm: tracepath Not tainted 2.6.27-rc4 #1 Call Trace: [<ffffffff8031af14>] debug_smp_processor_id+0xe4/0xf0 [<ffffffff80409405>] icmp_sk+0x15/0x30 [<ffffffff8040a17b>] icmp_send+0x4b/0x3f0 [<ffffffff8025a415>] ? trace_hardirqs_on_caller+0xd5/0x160 [<ffffffff8025a4ad>] ? trace_hardirqs_on+0xd/0x10 [<ffffffff8023a475>] ? local_bh_enable_ip+0x95/0x110 [<ffffffff804285b9>] ? _spin_unlock_bh+0x39/0x40 [<ffffffff8025a26c>] ? mark_held_locks+0x4c/0x90 [<ffffffff8025a4ad>] ? trace_hardirqs_on+0xd/0x10 [<ffffffff8025a415>] ? trace_hardirqs_on_caller+0xd5/0x160 [<ffffffff803e91b4>] ip_fragment+0x8d4/0x900 [<ffffffff803e7030>] ? ip_finish_output2+0x0/0x290 [<ffffffff803e91e0>] ? ip_finish_output+0x0/0x60 [<ffffffff803e6650>] ? dst_output+0x0/0x10 [<ffffffff803e922c>] ip_finish_output+0x4c/0x60 [<ffffffff803e92e3>] ip_output+0xa3/0xf0 [<ffffffff803e68d0>] ip_local_out+0x20/0x30 [<ffffffff803e753f>] ip_push_pending_frames+0x27f/0x400 [<ffffffff80406313>] udp_push_pending_frames+0x233/0x3d0 [<ffffffff804067d1>] udp_sendmsg+0x321/0x6f0 [<ffffffff8040d155>] inet_sendmsg+0x45/0x80 [<ffffffff803b967f>] sock_sendmsg+0xdf/0x110 [<ffffffff8024a100>] ? autoremove_wake_function+0x0/0x40 [<ffffffff80257ce5>] ? validate_chain+0x415/0x1010 [<ffffffff8027dc10>] ? __do_fault+0x140/0x450 [<ffffffff802597d0>] ? __lock_acquire+0x260/0x590 [<ffffffff803b9e55>] ? sockfd_lookup_light+0x45/0x80 [<ffffffff803ba50a>] sys_sendto+0xea/0x120 [<ffffffff80428e42>] ? _spin_unlock_irqrestore+0x42/0x80 [<ffffffff803134bc>] ? __up_read+0x4c/0xb0 [<ffffffff8024e0c6>] ? up_read+0x26/0x30 [<ffffffff8020b8bb>] system_call_fastpath+0x16/0x1b icmp6_sk() is similar. Signed-off-by: Denis V. Lunev <den@openvz.org> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 2 changed files with 26 additions and 19 deletions Side-by-side Diff
net/ipv4/icmp.c
... | ... | @@ -204,18 +204,22 @@ |
204 | 204 | return net->ipv4.icmp_sk[smp_processor_id()]; |
205 | 205 | } |
206 | 206 | |
207 | -static inline int icmp_xmit_lock(struct sock *sk) | |
207 | +static inline struct sock *icmp_xmit_lock(struct net *net) | |
208 | 208 | { |
209 | + struct sock *sk; | |
210 | + | |
209 | 211 | local_bh_disable(); |
210 | 212 | |
213 | + sk = icmp_sk(net); | |
214 | + | |
211 | 215 | if (unlikely(!spin_trylock(&sk->sk_lock.slock))) { |
212 | 216 | /* This can happen if the output path signals a |
213 | 217 | * dst_link_failure() for an outgoing ICMP packet. |
214 | 218 | */ |
215 | 219 | local_bh_enable(); |
216 | - return 1; | |
220 | + return NULL; | |
217 | 221 | } |
218 | - return 0; | |
222 | + return sk; | |
219 | 223 | } |
220 | 224 | |
221 | 225 | static inline void icmp_xmit_unlock(struct sock *sk) |
222 | 226 | |
223 | 227 | |
... | ... | @@ -354,15 +358,17 @@ |
354 | 358 | struct ipcm_cookie ipc; |
355 | 359 | struct rtable *rt = skb->rtable; |
356 | 360 | struct net *net = dev_net(rt->u.dst.dev); |
357 | - struct sock *sk = icmp_sk(net); | |
358 | - struct inet_sock *inet = inet_sk(sk); | |
361 | + struct sock *sk; | |
362 | + struct inet_sock *inet; | |
359 | 363 | __be32 daddr; |
360 | 364 | |
361 | 365 | if (ip_options_echo(&icmp_param->replyopts, skb)) |
362 | 366 | return; |
363 | 367 | |
364 | - if (icmp_xmit_lock(sk)) | |
368 | + sk = icmp_xmit_lock(net); | |
369 | + if (sk == NULL) | |
365 | 370 | return; |
371 | + inet = inet_sk(sk); | |
366 | 372 | |
367 | 373 | icmp_param->data.icmph.checksum = 0; |
368 | 374 | |
... | ... | @@ -419,7 +425,6 @@ |
419 | 425 | if (!rt) |
420 | 426 | goto out; |
421 | 427 | net = dev_net(rt->u.dst.dev); |
422 | - sk = icmp_sk(net); | |
423 | 428 | |
424 | 429 | /* |
425 | 430 | * Find the original header. It is expected to be valid, of course. |
... | ... | @@ -483,7 +488,8 @@ |
483 | 488 | } |
484 | 489 | } |
485 | 490 | |
486 | - if (icmp_xmit_lock(sk)) | |
491 | + sk = icmp_xmit_lock(net); | |
492 | + if (sk == NULL) | |
487 | 493 | return; |
488 | 494 | |
489 | 495 | /* |
net/ipv6/icmp.c
... | ... | @@ -91,19 +91,22 @@ |
91 | 91 | .flags = INET6_PROTO_NOPOLICY|INET6_PROTO_FINAL, |
92 | 92 | }; |
93 | 93 | |
94 | -static __inline__ int icmpv6_xmit_lock(struct sock *sk) | |
94 | +static __inline__ struct sock *icmpv6_xmit_lock(struct net *net) | |
95 | 95 | { |
96 | + struct sock *sk; | |
97 | + | |
96 | 98 | local_bh_disable(); |
97 | 99 | |
100 | + sk = icmpv6_sk(net); | |
98 | 101 | if (unlikely(!spin_trylock(&sk->sk_lock.slock))) { |
99 | 102 | /* This can happen if the output path (f.e. SIT or |
100 | 103 | * ip6ip6 tunnel) signals dst_link_failure() for an |
101 | 104 | * outgoing ICMP6 packet. |
102 | 105 | */ |
103 | 106 | local_bh_enable(); |
104 | - return 1; | |
107 | + return NULL; | |
105 | 108 | } |
106 | - return 0; | |
109 | + return sk; | |
107 | 110 | } |
108 | 111 | |
109 | 112 | static __inline__ void icmpv6_xmit_unlock(struct sock *sk) |
110 | 113 | |
... | ... | @@ -392,12 +395,11 @@ |
392 | 395 | fl.fl_icmp_code = code; |
393 | 396 | security_skb_classify_flow(skb, &fl); |
394 | 397 | |
395 | - sk = icmpv6_sk(net); | |
398 | + sk = icmpv6_xmit_lock(net); | |
399 | + if (sk == NULL) | |
400 | + return; | |
396 | 401 | np = inet6_sk(sk); |
397 | 402 | |
398 | - if (icmpv6_xmit_lock(sk)) | |
399 | - return; | |
400 | - | |
401 | 403 | if (!icmpv6_xrlim_allow(sk, type, &fl)) |
402 | 404 | goto out; |
403 | 405 | |
404 | 406 | |
... | ... | @@ -539,11 +541,10 @@ |
539 | 541 | fl.fl_icmp_type = ICMPV6_ECHO_REPLY; |
540 | 542 | security_skb_classify_flow(skb, &fl); |
541 | 543 | |
542 | - sk = icmpv6_sk(net); | |
543 | - np = inet6_sk(sk); | |
544 | - | |
545 | - if (icmpv6_xmit_lock(sk)) | |
544 | + sk = icmpv6_xmit_lock(net); | |
545 | + if (sk == NULL) | |
546 | 546 | return; |
547 | + np = inet6_sk(sk); | |
547 | 548 | |
548 | 549 | if (!fl.oif && ipv6_addr_is_multicast(&fl.fl6_dst)) |
549 | 550 | fl.oif = np->mcast_oif; |