af_unix: Fix MSG_OOB bugs with MSG_PEEK.

PATCH Applied Kuniyuki Iwashima 3 participants 12 messages Apr 10, 2024 – Apr 17, 2024

Subsystems (3)

Networking Network drivers Networking unix sockets

lore

Share & tools

lore

Kuniyuki Iwashima

author

2024-04-10 17:10 UTC #0 lore raw

v2 cover af_unix: Fix MSG_OOB bugs with MSG_PEEK.

Currently, OOB data can be read without MSG_OOB accidentally in two cases, and this seris fixes the bugs.

Changes:
v2:
Drop patch 3

v1: https://lore.kernel.org/netdev/[email protected]/

Kuniyuki Iwashima (2):
af_unix: Call manage_oob() for every skb in
unix_stream_read_generic().
af_unix: Don't peek OOB data without MSG_OOB.

 net/unix/af_unix.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Signature · 1 line

2.30.2

Kuniyuki Iwashima

author

2024-04-10 17:10 UTC #1 lore raw

PATCH v2 1/2 af_unix: Call manage_oob() for every skb in unix_stream_read_generic().

When we call recv() for AF_UNIX socket, we first peek one skb and calls manage_oob() to check if the skb is sent with MSG_OOB.

However, when we fetch the next (and the following) skb, manage_oob() is not called now, leading a wrong behaviour.

Let's say a socket send()s "hello" with MSG_OOB and the peer tries to recv() 5 bytes with MSG_PEEK. Here, we should get only "hell" without 'o', but actually not:

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'hello', MSG_OOB)

Quoted 1 line

Quoted 1 line
Quoted 1 line
c2.recv(5, MSG_PEEK)

b'hello'

The first skb fills 4 bytes, and the next skb is peeked but not properly checked by manage_oob().

Let's move up the again label to call manage_oob() for evry skb.

With this patch:

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'hello', MSG_OOB)

Quoted 1 line

Quoted 1 line
Quoted 1 line
c2.recv(5, MSG_PEEK)

b'hell'

Fixes 314001f0bf92 ("af_unix: Add OOB support")

Signed-off-by Kuniyuki Iwashima <[email protected]>

---
 net/unix/af_unix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

net/unix/af_unix.c +1 -1

index d032eb5fa6df..f297320438bf 100644
@@ -2741,6 +2741,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 		last = skb = skb_peek(&sk->sk_receive_queue);
 		last_len = last ? last->len : 0;
 
+again:
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
 		if (skb) {
 			skb = manage_oob(skb, sk, flags, copied);

@@ -2752,7 +2753,6 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 			}
 		}
 #endif
-again:
 		if (skb == NULL) {
 			if (copied >= target)
 				goto unlock;

Signature · 1 line

2.30.2

Rao Shoaib

Reviewed-by on v2 1/2

2024-04-15 02:52 UTC #2 lore raw

Re: [PATCH v2 net 1/2] af_unix: Call manage_oob() for every skb in unix_stream_read_generic().

Quoted 50 lines · Kuniyuki Iwashima

When we call recv() for AF_UNIX socket, we first peek one skb and calls manage_oob() to check if the skb is sent with MSG_OOB.
However, when we fetch the next (and the following) skb, manage_oob() is not called now, leading a wrong behaviour.
Let's say a socket send()s "hello" with MSG_OOB and the peer tries to recv() 5 bytes with MSG_PEEK. Here, we should get only "hell" without 'o', but actually not:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'hello', MSG_OOB)
5
Quoted 1 line
Quoted 1 line
Quoted 1 line
c2.recv(5, MSG_PEEK)
b'hello'
The first skb fills 4 bytes, and the next skb is peeked but not properly checked by manage_oob().
Let's move up the again label to call manage_oob() for evry skb.
With this patch:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'hello', MSG_OOB)
5
Quoted 1 line
Quoted 1 line
Quoted 1 line
c2.recv(5, MSG_PEEK)
b'hell'
Fixes 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by Kuniyuki Iwashima <[email protected]>
---
  net/unix/af_unix.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)
net/unix/af_unix.c +1 -0
index d032eb5fa6df..f297320438bf 100644
@@ -2741,6 +2741,7 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
  		last = skb = skb_peek(&sk->sk_receive_queue);
  		last_len = last ? last->len : 0;
 
+again:
  #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
  		if (skb) {
  			skb = manage_oob(skb, sk, flags, copied);

net/unix/af_unix.c static int unix_stream_read_generic(struct unix_stream_read_state *state, · line 2758

Quoted 7 lines

net/unix/af_unix.c +0 -1

@@ -2752,7 +2753,6 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
  			}
  		}
  #endif
-again:
  		if (skb == NULL) {
  			if (copied >= target)
  				goto unlock;

Looks Good.

Reviewed-by Rao shoaib <[email protected]>

Shoaib

Kuniyuki Iwashima

author

2024-04-10 17:11 UTC #3 lore raw

PATCH v2 2/2 af_unix: Don't peek OOB data without MSG_OOB.

In reply to Kuniyuki Iwashima

Currently, we can read OOB data without MSG_OOB by using MSG_PEEK when OOB data is sitting on the front row, which is apparently wrong.

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'a', MSG_OOB)

Quoted 1 line

Quoted 1 line
Quoted 1 line
c2.recv(1, MSG_PEEK | MSG_DONTWAIT)

b'a'

If manage_oob() is called when no data has been copied, we only check if the socket enables SO_OOBINLINE or MSG_PEEK is not used. Otherwise, the skb is returned as is.

However, here we should return NULL if MSG_PEEK is set and no data has been copied.

Also, in such a case, we should not jump to the redo label because we will be caught in the loop and hog the CPU until normal data comes in.

Then, we need to handle skb == NULL case with the if-clause below the manage_oob() block.

With this patch:

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'a', MSG_OOB)

Quoted 1 line

Quoted 1 line
Quoted 1 line
c2.recv(1, MSG_PEEK | MSG_DONTWAIT)

Traceback (most recent call last):

    File "<stdin>", line 1, in <module>

BlockingIOError: [Errno 11] Resource temporarily unavailable

Fixes 314001f0bf92 ("af_unix: Add OOB support")

Signed-off-by Kuniyuki Iwashima <[email protected]>

---
 net/unix/af_unix.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

net/unix/af_unix.c +5 -5

index f297320438bf..9a6ad5974dff 100644
@@ -2663,7 +2663,9 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 					WRITE_ONCE(u->oob_skb, NULL);
 					consume_skb(skb);
 				}
-			} else if (!(flags & MSG_PEEK)) {
+			} else if (flags & MSG_PEEK) {
+				skb = NULL;
+			} else {
 				skb_unlink(skb, &sk->sk_receive_queue);
 				WRITE_ONCE(u->oob_skb, NULL);
 				if (!WARN_ON_ONCE(skb_unref(skb)))

@@ -2745,11 +2747,9 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
 		if (skb) {
 			skb = manage_oob(skb, sk, flags, copied);
-			if (!skb) {
+			if (!skb && copied) {
 				unix_state_unlock(sk);
-				if (copied)
-					break;
-				goto redo;
+				break;
 			}
 		}
 #endif

Signature · 1 line

2.30.2

Rao Shoaib

2024-04-16 20:11 UTC #4 lore raw

Re: [PATCH v2 net 2/2] af_unix: Don't peek OOB data without MSG_OOB.

The proposed fix is not the correct fix as among other things it does not allow going pass the OOB if data is present. TCP allows that.

I have attached a patch that fixes this issue and other issues that I encountered in my testing. I compared TCP.

Shoaib

Quoted 61 lines · Kuniyuki Iwashima

Currently, we can read OOB data without MSG_OOB by using MSG_PEEK when OOB data is sitting on the front row, which is apparently wrong.
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'a', MSG_OOB)
1
Quoted 1 line
Quoted 1 line
Quoted 1 line
c2.recv(1, MSG_PEEK | MSG_DONTWAIT)
b'a'
If manage_oob() is called when no data has been copied, we only check if the socket enables SO_OOBINLINE or MSG_PEEK is not used. Otherwise, the skb is returned as is.
However, here we should return NULL if MSG_PEEK is set and no data has been copied.
Also, in such a case, we should not jump to the redo label because we will be caught in the loop and hog the CPU until normal data comes in.
Then, we need to handle skb == NULL case with the if-clause below the manage_oob() block.
With this patch:
Quoted 3 lines
Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
c1.send(b'a', MSG_OOB)
1
Quoted 1 line
Quoted 1 line
Quoted 1 line
c2.recv(1, MSG_PEEK | MSG_DONTWAIT)
Traceback (most recent call last):
     File "<stdin>", line 1, in <module>
BlockingIOError: [Errno 11] Resource temporarily unavailable
Fixes 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by Kuniyuki Iwashima <[email protected]>
---
  net/unix/af_unix.c | 10 +++++-----
  1 file changed, 5 insertions(+), 5 deletions(-)
net/unix/af_unix.c +3 -1
index f297320438bf..9a6ad5974dff 100644
@@ -2663,7 +2663,9 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
  					WRITE_ONCE(u->oob_skb, NULL);
  					consume_skb(skb);
  				}
-			} else if (!(flags & MSG_PEEK)) {
+			} else if (flags & MSG_PEEK) {
+				skb = NULL;
+			} else {
  				skb_unlink(skb, &sk->sk_receive_queue);
  				WRITE_ONCE(u->oob_skb, NULL);
  				if (!WARN_ON_ONCE(skb_unref(skb)))
@@ -2745,11 +2747,9 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
  #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
  		if (skb) {
  			skb = manage_oob(skb, sk, flags, copied);

net/unix/af_unix.c static int unix_stream_read_generic(struct unix_stream_read_state *state, · line 2755

Quoted 10 lines

net/unix/af_unix.c +2 -4

-			if (!skb) {
+			if (!skb && copied) {
  				unix_state_unlock(sk);
-				if (copied)
-					break;
-				goto redo;
+				break;
  			}
  		}
  #endif

From bf541423b785ae14ee2c7015d96dba6ac368358c Mon Sep 17 00:00:00 2001
From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 12:39:54 -0700
Subject: [PATCH] AF_UNIX: Fix handling of OOB Data

This patch fixes corner cases in reading when there is a pending OOB and MSG_PEEK flag is set.
Since there is no standard for handling MSG_OOB and MSG_PEEK, behavior of TCP is used as a reference.

First byte in the read queue is OOB byte
First byte in the read queue is OOB byte followed by normal data New OOB arrives before the previous one was consumed

Signed-off-by Rao Shoaib <[email protected]>

---
 net/unix/af_unix.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

net/unix/af_unix.c +20 ~~-12~~

index d032eb5fa6df..50cae03a1f98 100644
@@ -2217,8 +2217,13 @@ static int queue_oob(struct socket *sock, struct msghdr *msg, struct sock *other
 	maybe_add_creds(skb, sock, other);
 	skb_get(skb);
 
-	if (ousk->oob_skb)
-		consume_skb(ousk->oob_skb);
+	if (ousk->oob_skb) {
+		skb_unref(ousk->oob_skb);
+		if (!sock_flag(other, SOCK_URGINLINE)) {
+			skb_unlink(ousk->oob_skb, &other->sk_receive_queue);
+			consume_skb(ousk->oob_skb);
+		}
+	}
 
 	WRITE_ONCE(ousk->oob_skb, skb);

@@ -2658,17 +2663,20 @@ static struct sk_buff *manage_oob(struct sk_buff *skb, struct sock *sk,
 		if (skb == u->oob_skb) {
 			if (copied) {
 				skb = NULL;
-			} else if (sock_flag(sk, SOCK_URGINLINE)) {
-				if (!(flags & MSG_PEEK)) {
+			} else if (!(flags & MSG_PEEK)) {
+				if (sock_flag(sk, SOCK_URGINLINE)) {
 					WRITE_ONCE(u->oob_skb, NULL);
 					consume_skb(skb);
+				} else {
+					skb_unlink(skb, &sk->sk_receive_queue);
+					WRITE_ONCE(u->oob_skb, NULL);
+					if (!WARN_ON_ONCE(skb_unref(skb)))
+						kfree_skb(skb);
+					skb = skb_peek(&sk->sk_receive_queue);
 				}
-			} else if (!(flags & MSG_PEEK)) {
-				skb_unlink(skb, &sk->sk_receive_queue);
-				WRITE_ONCE(u->oob_skb, NULL);
-				if (!WARN_ON_ONCE(skb_unref(skb)))
-					kfree_skb(skb);
-				skb = skb_peek(&sk->sk_receive_queue);
+			} else {
+				if (!sock_flag(sk, SOCK_URGINLINE))
+					skb = skb_peek_next(skb, &sk->sk_receive_queue);
 			}
 		}
 	}

@@ -2741,18 +2749,18 @@ static int unix_stream_read_generic(struct unix_stream_read_state *state,
 		last = skb = skb_peek(&sk->sk_receive_queue);
 		last_len = last ? last->len : 0;
 
+again:
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
 		if (skb) {
 			skb = manage_oob(skb, sk, flags, copied);
 			if (!skb) {
 				unix_state_unlock(sk);
-				if (copied)
+				if (copied || (flags & MSG_PEEK))
 					break;
 				goto redo;
 			}
 		}
 #endif
-again:
 		if (skb == NULL) {
 			if (copied >= target)
 				goto unlock;

Signature · 1 line

2.39.3

Kuniyuki Iwashima

author

2024-04-16 20:51 UTC #5 lore raw

Re: [PATCH v2 net 2/2] af_unix: Don't peek OOB data without MSG_OOB.

From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 13:11:09 -0700

Quoted 2 lines

The proposed fix is not the correct fix as among other things it does not allow going pass the OOB if data is present. TCP allows that.

Ugh, exactly.

But the behaviour was broken initially, so the tag is

Fixes 314001f0bf92 ("af_unix: Add OOB support")

TCP:
---8<---

Quoted 8 lines

Quoted 8 lines
Quoted 8 lines
from socket import *
s = socket()
s.listen()
c1 = socket()
c1.connect(s.getsockname())
c2, _ = s.accept()
c1.send(b'h', MSG_OOB)

Quoted 1 line

Quoted 1 line
Quoted 1 line
c1.send(b'ello')

Quoted 1 line

Quoted 1 line
Quoted 1 line
c2.recv(5, MSG_PEEK)

b'ello'
---8<---

Latest net.git
---8<---

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX)
c1.send(b'h', MSG_OOB)

Quoted 1 line

Quoted 1 line
Quoted 1 line
c1.send(b'ello')

Quoted 2 lines

Quoted 2 lines
Quoted 2 lines
c2.recv(5, MSG_PEEK)

^C
---8<---

314001f0bf92:
---8<---

Quoted 3 lines

Quoted 3 lines
Quoted 3 lines
from socket import *
c1, c2 = socketpair(AF_UNIX)
c1.send(b'h', MSG_OOB)

Quoted 1 line

Quoted 1 line
Quoted 1 line
c1.send(b'ello')

Quoted 1 line

Quoted 1 line
Quoted 1 line
c2.recv(5, MSG_PEEK)

b'hello'
---8<---

Quoted 3 lines

I have attached a patch that fixes this issue and other issues that I encountered in my testing. I compared TCP.

Could you post patches formally on top of the latest net.git ? It seems one of my patch is squashed.

Also, please note that one patch should fix one issue. The change in queue_oob() should be another patch.

Thanks!

Rao Shoaib

2024-04-16 21:34 UTC #6 lore raw

Re: [PATCH v2 net 2/2] af_unix: Don't peek OOB data without MSG_OOB.

Quoted 11 lines · Kuniyuki Iwashima

From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 13:11:09 -0700
Quoted 2 lines
The proposed fix is not the correct fix as among other things it does not allow going pass the OOB if data is present. TCP allows that.
Ugh, exactly.
But the behaviour was broken initially, so the tag is
Fixes 314001f0bf92 ("af_unix: Add OOB support")

Where is this requirement listed?

Quoted 2 lines

Could you post patches formally on top of the latest net.git ? It seems one of my patch is squashed.

I pulled in last night, your last fix has not yet made it (I think)

[rshoaib@turbo-2 linux_oob]$ git describe
v6.9-rc4-32-gbf541423b785

Quoted 4 lines

Also, please note that one patch should fix one issue. The change in queue_oob() should be another patch.

I was just responding to your email. I was not sure if you wanted to modify your fix. If you prefer I submit the patches, I will later.

Shoaib

Kuniyuki Iwashima

author

2024-04-16 21:48 UTC #7 lore raw

Re: [PATCH v2 net 2/2] af_unix: Don't peek OOB data without MSG_OOB.

From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 14:34:20 -0700

Quoted 6 lines · Kuniyuki Iwashima

Quoted 5 lines
From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 13:11:09 -0700
Quoted 2 lines
The proposed fix is not the correct fix as among other things it does not allow going pass the OOB if data is present. TCP allows that.

Quoted 8 lines

Quoted 6 lines
Ugh, exactly.
But the behaviour was broken initially, so the tag is
Fixes 314001f0bf92 ("af_unix: Add OOB support")
Where is this requirement listed?

Please start with these docs.
https://docs.kernel.org/process/submitting-patches.html
https://docs.kernel.org/process/maintainer-netdev.html

Quoted 9 lines

Quoted 4 lines
Could you post patches formally on top of the latest net.git ? It seems one of my patch is squashed.
I pulled in last night, your last fix has not yet made it (I think)
[rshoaib@turbo-2 linux_oob]$ git describe
v6.9-rc4-32-gbf541423b785

Probably you are using another git tree or branch.

Networking subsystem uses net.git for fixes and net-next.git for new features as written in the 2nd doc above.

My patch landed on 4 days ago at least.
https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=283454c8a123072e5c386a5a2b5fc576aa455b6f

Also you should receive this email.
https://lore.kernel.org/netdev/171297422982.31124.3409808601326947596.git-patchwork-notify@kernel.org/

Quoted 8 lines

Quoted 5 lines
Also, please note that one patch should fix one issue. The change in queue_oob() should be another patch.
I was just responding to your email. I was not sure if you wanted to modify your fix. If you prefer I submit the patches, I will later.

As I said, my fix is already in net.git, so you can post a separte patch based on net.git/main.

Rao Shoaib

2024-04-16 22:01 UTC #8 lore raw

Re: [PATCH v2 net 2/2] af_unix: Don't peek OOB data without MSG_OOB.

Quoted 12 lines · Kuniyuki Iwashima

From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 14:34:20 -0700
Quoted 10 lines · Kuniyuki Iwashima
Quoted 9 lines
From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 13:11:09 -0700
Quoted 2 lines
The proposed fix is not the correct fix as among other things it does not allow going pass the OOB if data is present. TCP allows that.
Ugh, exactly.
But the behaviour was broken initially, so the tag is

Quoted 10 lines

Quoted 4 lines
Quoted 2 lines
Fixes 314001f0bf92 ("af_unix: Add OOB support")
Where is this requirement listed?
Please start with these docs.
https://urldefense.com/v3/__https://docs.kernel.org/process/submitting-patches.html__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUA5Yikc2A$
https://urldefense.com/v3/__https://docs.kernel.org/process/maintainer-netdev.html__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUAdoz3l7w$

That is a suggestion. I see commits in even af_unix.c which do not follow that convention. They just mention what the fix is about. In this case it is implied.

I am not opposed specifying it but it seems it's optional.

Quoted 28 lines

Quoted 9 lines
Quoted 4 lines
Could you post patches formally on top of the latest net.git ? It seems one of my patch is squashed.
I pulled in last night, your last fix has not yet made it (I think)
[rshoaib@turbo-2 linux_oob]$ git describe
v6.9-rc4-32-gbf541423b785
Probably you are using another git tree or branch.
Networking subsystem uses net.git for fixes and net-next.git for new features as written in the 2nd doc above.
My patch landed on 4 days ago at least.
https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=283454c8a123072e5c386a5a2b5fc576aa455b6f__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUA32gMtng$
Also you should receive this email.
https://urldefense.com/v3/__https://lore.kernel.org/netdev/171297422982.31124.3409808601326947596.git-patchwork-notify@kernel.org/__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUAnOykbCY$
Quoted 6 lines
Quoted 5 lines
Also, please note that one patch should fix one issue. The change in queue_oob() should be another patch.

Quoted 6 lines

Quoted 2 lines
I was just responding to your email. I was not sure if you wanted to modify your fix. If you prefer I submit the patches, I will later.
As I said, my fix is already in net.git, so you can post a separte patch based on net.git/main.

I used the latest from Linus.
I will submit the patches later.

Shoaib

Kuniyuki Iwashima

author

2024-04-16 22:11 UTC #9 lore raw

Re: [PATCH v2 net 2/2] af_unix: Don't peek OOB data without MSG_OOB.

From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 15:01:15 -0700

Quoted 23 lines · Kuniyuki Iwashima

Quoted 12 lines
From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 14:34:20 -0700
Quoted 10 lines · Kuniyuki Iwashima
Quoted 9 lines
From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 13:11:09 -0700
Quoted 2 lines
The proposed fix is not the correct fix as among other things it does not allow going pass the OOB if data is present. TCP allows that.
Ugh, exactly.
But the behaviour was broken initially, so the tag is
Quoted 10 lines
Quoted 4 lines
Quoted 2 lines
Fixes 314001f0bf92 ("af_unix: Add OOB support")
Where is this requirement listed?
Please start with these docs.
https://urldefense.com/v3/__https://docs.kernel.org/process/submitting-patches.html__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUA5Yikc2A$
https://urldefense.com/v3/__https://docs.kernel.org/process/maintainer-netdev.html__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUAdoz3l7w$

Quoted 5 lines

That is a suggestion. I see commits in even af_unix.c which do not follow that convention. They just mention what the fix is about. In this case it is implied.
I am not opposed specifying it but it seems it's optional.

You want to read the 2nd doc.

1.1 tl;dr
for fixes the Fixes: tag is required, regardless of the tree

Rao Shoaib

2024-04-17 04:38 UTC #10 lore raw

Re: [PATCH v2 net 2/2] af_unix: Don't peek OOB data without MSG_OOB.

Quoted 29 lines · Kuniyuki Iwashima

From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 15:01:15 -0700
Quoted 27 lines · Kuniyuki Iwashima
Quoted 12 lines
From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 14:34:20 -0700
Quoted 10 lines · Kuniyuki Iwashima
Quoted 9 lines
From: Rao Shoaib <[email protected]>
Date: Tue, 16 Apr 2024 13:11:09 -0700
Quoted 2 lines
The proposed fix is not the correct fix as among other things it does not allow going pass the OOB if data is present. TCP allows that.
Ugh, exactly.
But the behaviour was broken initially, so the tag is
Quoted 10 lines
Quoted 4 lines
Quoted 2 lines
Fixes 314001f0bf92 ("af_unix: Add OOB support")
Where is this requirement listed?
Please start with these docs.
https://urldefense.com/v3/__https://docs.kernel.org/process/submitting-patches.html__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUA5Yikc2A$
https://urldefense.com/v3/__https://docs.kernel.org/process/maintainer-netdev.html__;!!ACWV5N9M2RV99hQ!PswtQoZm7r5MGnH8pv3OewI_PvmSRJb29YcA0pnVOzuu8T3xvWlw4lLlLzFhzn6uO2lo0bUAdoz3l7w$
That is a suggestion. I see commits in even af_unix.c which do not follow that convention. They just mention what the fix is about. In this case it is implied.

Quoted 6 lines

Quoted 1 line
I am not opposed specifying it but it seems it's optional.
You want to read the 2nd doc.
1.1 tl;dr
for fixes the Fixes: tag is required, regardless of the tree

Thanks will do.

Shoaib

[email protected]

CI Applied

2024-04-13 02:10 UTC #11 lore raw

In reply to Kuniyuki Iwashima

This series was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 9 lines · On Wed, 10 Apr 2024 10:10:14 -0700 you wrote:

Currently, OOB data can be read without MSG_OOB accidentally in two cases, and this seris fixes the bugs.
Changes:
v2:
Drop patch 3
[...]

Here is the summary with links:
- [v2,net,1/2] af_unix: Call manage_oob() for every skb in unix_stream_read_generic().

    https://git.kernel.org/netdev/net/c/283454c8a123
  - [v2,net,2/2] af_unix: Don't peek OOB data without MSG_OOB.
    https://git.kernel.org/netdev/net/c/22dd70eb2c3d

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html