af_unix: Don't stop recv() at consumed ex-OOB skb.

Currently, recv() is stopped at a consumed OOB skb even if a new
OOB skb is queued and we can ignore the old OOB skb.

  >>> from socket import *
  >>> c1, c2 = socket(AF_UNIX, SOCK_STREAM)
  >>> c1.send(b'hellowor', MSG_OOB)
  8
  >>> c2.recv(1, MSG_OOB)  # consume OOB data stays at middle of recvq.
  b'r'
  >>> c1.send(b'ld', MSG_OOB)
  2
  >>> c2.recv(10)          # recv() stops at the old consumed OOB
  b'hellowo'               # should be 'hellowol'

manage_oob() should not stop recv() at the old consumed OOB skb if
there is a new OOB data queued.

Note that TCP behaviour is apparently wrong in this test case because
we can recv() the same OOB data twice.

Without fix:

  #  RUN           msg_oob.no_peek.ex_oob_ahead_break ...
  # msg_oob.c:138:ex_oob_ahead_break:AF_UNIX :hellowo
  # msg_oob.c:139:ex_oob_ahead_break:Expected:hellowol
  # msg_oob.c:141:ex_oob_ahead_break:Expected ret[0] (7) == expected_len (8)
  # ex_oob_ahead_break: Test terminated by assertion
  #          FAIL  msg_oob.no_peek.ex_oob_ahead_break
  not ok 11 msg_oob.no_peek.ex_oob_ahead_break

With fix:

  #  RUN           msg_oob.no_peek.ex_oob_ahead_break ...
  # msg_oob.c:146:ex_oob_ahead_break:AF_UNIX :hellowol
  # msg_oob.c:147:ex_oob_ahead_break:TCP     :helloworl
  #            OK  msg_oob.no_peek.ex_oob_ahead_break
  ok 11 msg_oob.no_peek.ex_oob_ahead_break

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index b0b97f8d0d09851a73cf8d667eb18c74ee56d6b3..07f5eaa04b5be373b6bbfccba33760a0b96fb353 100644 (file)
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2618,7 +2618,7 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 
                spin_lock(&sk->sk_receive_queue.lock);
 
-               if (copied) {
+               if (copied && (!u->oob_skb || skb == u->oob_skb)) {
                        skb = NULL;
                } else if (flags & MSG_PEEK) {
                        skb = skb_peek_next(skb, &sk->sk_receive_queue);

diff --git a/tools/testing/selftests/net/af_unix/msg_oob.c b/tools/testing/selftests/net/af_unix/msg_oob.c
index 46e92d06b0a399aa26bd2984768caac5c481136d..acf4bd0afe1722424872350fb52065d6133791d4 100644 (file)
--- a/tools/testing/selftests/net/af_unix/msg_oob.c
+++ b/tools/testing/selftests/net/af_unix/msg_oob.c
@@ -288,4 +288,20 @@ TEST_F(msg_oob, ex_oob_drop_2)
        }
 }
 
+TEST_F(msg_oob, ex_oob_ahead_break)
+{
+       sendpair("hello", 5, MSG_OOB);
+       sendpair("wor", 3, MSG_OOB);
+
+       recvpair("r", 1, 1, MSG_OOB);
+
+       sendpair("ld", 2, MSG_OOB);
+
+       tcp_incompliant {
+               recvpair("hellowol", 8, 10, 0); /* TCP recv()s "helloworl", why "r" ?? */
+       }
+
+       recvpair("d", 1, 1, MSG_OOB);
+}
+
 TEST_HARNESS_MAIN