diff options
author | Eric Dumazet <eric.dumazet@gmail.com> | 2009-12-15 00:47:03 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2009-12-16 00:12:21 -0500 |
commit | 1a35ca80c1db7279c3c0655063f6d3490e399b17 (patch) | |
tree | 3ff2f23730c2bc6ea8af20232d02dad65ae63f0a /net/packet | |
parent | 81e839efc22361e3fa7ee36f99fd57c57d0d1871 (diff) |
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>
Diffstat (limited to 'net/packet')
-rw-r--r-- | net/packet/af_packet.c | 71 |
1 files changed, 31 insertions, 40 deletions
diff --git a/net/packet/af_packet.c b/net/packet/af_packet.c index 020562164b5..e0516a22be2 100644 --- a/net/packet/af_packet.c +++ b/net/packet/af_packet.c | |||
@@ -415,7 +415,7 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock, | |||
415 | { | 415 | { |
416 | struct sock *sk = sock->sk; | 416 | struct sock *sk = sock->sk; |
417 | struct sockaddr_pkt *saddr = (struct sockaddr_pkt *)msg->msg_name; | 417 | struct sockaddr_pkt *saddr = (struct sockaddr_pkt *)msg->msg_name; |
418 | struct sk_buff *skb; | 418 | struct sk_buff *skb = NULL; |
419 | struct net_device *dev; | 419 | struct net_device *dev; |
420 | __be16 proto = 0; | 420 | __be16 proto = 0; |
421 | int err; | 421 | int err; |
@@ -437,6 +437,7 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock, | |||
437 | */ | 437 | */ |
438 | 438 | ||
439 | saddr->spkt_device[13] = 0; | 439 | saddr->spkt_device[13] = 0; |
440 | retry: | ||
440 | rcu_read_lock(); | 441 | rcu_read_lock(); |
441 | dev = dev_get_by_name_rcu(sock_net(sk), saddr->spkt_device); | 442 | dev = dev_get_by_name_rcu(sock_net(sk), saddr->spkt_device); |
442 | err = -ENODEV; | 443 | err = -ENODEV; |
@@ -456,58 +457,48 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock, | |||
456 | if (len > dev->mtu + dev->hard_header_len) | 457 | if (len > dev->mtu + dev->hard_header_len) |
457 | goto out_unlock; | 458 | goto out_unlock; |
458 | 459 | ||
459 | err = -ENOBUFS; | 460 | if (!skb) { |
460 | skb = sock_wmalloc(sk, len + LL_RESERVED_SPACE(dev), 0, GFP_KERNEL); | 461 | size_t reserved = LL_RESERVED_SPACE(dev); |
461 | 462 | unsigned int hhlen = dev->header_ops ? dev->hard_header_len : 0; | |
462 | /* | 463 | |
463 | * If the write buffer is full, then tough. At this level the user | 464 | rcu_read_unlock(); |
464 | * gets to deal with the problem - do your own algorithmic backoffs. | 465 | skb = sock_wmalloc(sk, len + reserved, 0, GFP_KERNEL); |
465 | * That's far more flexible. | 466 | if (skb == NULL) |
466 | */ | 467 | return -ENOBUFS; |
467 | 468 | /* FIXME: Save some space for broken drivers that write a hard | |
468 | if (skb == NULL) | 469 | * header at transmission time by themselves. PPP is the notable |
469 | goto out_unlock; | 470 | * one here. This should really be fixed at the driver level. |
470 | 471 | */ | |
471 | /* | 472 | skb_reserve(skb, reserved); |
472 | * Fill it in | 473 | skb_reset_network_header(skb); |
473 | */ | 474 | |
474 | 475 | /* Try to align data part correctly */ | |
475 | /* FIXME: Save some space for broken drivers that write a | 476 | if (hhlen) { |
476 | * hard header at transmission time by themselves. PPP is the | 477 | skb->data -= hhlen; |
477 | * notable one here. This should really be fixed at the driver level. | 478 | skb->tail -= hhlen; |
478 | */ | 479 | if (len < hhlen) |
479 | skb_reserve(skb, LL_RESERVED_SPACE(dev)); | 480 | skb_reset_network_header(skb); |
480 | skb_reset_network_header(skb); | 481 | } |
481 | 482 | err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len); | |
482 | /* Try to align data part correctly */ | 483 | if (err) |
483 | if (dev->header_ops) { | 484 | goto out_free; |
484 | skb->data -= dev->hard_header_len; | 485 | goto retry; |
485 | skb->tail -= dev->hard_header_len; | ||
486 | if (len < dev->hard_header_len) | ||
487 | skb_reset_network_header(skb); | ||
488 | } | 486 | } |
489 | 487 | ||
490 | /* Returns -EFAULT on error */ | 488 | |
491 | err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len); | ||
492 | skb->protocol = proto; | 489 | skb->protocol = proto; |
493 | skb->dev = dev; | 490 | skb->dev = dev; |
494 | skb->priority = sk->sk_priority; | 491 | skb->priority = sk->sk_priority; |
495 | skb->mark = sk->sk_mark; | 492 | skb->mark = sk->sk_mark; |
496 | if (err) | ||
497 | goto out_free; | ||
498 | |||
499 | /* | ||
500 | * Now send it | ||
501 | */ | ||
502 | 493 | ||
503 | dev_queue_xmit(skb); | 494 | dev_queue_xmit(skb); |
504 | rcu_read_unlock(); | 495 | rcu_read_unlock(); |
505 | return len; | 496 | return len; |
506 | 497 | ||
507 | out_free: | ||
508 | kfree_skb(skb); | ||
509 | out_unlock: | 498 | out_unlock: |
510 | rcu_read_unlock(); | 499 | rcu_read_unlock(); |
500 | out_free: | ||
501 | kfree_skb(skb); | ||
511 | return err; | 502 | return err; |
512 | } | 503 | } |
513 | 504 | ||