| Message ID | 20240409225209.58102-4-kuniyu@amazon.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | af_unix: Fix MSG_OOB bugs and prepare deprecation. | expand |
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
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.
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
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.
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. >
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
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
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(-)