[v1] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

[RESEND PATCH] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

Posted by André Draszik 2 years, 5 months ago

From: André Draszik <andre.draszik@linaro.org>

This reverts commit 3066ff93476c35679cb07a97cce37d9bb07632ff.

This patch breaks all existing userspace by requiring updates as
mentioned in the commit message, which is not allowed.

Revert to restore compatibility with existing userspace
implementations.

Cc: Bernd Schubert <bschubert@ddn.com>
Cc: Miklos Szeredi <mszeredi@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: André Draszik <andre.draszik@linaro.org>

---
resend because of missing people in Cc
---
 fs/fuse/inode.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 549358ffea8b..0b966b0e0962 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1132,10 +1132,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 		process_init_limits(fc, arg);
 
 		if (arg->minor >= 6) {
-			u64 flags = arg->flags;
-
-			if (flags & FUSE_INIT_EXT)
-				flags |= (u64) arg->flags2 << 32;
+			u64 flags = arg->flags | (u64) arg->flags2 << 32;
 
 			ra_pages = arg->max_readahead / PAGE_SIZE;
 			if (flags & FUSE_ASYNC_READ)
-- 
2.40.1

Re: [RESEND PATCH] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

Posted by Linux regression tracking #adding (Thorsten Leemhuis) 2 years, 3 months ago

[TLDR: I'm adding this report to the list of tracked Linux kernel
regressions; the text you find below is based on a few templates
paragraphs you might have encountered already in similar form.
See link in footer if these mails annoy you.]

On 04.09.23 15:33, André Draszik wrote:
> From: André Draszik <andre.draszik@linaro.org>
> 
> This reverts commit 3066ff93476c35679cb07a97cce37d9bb07632ff.
> 
> This patch breaks all existing userspace by requiring updates as
> mentioned in the commit message, which is not allowed.
> 
> Revert to restore compatibility with existing userspace
> implementations.

Thanks for the report. To be sure the issue doesn't fall through the
cracks unnoticed, I'm adding it to regzbot, the Linux kernel regression
tracking bot:

#regzbot ^introduced 3066ff93476c35679cb07a97cce37d9bb07632
#regzbot title fuse: creating new files, or reading existing files in i
Android, now returns -EFAULT
#regzbot ignore-activity

This isn't a regression? This issue or a fix for it are already
discussed somewhere else? It was fixed already? You want to clarify when
the regression started to happen? Or point out I got the title or
something else totally wrong? Then just reply and tell me -- ideally
while also telling regzbot about it, as explained by the page listed in
the footer of this mail.

Developers: When fixing the issue, remember to add 'Link:' tags pointing
to the report (the parent of this mail). See page linked in footer for
details.

Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat)
--
Everything you wanna know about Linux kernel regression tracking:
https://linux-regtracking.leemhuis.info/about/#tldr
That page also explains what to do if mails like this annoy you.

Re: [RESEND PATCH] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

Posted by Miklos Szeredi 2 years, 5 months ago

On Mon, 4 Sept 2023 at 15:34, André Draszik <git@andred.net> wrote:
>
> From: André Draszik <andre.draszik@linaro.org>
>
> This reverts commit 3066ff93476c35679cb07a97cce37d9bb07632ff.
>
> This patch breaks all existing userspace by requiring updates as
> mentioned in the commit message, which is not allowed.

It might break something, but you need to tell us what that is, please.

Thanks,
Miklos

Re: [RESEND PATCH] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

Posted by André Draszik 2 years, 5 months ago

On Mon, 2023-09-04 at 15:41 +0200, Miklos Szeredi wrote:
> On Mon, 4 Sept 2023 at 15:34, André Draszik <git@andred.net> wrote:
> > 
> > From: André Draszik <andre.draszik@linaro.org>
> > 
> > This reverts commit 3066ff93476c35679cb07a97cce37d9bb07632ff.
> > 
> > This patch breaks all existing userspace by requiring updates as
> > mentioned in the commit message, which is not allowed.
> 
> It might break something, but you need to tell us what that is, please.

In my case, it's Android.

More generally this breaks all user-spaces that haven't been updated. Not
breaking user-space is one of the top rules the kernel has, if not the topmost.


Cheers,
Andre

Re: [RESEND PATCH] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

Posted by Bernd Schubert 2 years, 5 months ago

On 9/4/23 16:21, André Draszik wrote:
> On Mon, 2023-09-04 at 15:41 +0200, Miklos Szeredi wrote:
>> On Mon, 4 Sept 2023 at 15:34, André Draszik <git@andred.net> wrote:
>>>
>>> From: André Draszik <andre.draszik@linaro.org>
>>>
>>> This reverts commit 3066ff93476c35679cb07a97cce37d9bb07632ff.
>>>
>>> This patch breaks all existing userspace by requiring updates as
>>> mentioned in the commit message, which is not allowed.
>>
>> It might break something, but you need to tell us what that is, please.
> 
> In my case, it's Android.
> 
> More generally this breaks all user-spaces that haven't been updated. Not
> breaking user-space is one of the top rules the kernel has, if not the topmost.

Hmm, I guess Android is using one of the extended flags in the mean time.
Do you have more data what exactly fails? I had posted this patch last year,
when it was still rather early introduction of FUSE_INIT_EXT, hoping there was
nothing in production yet using these flags. But virtiofsd was already using it,
so the patch got delayed (I had actually assumed it would just get dropped).

Sorry for the trouble!

Bernd

[PATCH v2] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

Posted by André Draszik 2 years, 3 months ago

From: André Draszik <andre.draszik@linaro.org>

This reverts commit 3066ff93476c35679cb07a97cce37d9bb07632ff.

This patch breaks all existing userspace by requiring updates as
mentioned in the commit message, which is not allowed.

Revert to restore compatibility with existing userspace
implementations.

Cc: Bernd Schubert <bschubert@ddn.com>
Cc: Miklos Szeredi <mszeredi@redhat.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: André Draszik <andre.draszik@linaro.org>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
v2: ping & add ack
v1: resend because of missing people in Cc
---
 fs/fuse/inode.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 2e4eb7cf26fb..b21ccc85c47b 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1154,10 +1154,7 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
 		process_init_limits(fc, arg);
 
 		if (arg->minor >= 6) {
-			u64 flags = arg->flags;
-
-			if (flags & FUSE_INIT_EXT)
-				flags |= (u64) arg->flags2 << 32;
+			u64 flags = arg->flags | (u64) arg->flags2 << 32;
 
 			ra_pages = arg->max_readahead / PAGE_SIZE;
 			if (flags & FUSE_ASYNC_READ)
-- 
2.40.1