[IP]: Simplify and consolidate MSG_PEEK error handling
authorHerbert Xu <herbert@gondor.apana.org.au>
Wed, 14 Dec 2005 07:16:37 +0000 (23:16 -0800)
committerDavid S. Miller <davem@sunset.davemloft.net>
Tue, 3 Jan 2006 21:10:41 +0000 (13:10 -0800)
When a packet is obtained from skb_recv_datagram with MSG_PEEK enabled
it is left on the socket receive queue.  This means that when we detect
a checksum error we have to be careful when trying to free the packet
as someone could have dequeued it in the time being.

Currently this delicate logic is duplicated three times between UDPv4,
UDPv6 and RAWv6.  This patch moves them into a one place and simplifies
the code somewhat.

This is based on a suggestion by Eric Dumazet.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/linux/skbuff.h
net/core/datagram.c
net/ipv4/udp.c
net/ipv6/raw.c
net/ipv6/udp.c

index 8c5d6001a923c5042b74dc1a0fc2bf6e30251f9b..97f6580ce039e2cd6ab82dd7829408b7b326bef9 100644 (file)
@@ -1239,6 +1239,8 @@ extern int               skb_copy_and_csum_datagram_iovec(struct sk_buff *skb,
                                                        int hlen,
                                                        struct iovec *iov);
 extern void           skb_free_datagram(struct sock *sk, struct sk_buff *skb);
+extern void           skb_kill_datagram(struct sock *sk, struct sk_buff *skb,
+                                        unsigned int flags);
 extern unsigned int    skb_checksum(const struct sk_buff *skb, int offset,
                                    int len, unsigned int csum);
 extern int            skb_copy_bits(const struct sk_buff *skb, int offset,
index 1bcfef51ac581562988c54fe9cee885572f819d6..f8d322e1ea9276c3f581fbda2393c829b6fd17f0 100644 (file)
@@ -47,6 +47,7 @@
 #include <linux/rtnetlink.h>
 #include <linux/poll.h>
 #include <linux/highmem.h>
+#include <linux/spinlock.h>
 
 #include <net/protocol.h>
 #include <linux/skbuff.h>
@@ -199,6 +200,41 @@ void skb_free_datagram(struct sock *sk, struct sk_buff *skb)
        kfree_skb(skb);
 }
 
+/**
+ *     skb_kill_datagram - Free a datagram skbuff forcibly
+ *     @sk: socket
+ *     @skb: datagram skbuff
+ *     @flags: MSG_ flags
+ *
+ *     This function frees a datagram skbuff that was received by
+ *     skb_recv_datagram.  The flags argument must match the one
+ *     used for skb_recv_datagram.
+ *
+ *     If the MSG_PEEK flag is set, and the packet is still on the
+ *     receive queue of the socket, it will be taken off the queue
+ *     before it is freed.
+ *
+ *     This function currently only disables BH when acquiring the
+ *     sk_receive_queue lock.  Therefore it must not be used in a
+ *     context where that lock is acquired in an IRQ context.
+ */
+
+void skb_kill_datagram(struct sock *sk, struct sk_buff *skb, unsigned int flags)
+{
+       if (flags & MSG_PEEK) {
+               spin_lock_bh(&sk->sk_receive_queue.lock);
+               if (skb == skb_peek(&sk->sk_receive_queue)) {
+                       __skb_unlink(skb, &sk->sk_receive_queue);
+                       atomic_dec(&skb->users);
+               }
+               spin_unlock_bh(&sk->sk_receive_queue.lock);
+       }
+
+       kfree_skb(skb);
+}
+
+EXPORT_SYMBOL(skb_kill_datagram);
+
 /**
  *     skb_copy_datagram_iovec - Copy a datagram to an iovec.
  *     @skb: buffer to copy
index 2422a5f7195d002b457f0ced6e43c3dcd5998bff..012c4621e40afc3ab9e7afc9c75cf56c852d6d4a 100644 (file)
@@ -846,20 +846,7 @@ out:
 csum_copy_err:
        UDP_INC_STATS_BH(UDP_MIB_INERRORS);
 
-       /* Clear queue. */
-       if (flags&MSG_PEEK) {
-               int clear = 0;
-               spin_lock_bh(&sk->sk_receive_queue.lock);
-               if (skb == skb_peek(&sk->sk_receive_queue)) {
-                       __skb_unlink(skb, &sk->sk_receive_queue);
-                       clear = 1;
-               }
-               spin_unlock_bh(&sk->sk_receive_queue.lock);
-               if (clear)
-                       kfree_skb(skb);
-       }
-
-       skb_free_datagram(sk, skb);
+       skb_kill_datagram(sk, skb, flags);
 
        if (noblock)
                return -EAGAIN; 
index a66900cda2afc79273b2722c9e6d755a9b8725f9..66f1d12ea578319f144044313a6776cefa7090d2 100644 (file)
@@ -32,6 +32,7 @@
 #include <linux/icmpv6.h>
 #include <linux/netfilter.h>
 #include <linux/netfilter_ipv6.h>
+#include <linux/skbuff.h>
 #include <asm/uaccess.h>
 #include <asm/ioctls.h>
 #include <asm/bug.h>
@@ -433,25 +434,14 @@ out:
        return err;
 
 csum_copy_err:
-       /* Clear queue. */
-       if (flags&MSG_PEEK) {
-               int clear = 0;
-               spin_lock_bh(&sk->sk_receive_queue.lock);
-               if (skb == skb_peek(&sk->sk_receive_queue)) {
-                       __skb_unlink(skb, &sk->sk_receive_queue);
-                       clear = 1;
-               }
-               spin_unlock_bh(&sk->sk_receive_queue.lock);
-               if (clear)
-                       kfree_skb(skb);
-       }
+       skb_kill_datagram(sk, skb, flags);
 
        /* Error for blocking case is chosen to masquerade
           as some normal condition.
         */
        err = (flags&MSG_DONTWAIT) ? -EAGAIN : -EHOSTUNREACH;
        /* FIXME: increment a raw6 drops counter here */
-       goto out_free;
+       goto out;
 }
 
 static int rawv6_push_pending_frames(struct sock *sk, struct flowi *fl,
index 5cc8731eb55b8585a75dfc97ea2226b565e75956..d8538dcea8130b0209941ec0f5eb1ab795a49cd0 100644 (file)
@@ -36,6 +36,7 @@
 #include <linux/ipv6.h>
 #include <linux/icmpv6.h>
 #include <linux/init.h>
+#include <linux/skbuff.h>
 #include <asm/uaccess.h>
 
 #include <net/sock.h>
@@ -300,20 +301,7 @@ out:
        return err;
 
 csum_copy_err:
-       /* Clear queue. */
-       if (flags&MSG_PEEK) {
-               int clear = 0;
-               spin_lock_bh(&sk->sk_receive_queue.lock);
-               if (skb == skb_peek(&sk->sk_receive_queue)) {
-                       __skb_unlink(skb, &sk->sk_receive_queue);
-                       clear = 1;
-               }
-               spin_unlock_bh(&sk->sk_receive_queue.lock);
-               if (clear)
-                       kfree_skb(skb);
-       }
-
-       skb_free_datagram(sk, skb);
+       skb_kill_datagram(sk, skb, flags);
 
        if (flags & MSG_DONTWAIT) {
                UDP6_INC_STATS_USER(UDP_MIB_INERRORS);