sock: deduplicate errqueue dequeue

sk->sk_error_queue is dequeued in four locations. All share the
exact same logic. Deduplicate.

Also collapse the two critical sections for dequeue (at the top of
the recv handler) and signal (at the bottom).

This moves signal generation for the next packet forward, which should
be harmless.

It also changes the behavior if the recv handler exits early with an
error. Previously, a signal for follow-up packets on the errqueue
would then not be scheduled. The new behavior, to always signal, is
arguably a bug fix.

For rxrpc, the change causes the same function to be called repeatedly
for each queued packet (because the recv handler == sk_error_report).
It is likely that all packets will fail for the same reason (e.g.,
memory exhaustion).

This code runs without sk_lock held, so it is not safe to trust that
sk->sk_err is immutable inbetween releasing q->lock and the subsequent
test. Introduce int err just to avoid this potential race.

Signed-off-by: Willem de Bruijn <willemb@google.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

2 files changed, 22 insertions, 12 deletions

diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 163b673f9e62..53ce536e3d6e 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -3491,6 +3491,26 @@ int sock_queue_err_skb(struct sock *sk, struct sk_buff *skb)
 }
 EXPORT_SYMBOL(sock_queue_err_skb);
 
+struct sk_buff *sock_dequeue_err_skb(struct sock *sk)
+{
+        struct sk_buff_head *q = &sk->sk_error_queue;
+        struct sk_buff *skb, *skb_next;
+        int err = 0;
+
+        spin_lock_bh(&q->lock);
+        skb = __skb_dequeue(q);
+        if (skb && (skb_next = skb_peek(q)))
+                err = SKB_EXT_ERR(skb_next)->ee.ee_errno;
+        spin_unlock_bh(&q->lock);
+
+        sk->sk_err = err;
+        if (err)
+                sk->sk_error_report(sk);
+
+        return skb;
+}
+EXPORT_SYMBOL(sock_dequeue_err_skb);
+
 void __skb_tstamp_tx(struct sk_buff *orig_skb,
                      struct skb_shared_hwtstamps *hwtstamps,
                      struct sock *sk, int tstype)
diff --git a/net/core/sock.c b/net/core/sock.c
index f7f2352200ad..f1a638ee93d9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -2488,11 +2488,11 @@ int sock_recv_errqueue(struct sock *sk, struct msghdr *msg, int len,
                        int level, int type)
 {
         struct sock_exterr_skb *serr;
-        struct sk_buff *skb, *skb2;
+        struct sk_buff *skb;
         int copied, err;
 
         err = -EAGAIN;
-        skb = skb_dequeue(&sk->sk_error_queue);
+        skb = sock_dequeue_err_skb(sk);
         if (skb == NULL)
                 goto out;
 
@@ -2513,16 +2513,6 @@ int sock_recv_errqueue(struct sock *sk, struct msghdr *msg, int len,
         msg->msg_flags |= MSG_ERRQUEUE;
         err = copied;
 
-        /* Reset and regenerate socket error */
-        spin_lock_bh(&sk->sk_error_queue.lock);
-        sk->sk_err = 0;
-        if ((skb2 = skb_peek(&sk->sk_error_queue)) != NULL) {
-                sk->sk_err = SKB_EXT_ERR(skb2)->ee.ee_errno;
-                spin_unlock_bh(&sk->sk_error_queue.lock);
-                sk->sk_error_report(sk);
-        } else
-                spin_unlock_bh(&sk->sk_error_queue.lock);
-
 out_free_skb:
         kfree_skb(skb);
 out: