packet: dont call sleeping functions while holding rcu_read_lock()
Eric Dumazet [Tue, 15 Dec 2009 05:47:03 +0000 (05:47 +0000)]
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>

net/packet/af_packet.c

index 0205621..e0516a2 100644 (file)
@@ -415,7 +415,7 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
 {
        struct sock *sk = sock->sk;
        struct sockaddr_pkt *saddr = (struct sockaddr_pkt *)msg->msg_name;
-       struct sk_buff *skb;
+       struct sk_buff *skb = NULL;
        struct net_device *dev;
        __be16 proto = 0;
        int err;
@@ -437,6 +437,7 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
         */
 
        saddr->spkt_device[13] = 0;
+retry:
        rcu_read_lock();
        dev = dev_get_by_name_rcu(sock_net(sk), saddr->spkt_device);
        err = -ENODEV;
@@ -456,58 +457,48 @@ static int packet_sendmsg_spkt(struct kiocb *iocb, struct socket *sock,
        if (len > dev->mtu + dev->hard_header_len)
                goto out_unlock;
 
-       err = -ENOBUFS;
-       skb = sock_wmalloc(sk, len + LL_RESERVED_SPACE(dev), 0, GFP_KERNEL);
-
-       /*
-        * If the write buffer is full, then tough. At this level the user
-        * gets to deal with the problem - do your own algorithmic backoffs.
-        * That's far more flexible.
-        */
-
-       if (skb == NULL)
-               goto out_unlock;
-
-       /*
-        *      Fill it in
-        */
-
-       /* FIXME: Save some space for broken drivers that write a
-        * hard header at transmission time by themselves. PPP is the
-        * notable one here. This should really be fixed at the driver level.
-        */
-       skb_reserve(skb, LL_RESERVED_SPACE(dev));
-       skb_reset_network_header(skb);
-
-       /* Try to align data part correctly */
-       if (dev->header_ops) {
-               skb->data -= dev->hard_header_len;
-               skb->tail -= dev->hard_header_len;
-               if (len < dev->hard_header_len)
-                       skb_reset_network_header(skb);
+       if (!skb) {
+               size_t reserved = LL_RESERVED_SPACE(dev);
+               unsigned int hhlen = dev->header_ops ? dev->hard_header_len : 0;
+
+               rcu_read_unlock();
+               skb = sock_wmalloc(sk, len + reserved, 0, GFP_KERNEL);
+               if (skb == NULL)
+                       return -ENOBUFS;
+               /* FIXME: Save some space for broken drivers that write a hard
+                * header at transmission time by themselves. PPP is the notable
+                * one here. This should really be fixed at the driver level.
+                */
+               skb_reserve(skb, reserved);
+               skb_reset_network_header(skb);
+
+               /* Try to align data part correctly */
+               if (hhlen) {
+                       skb->data -= hhlen;
+                       skb->tail -= hhlen;
+                       if (len < hhlen)
+                               skb_reset_network_header(skb);
+               }
+               err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
+               if (err)
+                       goto out_free;
+               goto retry;
        }
 
-       /* Returns -EFAULT on error */
-       err = memcpy_fromiovec(skb_put(skb, len), msg->msg_iov, len);
+
        skb->protocol = proto;
        skb->dev = dev;
        skb->priority = sk->sk_priority;
        skb->mark = sk->sk_mark;
-       if (err)
-               goto out_free;
-
-       /*
-        *      Now send it
-        */
 
        dev_queue_xmit(skb);
        rcu_read_unlock();
        return len;
 
-out_free:
-       kfree_skb(skb);
 out_unlock:
        rcu_read_unlock();
+out_free:
+       kfree_skb(skb);
        return err;
 }