All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v1 net 0/3] af_unix: Fix MSG_OOB bugs and prepare deprecation.
@ 2024-04-09 22:52 Kuniyuki Iwashima
  2024-04-09 22:52 ` [PATCH v1 net 1/3] af_unix: Call manage_oob() for every skb in unix_stream_read_generic() Kuniyuki Iwashima
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2024-04-09 22:52 UTC (permalink / raw
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Rao shoaib, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

Currently, OOB data can be read without MSG_OOB accidentally in two cases.

This series fixes 2 bugs, switch the default of CONFIG_AF_UNIX_OOB to n,
and add warning to deprecate MSG_OOB support in the near future.

The last patch can be dropped if not needed.


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

 net/unix/Kconfig   |  4 ++--
 net/unix/af_unix.c | 14 ++++++++------
 2 files changed, 10 insertions(+), 8 deletions(-)

-- 
2.30.2


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v1 net 1/3] af_unix: Call manage_oob() for every skb in unix_stream_read_generic().
  2024-04-09 22:52 [PATCH v1 net 0/3] af_unix: Fix MSG_OOB bugs and prepare deprecation Kuniyuki Iwashima
@ 2024-04-09 22:52 ` Kuniyuki Iwashima
  2024-04-09 22:52 ` [PATCH v1 net 2/3] af_unix: Don't peek OOB data without MSG_OOB Kuniyuki Iwashima
  2024-04-09 22:52 ` [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation Kuniyuki Iwashima
  2 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2024-04-09 22:52 UTC (permalink / raw
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Rao shoaib, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 net 2/3] af_unix: Don't peek OOB data without MSG_OOB.
  2024-04-09 22:52 [PATCH v1 net 0/3] af_unix: Fix MSG_OOB bugs and prepare deprecation Kuniyuki Iwashima
  2024-04-09 22:52 ` [PATCH v1 net 1/3] af_unix: Call manage_oob() for every skb in unix_stream_read_generic() Kuniyuki Iwashima
@ 2024-04-09 22:52 ` Kuniyuki Iwashima
  2024-04-09 22:52 ` [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation Kuniyuki Iwashima
  2 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2024-04-09 22:52 UTC (permalink / raw
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Rao shoaib, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

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


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation.
  2024-04-09 22:52 [PATCH v1 net 0/3] af_unix: Fix MSG_OOB bugs and prepare deprecation Kuniyuki Iwashima
  2024-04-09 22:52 ` [PATCH v1 net 1/3] af_unix: Call manage_oob() for every skb in unix_stream_read_generic() Kuniyuki Iwashima
  2024-04-09 22:52 ` [PATCH v1 net 2/3] af_unix: Don't peek OOB data without MSG_OOB Kuniyuki Iwashima
@ 2024-04-09 22:52 ` Kuniyuki Iwashima
  2024-04-10  0:09   ` Rao Shoaib
  2 siblings, 1 reply; 10+ messages in thread
From: Kuniyuki Iwashima @ 2024-04-09 22:52 UTC (permalink / raw
  To: David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
  Cc: Rao shoaib, Kuniyuki Iwashima, Kuniyuki Iwashima, netdev

Commit 314001f0bf92 ("af_unix: Add OOB support") introduced MSG_OOB
support for AF_UNIX, and it's about 3 years ago.  Since then, MSG_OOB
is the playground for syzbot.

MSG_OOB support is guarded with CONFIG_AF_UNIX_OOB, but it's enabled
by default and cannot be disabled without editing .config manually
because of the lack of prompt.

We recently found 3 wrong behaviours with basic functionality that
no one have noticed for 3 years, so it seems there is no real user
and even the author is not using OOB feature.  [0]

This is a good opportunity to drop MSG_OOB support.

Let's switch the default config to n and add warning so that someone
using MSG_OOB in a real workload can notice it before MSG_OOB support
is removed completely.

Link: https://lore.kernel.org/netdev/472044aa-4427-40f0-9b9a-bce75d5c8aac@oracle.com/ [0]
Fixes: 314001f0bf92 ("af_unix: Add OOB support")
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Added Fixes tag so that it can be backported to corresponding stable
kernels.
---
 net/unix/Kconfig   | 4 ++--
 net/unix/af_unix.c | 2 ++
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/unix/Kconfig b/net/unix/Kconfig
index 8b5d04210d7c..9d9270fdc1fe 100644
--- a/net/unix/Kconfig
+++ b/net/unix/Kconfig
@@ -17,9 +17,9 @@ config UNIX
 	  Say Y unless you know what you are doing.
 
 config	AF_UNIX_OOB
-	bool
+	bool "Unix MSG_OOB support"
 	depends on UNIX
-	default y
+	default n
 
 config UNIX_DIAG
 	tristate "UNIX: socket monitoring interface"
diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
index 9a6ad5974dff..fecca27aa77f 100644
--- a/net/unix/af_unix.c
+++ b/net/unix/af_unix.c
@@ -2253,6 +2253,8 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
 	err = -EOPNOTSUPP;
 	if (msg->msg_flags & MSG_OOB) {
 #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
+		pr_warn_once("MSG_OOB support will be removed in 2025.\n");
+
 		if (len)
 			len--;
 		else
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation.
  2024-04-09 22:52 ` [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation Kuniyuki Iwashima
@ 2024-04-10  0:09   ` Rao Shoaib
  2024-04-10  0:27     ` Kuniyuki Iwashima
  0 siblings, 1 reply; 10+ messages in thread
From: Rao Shoaib @ 2024-04-10  0:09 UTC (permalink / raw
  To: Kuniyuki Iwashima, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Kuniyuki Iwashima, netdev

This feature was added because it was needed by Oracle products. The 
bugs found are corner cases and happen with new feature, at the time all 
tests passed. If you do not feel like fixing these bugs that is fine, 
let me know and I will address them, but removing the feature completely 
should not be an option.

Plus Amazon has it's own closed/proprietary distribution. If this is an 
issue please configure your repo to not include this feature. Many 
distributions choose not to include several features.

Shoaib

On 4/9/24 15:52, Kuniyuki Iwashima wrote:
> Commit 314001f0bf92 ("af_unix: Add OOB support") introduced MSG_OOB
> support for AF_UNIX, and it's about 3 years ago.  Since then, MSG_OOB
> is the playground for syzbot.
> 
> MSG_OOB support is guarded with CONFIG_AF_UNIX_OOB, but it's enabled
> by default and cannot be disabled without editing .config manually
> because of the lack of prompt.
> 
> We recently found 3 wrong behaviours with basic functionality that
> no one have noticed for 3 years, so it seems there is no real user
> and even the author is not using OOB feature.  [0]
> 
> This is a good opportunity to drop MSG_OOB support.
> 
> Let's switch the default config to n and add warning so that someone
> using MSG_OOB in a real workload can notice it before MSG_OOB support
> is removed completely.
> 
> Link: https://urldefense.com/v3/__https://lore.kernel.org/netdev/472044aa-4427-40f0-9b9a-bce75d5c8aac@oracle.com/__;!!ACWV5N9M2RV99hQ!M7skvfZ7iV_Wz5V4lcoDCSabTe02sk-cpFNYB5WNcgszkzbp3hHoasDagxKSqLdcBtgZ_ckaf5-RBE4$  [0]
> Fixes: 314001f0bf92 ("af_unix: Add OOB support")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> Added Fixes tag so that it can be backported to corresponding stable
> kernels.
> ---
>   net/unix/Kconfig   | 4 ++--
>   net/unix/af_unix.c | 2 ++
>   2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/net/unix/Kconfig b/net/unix/Kconfig
> index 8b5d04210d7c..9d9270fdc1fe 100644
> --- a/net/unix/Kconfig
> +++ b/net/unix/Kconfig
> @@ -17,9 +17,9 @@ config UNIX
>   	  Say Y unless you know what you are doing.
>   
>   config	AF_UNIX_OOB
> -	bool
> +	bool "Unix MSG_OOB support"
>   	depends on UNIX
> -	default y
> +	default n
>   
>   config UNIX_DIAG
>   	tristate "UNIX: socket monitoring interface"
> diff --git a/net/unix/af_unix.c b/net/unix/af_unix.c
> index 9a6ad5974dff..fecca27aa77f 100644
> --- a/net/unix/af_unix.c
> +++ b/net/unix/af_unix.c
> @@ -2253,6 +2253,8 @@ static int unix_stream_sendmsg(struct socket *sock, struct msghdr *msg,
>   	err = -EOPNOTSUPP;
>   	if (msg->msg_flags & MSG_OOB) {
>   #if IS_ENABLED(CONFIG_AF_UNIX_OOB)
> +		pr_warn_once("MSG_OOB support will be removed in 2025.\n");
> +
>   		if (len)
>   			len--;
>   		else

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation.
  2024-04-10  0:09   ` Rao Shoaib
@ 2024-04-10  0:27     ` Kuniyuki Iwashima
  2024-04-10  0:48       ` Rao Shoaib
  0 siblings, 1 reply; 10+ messages in thread
From: Kuniyuki Iwashima @ 2024-04-10  0:27 UTC (permalink / raw
  To: rao.shoaib; +Cc: davem, edumazet, kuba, kuni1840, kuniyu, netdev, pabeni

From: Rao Shoaib <rao.shoaib@oracle.com>
Date: Tue, 9 Apr 2024 17:09:24 -0700
> This feature was added because it was needed by Oracle products.

I know.  What's about now ?

I just took the silence as no here.
https://lore.kernel.org/netdev/472044aa-4427-40f0-9b9a-bce75d5c8aac@oracle.com/

As I noted in the cover letter, I'm fine to drop this patch if there's
a real user.


> The 
> bugs found are corner cases and happen with new feature, at the time all 
> tests passed.

Yes, but the test was not sufficient.


> If you do not feel like fixing these bugs that is fine, 
> let me know and I will address them,

Please do even if I don't let you know.


> but removing the feature completely 
> should not be an option.
> 
> Plus Amazon has it's own closed/proprietary distribution. If this is an 
> issue please configure your repo to not include this feature. Many 
> distributions choose not to include several features.

The problem is that the buggy feature risks many distributions.
If not-well-maintained feature is really needed only for a single
distro, it should be rather maintained as downstream patch.

If no one is using it, no reason to keep the attack sarface alive.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation.
  2024-04-10  0:27     ` Kuniyuki Iwashima
@ 2024-04-10  0:48       ` Rao Shoaib
  2024-04-10  6:01         ` Kuniyuki Iwashima
  0 siblings, 1 reply; 10+ messages in thread
From: Rao Shoaib @ 2024-04-10  0:48 UTC (permalink / raw
  To: Kuniyuki Iwashima; +Cc: davem, edumazet, kuba, kuni1840, netdev, pabeni



On 4/9/24 17:27, Kuniyuki Iwashima wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> Date: Tue, 9 Apr 2024 17:09:24 -0700
>> This feature was added because it was needed by Oracle products.
> 
> I know.  What's about now ?
> 
> I just took the silence as no here.
> https://urldefense.com/v3/__https://lore.kernel.org/netdev/472044aa-4427-40f0-9b9a-bce75d5c8aac@oracle.com/__;!!ACWV5N9M2RV99hQ!Nk1WvCk4-rstASn7PUW4QiAejf0gQ7ktNz-AhuB2UHt9Vx7yUVcfcJ82f9XM3tsDanwnWusycGdUfF4$
> 
> As I noted in the cover letter, I'm fine to drop this patch if there's
> a real user.
> 
> 
>> The
>> bugs found are corner cases and happen with new feature, at the time all
>> tests passed.
> 
> Yes, but the test was not sufficient.
> 

Yes they were not but we ran the tests that were required and available.
If bugs are found later we are responsible for fixing them and we will.

> 
>> If you do not feel like fixing these bugs that is fine,
>> let me know and I will address them,
> 
> Please do even if I don't let you know.
> 

The way we use it we have not run into these unusual test cases. If you 
or anyone runs into any bugs please report and I personally will debug 
and fix the issue, just like open source is suppose to work.

> 
>> but removing the feature completely
>> should not be an option.
>>
>> Plus Amazon has it's own closed/proprietary distribution. If this is an
>> issue please configure your repo to not include this feature. Many
>> distributions choose not to include several features.
> 
> The problem is that the buggy feature risks many distributions.
> If not-well-maintained feature is really needed only for a single
> distro, it should be rather maintained as downstream patch.
> 
> If no one is using it, no reason to keep the attack sarface alive.

Tell me one feature in Linux that does not have bugs?
The feature if used normally works just fine, the bugs that have been 
found do not cause any stability issue, may be functional issue at best. 
How many applications do you know use MSG_PEEK that these tests are 
exploiting.

Plus if it is annoying to you just remove the feature from your private 
distribution and let the others decide for them selves.

Shoaib

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation.
  2024-04-10  0:48       ` Rao Shoaib
@ 2024-04-10  6:01         ` Kuniyuki Iwashima
  2024-04-10  8:36           ` Rao Shoaib
  0 siblings, 1 reply; 10+ messages in thread
From: Kuniyuki Iwashima @ 2024-04-10  6:01 UTC (permalink / raw
  To: rao.shoaib; +Cc: davem, edumazet, kuba, kuni1840, kuniyu, netdev, pabeni

From: Rao Shoaib <rao.shoaib@oracle.com>
Date: Tue, 9 Apr 2024 17:48:37 -0700
> On 4/9/24 17:27, Kuniyuki Iwashima wrote:
> > From: Rao Shoaib <rao.shoaib@oracle.com>
> > Date: Tue, 9 Apr 2024 17:09:24 -0700
> >> This feature was added because it was needed by Oracle products.
> > 
> > I know.  What's about now ?

Why do you ingore this again ?

If it's really used in Oracle products, you can just say yes,
but it seems no ?


> > 
> > I just took the silence as no here.
> > https://urldefense.com/v3/__https://lore.kernel.org/netdev/472044aa-4427-40f0-9b9a-bce75d5c8aac@oracle.com/__;!!ACWV5N9M2RV99hQ!Nk1WvCk4-rstASn7PUW4QiAejf0gQ7ktNz-AhuB2UHt9Vx7yUVcfcJ82f9XM3tsDanwnWusycGdUfF4$
> > 
> > As I noted in the cover letter, I'm fine to drop this patch if there's
> > a real user.
> > 
> > 
> >> The
> >> bugs found are corner cases and happen with new feature, at the time all
> >> tests passed.
> > 
> > Yes, but the test was not sufficient.
> > 
> 
> Yes they were not but we ran the tests that were required and available.
> If bugs are found later we are responsible for fixing them and we will.

This is nice,


> 
> > 
> >> If you do not feel like fixing these bugs that is fine,
> >> let me know and I will address them,
> > 
> > Please do even if I don't let you know.
> > 
> 
> The way we use it we have not run into these unusual test cases. If you 
> or anyone runs into any bugs please report and I personally will debug 
> and fix the issue, just like open source is suppose to work.

but why personally ?  because Oracle products no longer use it ?
If so, why do you want to keep the feature with no user ?


> > 
> >> but removing the feature completely
> >> should not be an option.
> >>
> >> Plus Amazon has it's own closed/proprietary distribution. If this is an
> >> issue please configure your repo to not include this feature. Many
> >> distributions choose not to include several features.
> > 
> > The problem is that the buggy feature risks many distributions.
> > If not-well-maintained feature is really needed only for a single
> > distro, it should be rather maintained as downstream patch.
> > 
> > If no one is using it, no reason to keep the attack sarface alive.
> 
> Tell me one feature in Linux that does not have bugs?

I'm not talking about features with no bug.  It's fine to have bugs
if it's maintained and fixed in timely manner.

I'm talking about a feature with bugs that seems not to be used by
anyone nor maintained.


> The feature if used normally works just fine, the bugs that have been 
> found do not cause any stability issue, may be functional issue at best.

It caused memory leaks in some ways easily without admin privilege.


> How many applications do you know use MSG_PEEK that these tests are 
> exploiting.

Security is not that way of thinking.  Even when the bug is triggered
with unusual sequence of calls, it must be fixed, especially on a host
that could execute untrusted code.


> 
> Plus if it is annoying to you just remove the feature from your private 
> distribution and let the others decide for them selves.

If no one uses the feature that has bugs without maintenance,
it's natural to deprecate it.  Then, no one need to be burdened
by unnecessary bug fixes.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation.
  2024-04-10  6:01         ` Kuniyuki Iwashima
@ 2024-04-10  8:36           ` Rao Shoaib
  2024-04-10 16:52             ` Kuniyuki Iwashima
  0 siblings, 1 reply; 10+ messages in thread
From: Rao Shoaib @ 2024-04-10  8:36 UTC (permalink / raw
  To: Kuniyuki Iwashima; +Cc: davem, edumazet, kuba, kuni1840, netdev, pabeni

It is used by Oracle products. File bugs and someone from Oracle will 
fix it (most likely me). Oracle has addressed any bugs reported in a 
very timely manner. So in summary the feature is being used and is 
actively maintained.

You can also turn off the feature in your private/closed distro and not 
worry about it.

That is all I have to say on this subject.

Shoaib

On 4/9/24 23:01, Kuniyuki Iwashima wrote:
> From: Rao Shoaib <rao.shoaib@oracle.com>
> Date: Tue, 9 Apr 2024 17:48:37 -0700
>> On 4/9/24 17:27, Kuniyuki Iwashima wrote:
>>> From: Rao Shoaib <rao.shoaib@oracle.com>
>>> Date: Tue, 9 Apr 2024 17:09:24 -0700
>>>> This feature was added because it was needed by Oracle products.
>>>
>>> I know.  What's about now ?
> 
> Why do you ingore this again ?
> 
> If it's really used in Oracle products, you can just say yes,
> but it seems no ?
> 
> 
>>>
>>> I just took the silence as no here.
>>> https://urldefense.com/v3/__https://lore.kernel.org/netdev/472044aa-4427-40f0-9b9a-bce75d5c8aac@oracle.com/__;!!ACWV5N9M2RV99hQ!Nk1WvCk4-rstASn7PUW4QiAejf0gQ7ktNz-AhuB2UHt9Vx7yUVcfcJ82f9XM3tsDanwnWusycGdUfF4$
>>>
>>> As I noted in the cover letter, I'm fine to drop this patch if there's
>>> a real user.
>>>
>>>
>>>> The
>>>> bugs found are corner cases and happen with new feature, at the time all
>>>> tests passed.
>>>
>>> Yes, but the test was not sufficient.
>>>
>>
>> Yes they were not but we ran the tests that were required and available.
>> If bugs are found later we are responsible for fixing them and we will.
> 
> This is nice,
> 
> 
>>
>>>
>>>> If you do not feel like fixing these bugs that is fine,
>>>> let me know and I will address them,
>>>
>>> Please do even if I don't let you know.
>>>
>>
>> The way we use it we have not run into these unusual test cases. If you
>> or anyone runs into any bugs please report and I personally will debug
>> and fix the issue, just like open source is suppose to work.
> 
> but why personally ?  because Oracle products no longer use it ?
> If so, why do you want to keep the feature with no user ?
> 
> 
>>>
>>>> but removing the feature completely
>>>> should not be an option.
>>>>
>>>> Plus Amazon has it's own closed/proprietary distribution. If this is an
>>>> issue please configure your repo to not include this feature. Many
>>>> distributions choose not to include several features.
>>>
>>> The problem is that the buggy feature risks many distributions.
>>> If not-well-maintained feature is really needed only for a single
>>> distro, it should be rather maintained as downstream patch.
>>>
>>> If no one is using it, no reason to keep the attack sarface alive.
>>
>> Tell me one feature in Linux that does not have bugs?
> 
> I'm not talking about features with no bug.  It's fine to have bugs
> if it's maintained and fixed in timely manner.
> 
> I'm talking about a feature with bugs that seems not to be used by
> anyone nor maintained.
> 
> 
>> The feature if used normally works just fine, the bugs that have been
>> found do not cause any stability issue, may be functional issue at best.
> 
> It caused memory leaks in some ways easily without admin privilege.
> 
> 
>> How many applications do you know use MSG_PEEK that these tests are
>> exploiting.
> 
> Security is not that way of thinking.  Even when the bug is triggered
> with unusual sequence of calls, it must be fixed, especially on a host
> that could execute untrusted code.
> 
> 
>>
>> Plus if it is annoying to you just remove the feature from your private
>> distribution and let the others decide for them selves.
> 
> If no one uses the feature that has bugs without maintenance,
> it's natural to deprecate it.  Then, no one need to be burdened
> by unnecessary bug fixes.
> 

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation.
  2024-04-10  8:36           ` Rao Shoaib
@ 2024-04-10 16:52             ` Kuniyuki Iwashima
  0 siblings, 0 replies; 10+ messages in thread
From: Kuniyuki Iwashima @ 2024-04-10 16:52 UTC (permalink / raw
  To: rao.shoaib; +Cc: davem, edumazet, kuba, kuni1840, kuniyu, netdev, pabeni

From: Rao Shoaib <rao.shoaib@oracle.com>
Date: Wed, 10 Apr 2024 01:36:01 -0700
> It is used by Oracle products.

This is what I wanted to know, and then I'm fine to respin v2
without this patch.

Thank you.


> File bugs and someone from Oracle will 
> fix it (most likely me). Oracle has addressed any bugs reported in a 
> very timely manner. So in summary the feature is being used and is 
> actively maintained.
>
> You can also turn off the feature in your private/closed distro and not 
> worry about it.
> 
> That is all I have to say on this subject.
> 
> Shoaib

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2024-04-10 16:53 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-09 22:52 [PATCH v1 net 0/3] af_unix: Fix MSG_OOB bugs and prepare deprecation Kuniyuki Iwashima
2024-04-09 22:52 ` [PATCH v1 net 1/3] af_unix: Call manage_oob() for every skb in unix_stream_read_generic() Kuniyuki Iwashima
2024-04-09 22:52 ` [PATCH v1 net 2/3] af_unix: Don't peek OOB data without MSG_OOB Kuniyuki Iwashima
2024-04-09 22:52 ` [PATCH v1 net 3/3] af_unix: Prepare MSG_OOB deprecation Kuniyuki Iwashima
2024-04-10  0:09   ` Rao Shoaib
2024-04-10  0:27     ` Kuniyuki Iwashima
2024-04-10  0:48       ` Rao Shoaib
2024-04-10  6:01         ` Kuniyuki Iwashima
2024-04-10  8:36           ` Rao Shoaib
2024-04-10 16:52             ` Kuniyuki Iwashima

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.