[PATCH v2 net 0/2] af_unix: Fix MSG_OOB bugs with MSG_PEEK.

[PATCH v2 net 0/2] af_unix: Fix MSG_OOB bugs with MSG_PEEK.

[Kuniyuki Iwashima]

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/20240409225209.58102-1-kuniyu@amazon.com/


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(-)

-- 
2.30.2

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

[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:

  >>> from socket import *
  >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
  >>> c1.send(b'hello', MSG_OOB)
  5
  >>> 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:

  >>> from socket import *
  >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
  >>> c1.send(b'hello', MSG_OOB)
  5
  >>> c2.recv(5, MSG_PEEK)
  b'hell'

Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index d032eb5fa6df..f297320438bf 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -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;
-- 
2.30.2

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

[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.

  >>> from socket import *
  >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
  >>> c1.send(b'a', MSG_OOB)
  1
  >>> 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:

  >>> from socket import *
  >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
  >>> c1.send(b'a', MSG_OOB)
  1
  >>> 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 <kuniyu@amazon.com>
---
 net/unix/af_unix.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index f297320438bf..9a6ad5974dff 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -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
-- 
2.30.2

Re: [PATCH v2 net 0/2] af_unix: Fix MSG_OOB bugs with MSG_PEEK.

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

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!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

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

[Rao Shoaib]

On 4/10/24 10:10, Kuniyuki Iwashima wrote:
> 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:
> 
>    >>> from socket import *
>    >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
>    >>> c1.send(b'hello', MSG_OOB)
>    5
>    >>> 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:
> 
>    >>> from socket import *
>    >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
>    >>> c1.send(b'hello', MSG_OOB)
>    5
>    >>> c2.recv(5, MSG_PEEK)
>    b'hell'
> 
> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
>   net/unix/af_unix.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index d032eb5fa6df..f297320438bf 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -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;

Looks Good.

Reviewed-by: Rao shoaib <rao.shoaib@oracle.com>

Shoaib

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

[Rao Shoaib]

[-- Attachment #1: Type: text/plain, Size: 2624 bytes --]

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

On 4/10/24 10:10, Kuniyuki Iwashima wrote:
> 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.
> 
>    >>> from socket import *
>    >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
>    >>> c1.send(b'a', MSG_OOB)
>    1
>    >>> 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:
> 
>    >>> from socket import *
>    >>> c1, c2 = socketpair(AF_UNIX, SOCK_STREAM)
>    >>> c1.send(b'a', MSG_OOB)
>    1
>    >>> 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 <kuniyu@amazon.com>
> ---
>   net/unix/af_unix.c | 10 +++++-----
>   1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index f297320438bf..9a6ad5974dff 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -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

[-- Attachment #2: 0001-AF_UNIX-Fix-handling-of-OOB-Data.patch --]
[-- Type: text/x-patch, Size: 2772 bytes --]

From bf541423b785ae14ee2c7015d96dba6ac368358c Mon Sep 17 00:00:00 2001
From: Rao Shoaib <Rao.Shoaib@oracle.com>
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 <Rao.Shoaib@oracle.com>
---
 net/unix/af_unix.c | 32 ++++++++++++++++++++------------
 1 file changed, 20 insertions(+), 12 deletions(-)

diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index d032eb5fa6df..50cae03a1f98 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -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;
-- 
2.39.3

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

[Kuniyuki Iwashima]

From: Rao Shoaib <rao.shoaib@oracle.com>
Date: Tue, 16 Apr 2024 13:11:09 -0700
> 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<---
>>> from socket import *
>>> s = socket()
>>> s.listen()
>>> c1 = socket()
>>> c1.connect(s.getsockname())
>>> c2, _ = s.accept()
>>> 
>>> c1.send(b'h', MSG_OOB)
1
>>> c1.send(b'ello')
4
>>> c2.recv(5, MSG_PEEK)
b'ello'
---8<---

Latest net.git
---8<---
>>> from socket import *
>>> c1, c2 = socketpair(AF_UNIX)
>>> c1.send(b'h', MSG_OOB)
1
>>> c1.send(b'ello')
4
>>> 
>>> c2.recv(5, MSG_PEEK)
^C
---8<---

314001f0bf92:
---8<---
>>> from socket import *
>>> c1, c2 = socketpair(AF_UNIX)
>>> c1.send(b'h', MSG_OOB)
1
>>> c1.send(b'ello')
4
>>> c2.recv(5, MSG_PEEK)
b'hello'
---8<---


> 
> 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!

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

[Rao Shoaib]

On 4/16/24 13:51, Kuniyuki Iwashima wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> Date: Tue, 16 Apr 2024 13:11:09 -0700
>> 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?


> 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

> 
> 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

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

[Kuniyuki Iwashima]

From: Rao Shoaib <rao.shoaib@oracle.com>
Date: Tue, 16 Apr 2024 14:34:20 -0700
> On 4/16/24 13:51, Kuniyuki Iwashima wrote:
> > From: Rao Shoaib <rao.shoaib@oracle.com>
> > Date: Tue, 16 Apr 2024 13:11:09 -0700
> >> 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?

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


> 
> 
> > 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/


> 
> > 
> > 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.

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

[Rao Shoaib]

On 4/16/24 14:47, Kuniyuki Iwashima wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> Date: Tue, 16 Apr 2024 14:34:20 -0700
>> On 4/16/24 13:51, Kuniyuki Iwashima wrote:
>>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>> Date: Tue, 16 Apr 2024 13:11:09 -0700
>>>> 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?
> 
> 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.


>>
>>
>>> 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$
> 
> 
>>
>>>
>>> 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.
> 
I used the latest from Linus.
I will submit the patches later.

Shoaib

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

[Kuniyuki Iwashima]

From: Rao Shoaib <rao.shoaib@oracle.com>
Date: Tue, 16 Apr 2024 15:01:15 -0700
> On 4/16/24 14:47, Kuniyuki Iwashima wrote:
> > From: Rao Shoaib <rao.shoaib@oracle.com>
> > Date: Tue, 16 Apr 2024 14:34:20 -0700
> >> On 4/16/24 13:51, Kuniyuki Iwashima wrote:
> >>> From: Rao Shoaib <rao.shoaib@oracle.com>
> >>> Date: Tue, 16 Apr 2024 13:11:09 -0700
> >>>> 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?
> > 
> > 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.

You want to read the 2nd doc.

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

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

[Rao Shoaib]

On 4/16/24 15:10, Kuniyuki Iwashima wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> Date: Tue, 16 Apr 2024 15:01:15 -0700
>> On 4/16/24 14:47, Kuniyuki Iwashima wrote:
>>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>> Date: Tue, 16 Apr 2024 14:34:20 -0700
>>>> On 4/16/24 13:51, Kuniyuki Iwashima wrote:
>>>>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>>>> Date: Tue, 16 Apr 2024 13:11:09 -0700
>>>>>> 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?
>>>
>>> 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.
> 
> 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