Commit 1a35ca80c1db7279c3c0655063f6d3490e399b17
Committed by
David S. Miller
David S. Miller
1 parent
81e839efc2
Exists in
master
and in
7 other branches
packet: dont call sleeping functions while holding rcu_read_lock()
commit 654d1f8a019dfa06d (packet: less dev_put() calls) introduced a problem, calling potentially sleeping functions from a rcu_read_lock() protected section. Fix this by releasing lock before the sock_wmalloc()/memcpy_fromiovec() calls. After skb allocation and copy from user space, we redo device lookup and appropriate tests. Reported-and-tested-by: Frederic Weisbecker <fweisbec@gmail.com> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Showing 1 changed file with 31 additions and 40 deletions Side-by-side Diff
net/packet/af_packet.c
| ... | ... | @@ -415,7 +415,7 @@ |
| 415 | 415 | { |
| 416 | 416 | struct sock *sk = sock->sk; |
| 417 | 417 | struct sockaddr_pkt *saddr = (struct sockaddr_pkt *)msg->msg_name; |
| 418 | - struct sk_buff *skb; | |
| 418 | + struct sk_buff *skb = NULL; | |
| 419 | 419 | struct net_device *dev; |
| 420 | 420 | __be16 proto = 0; |
| 421 | 421 | int err; |
| ... | ... | @@ -437,6 +437,7 @@ |
| 437 | 437 | */ |
| 438 | 438 | |
| 439 | 439 | saddr->spkt_device[13] = 0; |
| 440 | +retry: | |
| 440 | 441 | rcu_read_lock(); |
| 441 | 442 | dev = dev_get_by_name_rcu(sock_net(sk), saddr->spkt_device); |
| 442 | 443 | err = -ENODEV; |
| 443 | 444 | |
| 444 | 445 | |
| 445 | 446 | |
| 446 | 447 | |
| 447 | 448 | |
| 448 | 449 | |
| 449 | 450 | |
| ... | ... | @@ -456,58 +457,48 @@ |
| 456 | 457 | if (len > dev->mtu + dev->hard_header_len) |
| 457 | 458 | goto out_unlock; |
| 458 | 459 | |
| 459 | - err = -ENOBUFS; | |
| 460 | - skb = sock_wmalloc(sk, len + LL_RESERVED_SPACE(dev), 0, GFP_KERNEL); | |
| 460 | + if (!skb) { | |
| 461 | + size_t reserved = LL_RESERVED_SPACE(dev); | |
| 462 | + unsigned int hhlen = dev->header_ops ? dev->hard_header_len : 0; | |
| 461 | 463 | |
| 462 | - /* | |
| 463 | - * If the write buffer is full, then tough. At this level the user | |
| 464 | - * gets to deal with the problem - do your own algorithmic backoffs. | |
| 465 | - * That's far more flexible. | |
| 466 | - */ | |
| 464 | + rcu_read_unlock(); | |
| 465 | + skb = sock_wmalloc(sk, len + reserved, 0, GFP_KERNEL); | |
| 466 | + if (skb == NULL) | |
| 467 | + return -ENOBUFS; | |
| 468 | + /* FIXME: Save some space for broken drivers that write a hard | |
| 469 | + * header at transmission time by themselves. PPP is the notable | |
| 470 | + * one here. This should really be fixed at the driver level. | |
| 471 | + */ | |
| 472 | + skb_reserve(skb, reserved); | |
| 473 | + skb_reset_network_header(skb); | |
| 467 | 474 | |
| 468 | - if (skb == NULL) | |
| 469 | - goto out_unlock; | |
| 470 | - | |
| 471 | - /* | |
| 472 | - * Fill it in | |
| 473 | - */ | |
| 474 | - | |
| 475 | - /* FIXME: Save some space for broken drivers that write a | |
| 476 | - * hard header at transmission time by themselves. PPP is the | |
| 477 | - * notable one here. This should really be fixed at the driver level. | |
| 478 | - */ | |
| 479 | - skb_reserve(skb, LL_RESERVED_SPACE(dev)); | |
| 480 | - skb_reset_network_header(skb); | |
| 481 | - | |
| 482 | - /* Try to align data part correctly */ | |
| 483 | - if (dev->header_ops) { | |
| 484 | - skb->data -= dev->hard_header_len; | |
| 485 | - skb->tail -= dev->hard_header_len; | |
| 486 | - if (len < dev->hard_header_len) | |
| 487 | - skb_reset_network_header(skb); | |
| 475 | + /* Try to align data part correctly */ | |
| 476 | + if (hhlen) { | |
| 477 | + skb->data -= hhlen; | |
| 478 | + skb->tail -= hhlen; | |
| 479 | + if (len < hhlen) | |
| 480 | + skb_reset_network_header(skb); | |
| 481 | + } | |
| 482 | + err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len); | |
| 483 | + if (err) | |
| 484 | + goto out_free; | |
| 485 | + goto retry; | |
| 488 | 486 | } |
| 489 | 487 | |
| 490 | - /* Returns -EFAULT on error */ | |
| 491 | - err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len); | |
| 488 | + | |
| 492 | 489 | skb->protocol = proto; |
| 493 | 490 | skb->dev = dev; |
| 494 | 491 | skb->priority = sk->sk_priority; |
| 495 | 492 | skb->mark = sk->sk_mark; |
| 496 | - if (err) | |
| 497 | - goto out_free; | |
| 498 | 493 | |
| 499 | - /* | |
| 500 | - * Now send it | |
| 501 | - */ | |
| 502 | - | |
| 503 | 494 | dev_queue_xmit(skb); |
| 504 | 495 | rcu_read_unlock(); |
| 505 | 496 | return len; |
| 506 | 497 | |
| 507 | -out_free: | |
| 508 | - kfree_skb(skb); | |
| 509 | 498 | out_unlock: |
| 510 | 499 | rcu_read_unlock(); |
| 500 | +out_free: | |
| 501 | + kfree_skb(skb); | |
| 511 | 502 | return err; |
| 512 | 503 | } |
| 513 | 504 |