linux-unionfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
@ 2022-07-04 18:36 Christian Kohlschütter
  2022-07-18  9:14 ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Christian Kohlschütter @ 2022-07-04 18:36 UTC (permalink / raw)
  To: Miklos Szeredi, linux-unionfs; +Cc: linux-kernel

overlayfs may fail to complete updates when a filesystem lacks
fileattr/xattr syscall support and responds with an ENOSYS error code,
resulting in an unexpected "Function not implemented" error.

This bug may occur with FUSE filesystems, such as davfs2.

Steps to reproduce:

  # install davfs2, e.g., apk add davfs2
  mkdir /test mkdir /test/lower /test/upper /test/work /test/mnt
  yes '' | mount -t davfs -o ro http://some-web-dav-server/path \
    /test/lower
  mount -t overlay -o upperdir=/test/upper,lowerdir=/test/lower \
    -o workdir=/test/work overlay /test/mnt

  # when "some-file" exists in the lowerdir, this fails with "Function
  # not implemented", with dmesg showing "overlayfs: failed to retrieve
  # lower fileattr (/some-file, err=-38)"
  touch /test/mnt/some-file

This bug is related to a regression in v5.15 that was partially fixed in
v5.16.

This patch also adds checks for ENOSYS in case the upper file system
does not support file attributes. That change is related to a partial
fix in v5.17.

Reported-by: Christian Kohlschütter <christian@kohlschutter.com>
Fixes: 5b0a414d06c ("ovl: fix filattr copy-up failure")
Fixes: 24d7f48c723 ("ovl: don't fail copy up if no fileattr support on upper")
Cc: <stable@vger.kernel.org> # v5.15
Signed-off-by: Christian Kohlschütter <christian@kohlschutter.com>
---
 fs/overlayfs/copy_up.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 714ec569d25b..0ad88573e77a 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -142,7 +142,7 @@ static int ovl_copy_fileattr(struct inode *inode, struct path *old,
 	err = ovl_real_fileattr_get(old, &oldfa);
 	if (err) {
 		/* Ntfs-3g returns -EINVAL for "no fileattr support" */
-		if (err == -ENOTTY || err == -EINVAL)
+		if (err == -ENOTTY || err == -EINVAL || err == -ENOSYS)
 			return 0;
 		pr_warn("failed to retrieve lower fileattr (%pd2, err=%i)\n",
 			old->dentry, err);
@@ -173,7 +173,7 @@ static int ovl_copy_fileattr(struct inode *inode, struct path *old,
 		 * Returning an error if upper doesn't support fileattr will
 		 * result in a regression, so revert to the old behavior.
 		 */
-		if (err == -ENOTTY || err == -EINVAL) {
+		if (err == -ENOTTY || err == -EINVAL || err == -ENOSYS) {
 			pr_warn_once("copying fileattr: no support on upper\n");
 			return 0;
 		}
-- 
2.36.1



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

* Re: [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-04 18:36 [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs Christian Kohlschütter
@ 2022-07-18  9:14 ` Miklos Szeredi
  2022-07-18 10:10   ` Christian Kohlschütter
  0 siblings, 1 reply; 28+ messages in thread
From: Miklos Szeredi @ 2022-07-18  9:14 UTC (permalink / raw)
  To: Christian Kohlschütter; +Cc: overlayfs, linux-kernel, linux-fsdevel

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

On Mon, 4 Jul 2022 at 20:36, Christian Kohlschütter
<christian@kohlschutter.com> wrote:
>
> overlayfs may fail to complete updates when a filesystem lacks
> fileattr/xattr syscall support and responds with an ENOSYS error code,
> resulting in an unexpected "Function not implemented" error.

Issue seems to be with fuse: nothing should be returning ENOSYS to
userspace except the syscall lookup code itself.  ENOSYS means that
the syscall does not exist.

Fuse uses ENOSYS in the protocol to indicate that the filesystem does
not support that operation, but that's not the value that the
filesystem should be returning to userspace.

The getxattr/setxattr implementations already translate ENOSYS to
EOPNOTSUPP, but ioctl doesn't.

The attached patch (untested) should do this.   Can you please give it a try?

Thanks,
Miklos

[-- Attachment #2: fuse-ioctl-translate-enosys.patch --]
[-- Type: text/x-patch, Size: 1163 bytes --]

---
 fs/fuse/ioctl.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

--- a/fs/fuse/ioctl.c
+++ b/fs/fuse/ioctl.c
@@ -9,6 +9,17 @@
 #include <linux/compat.h>
 #include <linux/fileattr.h>
 
+static ssize_t fuse_send_ioctl(struct fuse_mount *fm, struct fuse_args *args)
+{
+	ssize_t ret = fuse_simple_request(fm, args);
+
+	/* Translate ENOSYS, which shouldn't be returned from fs */
+	if (ret == -ENOSYS)
+		ret = -ENOTTY;
+
+	return ret;
+}
+
 /*
  * CUSE servers compiled on 32bit broke on 64bit kernels because the
  * ABI was defined to be 'struct iovec' which is different on 32bit
@@ -259,7 +270,7 @@ long fuse_do_ioctl(struct file *file, un
 	ap.args.out_pages = true;
 	ap.args.out_argvar = true;
 
-	transferred = fuse_simple_request(fm, &ap.args);
+	transferred = fuse_send_ioctl(fm, &ap.args);
 	err = transferred;
 	if (transferred < 0)
 		goto out;
@@ -393,7 +404,7 @@ static int fuse_priv_ioctl(struct inode
 	args.out_args[1].size = inarg.out_size;
 	args.out_args[1].value = ptr;
 
-	err = fuse_simple_request(fm, &args);
+	err = fuse_send_ioctl(fm, &args);
 	if (!err) {
 		if (outarg.result < 0)
 			err = outarg.result;

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

* Re: [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18  9:14 ` Miklos Szeredi
@ 2022-07-18 10:10   ` Christian Kohlschütter
  2022-07-18 10:31     ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Christian Kohlschütter @ 2022-07-18 10:10 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: overlayfs, linux-kernel, linux-fsdevel

> Am 18.07.2022 um 11:14 schrieb Miklos Szeredi <miklos@szeredi.hu>:
> 
> On Mon, 4 Jul 2022 at 20:36, Christian Kohlschütter
> <christian@kohlschutter.com> wrote:
>> 
>> overlayfs may fail to complete updates when a filesystem lacks
>> fileattr/xattr syscall support and responds with an ENOSYS error code,
>> resulting in an unexpected "Function not implemented" error.
> 
> Issue seems to be with fuse: nothing should be returning ENOSYS to
> userspace except the syscall lookup code itself.  ENOSYS means that
> the syscall does not exist.
> 
> Fuse uses ENOSYS in the protocol to indicate that the filesystem does
> not support that operation, but that's not the value that the
> filesystem should be returning to userspace.
> 
> The getxattr/setxattr implementations already translate ENOSYS to
> EOPNOTSUPP, but ioctl doesn't.
> 
> The attached patch (untested) should do this.   Can you please give it a try?
> 
> Thanks,
> Miklos
> <fuse-ioctl-translate-enosys.patch>

Yes, that change basically has the same effect for the demo use case,.

However: it will change (and potentially) break assumptions in user space. We should never break user space.

Example: lsattr /test/lower
Currently, fuse returns ENOSYS, e.g.
> lsattr: reading ./lost+found: Function not implemented
With your change, it would return ENOTTY
> lsattr: reading ./lost+found: Not a tty


I also tried the setup (without patches) on a very old 4.4.176 system, and everything works fine. ovl introduced the regression, so it should also be fixed there.
It may affect other filing systems as well (I see some other fs also return ENOSYS on occasion).

It's safe to say that adding the ENOSYS to the ovl code is probably the best move. Besides, you already have a workaround for ntfs-3g there as well.

Best,
Christian


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

* Re: [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 10:10   ` Christian Kohlschütter
@ 2022-07-18 10:31     ` Miklos Szeredi
  2022-07-18 10:56       ` Christian Kohlschütter
  0 siblings, 1 reply; 28+ messages in thread
From: Miklos Szeredi @ 2022-07-18 10:31 UTC (permalink / raw)
  To: Christian Kohlschütter; +Cc: overlayfs, linux-kernel, linux-fsdevel

On Mon, 18 Jul 2022 at 12:10, Christian Kohlschütter
<christian@kohlschutter.com> wrote:
>
> > Am 18.07.2022 um 11:14 schrieb Miklos Szeredi <miklos@szeredi.hu>:
> >
> > On Mon, 4 Jul 2022 at 20:36, Christian Kohlschütter
> > <christian@kohlschutter.com> wrote:
> >>
> >> overlayfs may fail to complete updates when a filesystem lacks
> >> fileattr/xattr syscall support and responds with an ENOSYS error code,
> >> resulting in an unexpected "Function not implemented" error.
> >
> > Issue seems to be with fuse: nothing should be returning ENOSYS to
> > userspace except the syscall lookup code itself.  ENOSYS means that
> > the syscall does not exist.
> >
> > Fuse uses ENOSYS in the protocol to indicate that the filesystem does
> > not support that operation, but that's not the value that the
> > filesystem should be returning to userspace.
> >
> > The getxattr/setxattr implementations already translate ENOSYS to
> > EOPNOTSUPP, but ioctl doesn't.
> >
> > The attached patch (untested) should do this.   Can you please give it a try?
> >
> > Thanks,
> > Miklos
> > <fuse-ioctl-translate-enosys.patch>
>
> Yes, that change basically has the same effect for the demo use case,.
>
> However: it will change (and potentially) break assumptions in user space. We should never break user space.
>
> Example: lsattr /test/lower
> Currently, fuse returns ENOSYS, e.g.
> > lsattr: reading ./lost+found: Function not implemented
> With your change, it would return ENOTTY
> > lsattr: reading ./lost+found: Not a tty

No, it would return success.

> I also tried the setup (without patches) on a very old 4.4.176 system, and everything works fine. ovl introduced the regression, so it should also be fixed there.
> It may affect other filing systems as well (I see some other fs also return ENOSYS on occasion).
>
> It's safe to say that adding the ENOSYS to the ovl code is probably the best move. Besides, you already have a workaround for ntfs-3g there as well.

Flawed arguments.  The change in overlayfs just made the preexisting
bug in fuse visible.  The bug should still be fixed in fuse.

Thanks,
Miklos

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

* Re: [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 10:31     ` Miklos Szeredi
@ 2022-07-18 10:56       ` Christian Kohlschütter
  2022-07-18 12:21         ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Christian Kohlschütter @ 2022-07-18 10:56 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: overlayfs, linux-kernel, linux-fsdevel


-- 
Dr. Christian Kohlschütter



> Am 18.07.2022 um 12:31 schrieb Miklos Szeredi <miklos@szeredi.hu>:
> 
> On Mon, 18 Jul 2022 at 12:10, Christian Kohlschütter
> <christian@kohlschutter.com> wrote:
>> 
>>> Am 18.07.2022 um 11:14 schrieb Miklos Szeredi <miklos@szeredi.hu>:
>>> 
>>> On Mon, 4 Jul 2022 at 20:36, Christian Kohlschütter
>>> <christian@kohlschutter.com> wrote:
>>>> 
>>>> overlayfs may fail to complete updates when a filesystem lacks
>>>> fileattr/xattr syscall support and responds with an ENOSYS error code,
>>>> resulting in an unexpected "Function not implemented" error.
>>> 
>>> Issue seems to be with fuse: nothing should be returning ENOSYS to
>>> userspace except the syscall lookup code itself.  ENOSYS means that
>>> the syscall does not exist.
>>> 
>>> Fuse uses ENOSYS in the protocol to indicate that the filesystem does
>>> not support that operation, but that's not the value that the
>>> filesystem should be returning to userspace.
>>> 
>>> The getxattr/setxattr implementations already translate ENOSYS to
>>> EOPNOTSUPP, but ioctl doesn't.
>>> 
>>> The attached patch (untested) should do this.   Can you please give it a try?
>>> 
>>> Thanks,
>>> Miklos
>>> <fuse-ioctl-translate-enosys.patch>
>> 
>> Yes, that change basically has the same effect for the demo use case,.
>> 
>> However: it will change (and potentially) break assumptions in user space. We should never break user space.
>> 
>> Example: lsattr /test/lower
>> Currently, fuse returns ENOSYS, e.g.
>>> lsattr: reading ./lost+found: Function not implemented
>> With your change, it would return ENOTTY
>>> lsattr: reading ./lost+found: Not a tty
> 
> No, it would return success.

I'm referring to /test/lower (powered by fuse davfs2), not /test/mnt (overlayfs).

> 
>> I also tried the setup (without patches) on a very old 4.4.176 system, and everything works fine. ovl introduced the regression, so it should also be fixed there.
>> It may affect other filing systems as well (I see some other fs also return ENOSYS on occasion).
>> 
>> It's safe to say that adding the ENOSYS to the ovl code is probably the best move. Besides, you already have a workaround for ntfs-3g there as well.
> 
> Flawed arguments.  The change in overlayfs just made the preexisting
> bug in fuse visible.  The bug should still be fixed in fuse.

I understand your point from ovl's perspective, however you are proposing a fix in fuse, and so we have to see it from fuse's perspective (and its users).

From ovl's point of view, your patch fixes it because the behavior in ovl will now become how it was before the regression was introduced.
However, users of fuse that have no business with overlayfs suddenly see their ioctl return ENOTTY instead of ENOSYS.


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

* Re: [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 10:56       ` Christian Kohlschütter
@ 2022-07-18 12:21         ` Miklos Szeredi
  2022-07-18 13:03           ` [PATCH] [REGRESSION] " Christian Kohlschütter
  0 siblings, 1 reply; 28+ messages in thread
From: Miklos Szeredi @ 2022-07-18 12:21 UTC (permalink / raw)
  To: Christian Kohlschütter; +Cc: overlayfs, linux-kernel, linux-fsdevel

On Mon, 18 Jul 2022 at 12:56, Christian Kohlschütter
<christian@kohlschutter.com> wrote:

> However, users of fuse that have no business with overlayfs suddenly see their ioctl return ENOTTY instead of ENOSYS.

And returning ENOTTY is the correct behavior.  See this comment in
<asm-generic/errrno.h>:

/*
 * This error code is special: arch syscall entry code will return
 * -ENOSYS if users try to call a syscall that doesn't exist.  To keep
 * failures of syscalls that really do exist distinguishable from
 * failures due to attempts to use a nonexistent syscall, syscall
 * implementations should refrain from returning -ENOSYS.
 */
#define ENOSYS 38 /* Invalid system call number */

Thanks,
Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 12:21         ` Miklos Szeredi
@ 2022-07-18 13:03           ` Christian Kohlschütter
  2022-07-18 13:13             ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Christian Kohlschütter @ 2022-07-18 13:03 UTC (permalink / raw)
  To: Miklos Szeredi, torvalds; +Cc: overlayfs, linux-kernel, linux-fsdevel

Am 18.07.2022 um 14:21 schrieb Miklos Szeredi <miklos@szeredi.hu>:
> 
> On Mon, 18 Jul 2022 at 12:56, Christian Kohlschütter
> <christian@kohlschutter.com> wrote:
> 
>> However, users of fuse that have no business with overlayfs suddenly see their ioctl return ENOTTY instead of ENOSYS.
> 
> And returning ENOTTY is the correct behavior.  See this comment in
> <asm-generic/errrno.h>:
> 
> /*
> * This error code is special: arch syscall entry code will return
> * -ENOSYS if users try to call a syscall that doesn't exist.  To keep
> * failures of syscalls that really do exist distinguishable from
> * failures due to attempts to use a nonexistent syscall, syscall
> * implementations should refrain from returning -ENOSYS.
> */
> #define ENOSYS 38 /* Invalid system call number */
> 
> Thanks,
> Miklos

That ship is sailed since ENOSYS was returned to user-space for the first time.

It reminds me a bit of Linus' "we do not break userspace" email from 2012 [1, 2], where Linus wrote:
> Applications *do* care about error return values. There's no way in
> hell you can willy-nilly just change them. And if you do change them,
> and applications break, there is no way in hell you can then blame the
> application.

(Adding Linus for clarification whether his statement is still true in 2022, and marking subject with regression tag for visibility.)

As far as I, a fuse user, understand, fuse uses ENOSYS as a specific marker to indicate permanent failure:

From libfuse https://libfuse.github.io/doxygen/structfuse__lowlevel__ops.html:
> If this request is answered with an error code of ENOSYS, this is treated as a permanent failure with error code EOPNOTSUPP, i.e. all future getxattr() requests will fail with EOPNOTSUPP without being send to the filesystem process.

(also see  https://man.openbsd.org/fuse_new.3)


Again, in summary:
1. I believe the fix should be in ovl, because ovl caused the regression.
2. Fixing in fuse would not be sufficient because other lower filesystems that also return ENOSYS remain affected (a cursory search indicates ecryptfs also returns ENOSYS).
3. Fixing in fuse would break user-space. We do not break userspace.

Cheers,
Christian



[1] https://lkml.org/lkml/2012/12/23/75
[2] https://lkml.org/lkml/2012/12/23/99

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 13:03           ` [PATCH] [REGRESSION] " Christian Kohlschütter
@ 2022-07-18 13:13             ` Miklos Szeredi
  2022-07-18 14:25               ` Christian Kohlschütter
  2022-07-18 18:29               ` Linus Torvalds
  0 siblings, 2 replies; 28+ messages in thread
From: Miklos Szeredi @ 2022-07-18 13:13 UTC (permalink / raw)
  To: Christian Kohlschütter
  Cc: Linus Torvalds, overlayfs, linux-kernel, linux-fsdevel

On Mon, 18 Jul 2022 at 15:03, Christian Kohlschütter
<christian@kohlschutter.com> wrote:
>
> Am 18.07.2022 um 14:21 schrieb Miklos Szeredi <miklos@szeredi.hu>:
> >
> > On Mon, 18 Jul 2022 at 12:56, Christian Kohlschütter
> > <christian@kohlschutter.com> wrote:
> >
> >> However, users of fuse that have no business with overlayfs suddenly see their ioctl return ENOTTY instead of ENOSYS.
> >
> > And returning ENOTTY is the correct behavior.  See this comment in
> > <asm-generic/errrno.h>:
> >
> > /*
> > * This error code is special: arch syscall entry code will return
> > * -ENOSYS if users try to call a syscall that doesn't exist.  To keep
> > * failures of syscalls that really do exist distinguishable from
> > * failures due to attempts to use a nonexistent syscall, syscall
> > * implementations should refrain from returning -ENOSYS.
> > */
> > #define ENOSYS 38 /* Invalid system call number */
> >
> > Thanks,
> > Miklos
>
> That ship is sailed since ENOSYS was returned to user-space for the first time.
>
> It reminds me a bit of Linus' "we do not break userspace" email from 2012 [1, 2], where Linus wrote:
> > Applications *do* care about error return values. There's no way in
> > hell you can willy-nilly just change them. And if you do change them,
> > and applications break, there is no way in hell you can then blame the
> > application.

Correct.  The question is whether any application would break in this
case.  I think not, but you are free to prove otherwise.

Thanks,
Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 13:13             ` Miklos Szeredi
@ 2022-07-18 14:25               ` Christian Kohlschütter
  2022-07-18 15:02                 ` Antonio SJ Musumeci
  2022-07-18 17:23                 ` Miklos Szeredi
  2022-07-18 18:29               ` Linus Torvalds
  1 sibling, 2 replies; 28+ messages in thread
From: Christian Kohlschütter @ 2022-07-18 14:25 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Linus Torvalds, overlayfs, linux-kernel, linux-fsdevel

> Am 18.07.2022 um 15:13 schrieb Miklos Szeredi <miklos@szeredi.hu>:
> 
> On Mon, 18 Jul 2022 at 15:03, Christian Kohlschütter
> <christian@kohlschutter.com> wrote:
>> 
>> Am 18.07.2022 um 14:21 schrieb Miklos Szeredi <miklos@szeredi.hu>:
>>> 
>>> On Mon, 18 Jul 2022 at 12:56, Christian Kohlschütter
>>> <christian@kohlschutter.com> wrote:
>>> 
>>>> However, users of fuse that have no business with overlayfs suddenly see their ioctl return ENOTTY instead of ENOSYS.
>>> 
>>> And returning ENOTTY is the correct behavior.  See this comment in
>>> <asm-generic/errrno.h>:
>>> 
>>> /*
>>> * This error code is special: arch syscall entry code will return
>>> * -ENOSYS if users try to call a syscall that doesn't exist.  To keep
>>> * failures of syscalls that really do exist distinguishable from
>>> * failures due to attempts to use a nonexistent syscall, syscall
>>> * implementations should refrain from returning -ENOSYS.
>>> */
>>> #define ENOSYS 38 /* Invalid system call number */
>>> 
>>> Thanks,
>>> Miklos
>> 
>> That ship is sailed since ENOSYS was returned to user-space for the first time.
>> 
>> It reminds me a bit of Linus' "we do not break userspace" email from 2012 [1, 2], where Linus wrote:
>>> Applications *do* care about error return values. There's no way in
>>> hell you can willy-nilly just change them. And if you do change them,
>>> and applications break, there is no way in hell you can then blame the
>>> application.
> 
> Correct.  The question is whether any application would break in this
> case.  I think not, but you are free to prove otherwise.
> 
> Thanks,
> Miklos

I'm not going to do that since I expect any answer I give would not change your position here. All I know is there is a non-zero chance such programs exist.

If you're willing to go ahead with the fuse change you proposed, I see no purpose in debating with you further since you're the kernel maintainer of both file systems.
That change "fixes" the problem that I had seen in my setup; I do not know the extent of side effects, but I expect some could surface eventually.

Once you're done fixing fuse, please also talk to the folks over at https://github.com/trapexit/mergerfs who explicitly return ENOSYS upon request. Who knows, maybe someone is audacious enough to try mergerfs as a lower filesystem for overlay?

Alas, I think this a clash between the philosophies of writing robust code versus writing against a personal interpretation of some specification.
You refer to "asm-generic/errno.h" as the specification and rationale for treating ENOSYS as sacrosanct. Note that the comment says "should refrain from", it doesn't say "must not", and that's why we're in this mess.

It therefore wouldn't hurt to be lenient when a lower filesystem returns an error code known to refer to "unsupported operation", and that's what my original patch to ovl does.

I thought this approach would resonate with you, since you must have been following the same logic when you added the special-case check for "EINVAL" as an exception for ntfs-3g in the commit that most likely triggered the regression ("ovl: fix filattr copy-up failure") 9 months ago.

I honestly wonder why you're risking further breakage, having introduced that regression only recently.

So long,
Christian


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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 14:25               ` Christian Kohlschütter
@ 2022-07-18 15:02                 ` Antonio SJ Musumeci
  2022-07-18 17:23                 ` Miklos Szeredi
  1 sibling, 0 replies; 28+ messages in thread
From: Antonio SJ Musumeci @ 2022-07-18 15:02 UTC (permalink / raw)
  To: Christian Kohlschütter, Miklos Szeredi
  Cc: Linus Torvalds, overlayfs, linux-kernel, linux-fsdevel

On 7/18/22 10:25, Christian Kohlschütter wrote:
>> Am 18.07.2022 um 15:13 schrieb Miklos Szeredi <miklos@szeredi.hu>:
>>
>> On Mon, 18 Jul 2022 at 15:03, Christian Kohlschütter
>> <christian@kohlschutter.com> wrote:
>>> Am 18.07.2022 um 14:21 schrieb Miklos Szeredi <miklos@szeredi.hu>:
>>>> On Mon, 18 Jul 2022 at 12:56, Christian Kohlschütter
>>>> <christian@kohlschutter.com> wrote:
>>>>
>>>>> However, users of fuse that have no business with overlayfs suddenly see their ioctl return ENOTTY instead of ENOSYS.
>>>> And returning ENOTTY is the correct behavior.  See this comment in
>>>> <asm-generic/errrno.h>:
>>>>
>>>> /*
>>>> * This error code is special: arch syscall entry code will return
>>>> * -ENOSYS if users try to call a syscall that doesn't exist.  To keep
>>>> * failures of syscalls that really do exist distinguishable from
>>>> * failures due to attempts to use a nonexistent syscall, syscall
>>>> * implementations should refrain from returning -ENOSYS.
>>>> */
>>>> #define ENOSYS 38 /* Invalid system call number */
>>>>
>>>> Thanks,
>>>> Miklos
>>> That ship is sailed since ENOSYS was returned to user-space for the first time.
>>>
>>> It reminds me a bit of Linus' "we do not break userspace" email from 2012 [1, 2], where Linus wrote:
>>>> Applications *do* care about error return values. There's no way in
>>>> hell you can willy-nilly just change them. And if you do change them,
>>>> and applications break, there is no way in hell you can then blame the
>>>> application.
>> Correct.  The question is whether any application would break in this
>> case.  I think not, but you are free to prove otherwise.
>>
>> Thanks,
>> Miklos
> I'm not going to do that since I expect any answer I give would not change your position here. All I know is there is a non-zero chance such programs exist.
>
> If you're willing to go ahead with the fuse change you proposed, I see no purpose in debating with you further since you're the kernel maintainer of both file systems.
> That change "fixes" the problem that I had seen in my setup; I do not know the extent of side effects, but I expect some could surface eventually.
>
> Once you're done fixing fuse, please also talk to the folks over at https://github.com/trapexit/mergerfs who explicitly return ENOSYS upon request. Who knows, maybe someone is audacious enough to try mergerfs as a lower filesystem for overlay?
>
> Alas, I think this a clash between the philosophies of writing robust code versus writing against a personal interpretation of some specification.
> You refer to "asm-generic/errno.h" as the specification and rationale for treating ENOSYS as sacrosanct. Note that the comment says "should refrain from", it doesn't say "must not", and that's why we're in this mess.
>
> It therefore wouldn't hurt to be lenient when a lower filesystem returns an error code known to refer to "unsupported operation", and that's what my original patch to ovl does.
>
> I thought this approach would resonate with you, since you must have been following the same logic when you added the special-case check for "EINVAL" as an exception for ntfs-3g in the commit that most likely triggered the regression ("ovl: fix filattr copy-up failure") 9 months ago.
>
> I honestly wonder why you're risking further breakage, having introduced that regression only recently.
>
> So long,
> Christian

Author of mergerfs here. What are you referring to exactly? It's 
possible I'm forgetting something but  I should only be returning ENOSYS 
in similar cases to libfuse where some function is not supported or when 
wishing to disable xattr calls.


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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 14:25               ` Christian Kohlschütter
  2022-07-18 15:02                 ` Antonio SJ Musumeci
@ 2022-07-18 17:23                 ` Miklos Szeredi
  1 sibling, 0 replies; 28+ messages in thread
From: Miklos Szeredi @ 2022-07-18 17:23 UTC (permalink / raw)
  To: Christian Kohlschütter
  Cc: Linus Torvalds, overlayfs, linux-kernel, linux-fsdevel

On Mon, 18 Jul 2022 at 16:25, Christian Kohlschütter
<christian@kohlschutter.com> wrote:
>
> > Am 18.07.2022 um 15:13 schrieb Miklos Szeredi <miklos@szeredi.hu>:

> > Correct.  The question is whether any application would break in this
> > case.  I think not, but you are free to prove otherwise.
> >
> > Thanks,
> > Miklos
>
> I'm not going to do that since I expect any answer I give would not change your position here. All I know is there is a non-zero chance such programs exist.

If you (or anyone) gave a real life example of an application relying
on e.g. ioctl(..., FS_IOC_SETFLAGS) returning ENOSYS, then I would
have no choice but to revert this change.

However I think that's highly unlikely given that such an application
would only have been tested on fuse filesystems and also given that
very few (if any) fuse filesystems support these ioctls in the first
place.

Thanks,
Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 13:13             ` Miklos Szeredi
  2022-07-18 14:25               ` Christian Kohlschütter
@ 2022-07-18 18:29               ` Linus Torvalds
  2022-07-18 19:04                 ` Christian Kohlschütter
  1 sibling, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2022-07-18 18:29 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Christian Kohlschütter, overlayfs, linux-kernel, linux-fsdevel

On Mon, Jul 18, 2022 at 6:13 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> Correct.  The question is whether any application would break in this
> case.  I think not, but you are free to prove otherwise.

Most often, an error is "just an error", and most applications usually
won't care.

There are exceptions: some errors are very much "do something special"
(eg EAGAIN or EINTR _are_ often separately tested for and often mean
"just retry"). And permission error handling is often different from
EINVAL etc.

And ENOSYS can easily be such an error - people probing whether they
are running on a new kernel that supports a new system call or not.

And yeah, some of our ioctl's are odd, and we have a lot of drivers
(and driver infrastructure) that basically does "this device does not
support this ioctl, so return ENOSYS".

I don't think that's the right thing to do, but I think it's
understandable. The traditional error for "this device does not
support this ioctl" is ENOTTY, which sounds so crazy to non-tty people
that I understand why people have used ENOSYS instead.

It's sad that it's called "ENOTTY" and some (at least historical)
strerror() implementations will indeed return "Not a tty". Never mind
that modern ones will say "inappropriate ioctl for device" - even when
the string has been updated, the error number isn't called
EINAPPROPRAITEDEVICE.

But it is what it is, and so I think ENOTTY is understandably not used
in many situations just because it's such a senseless historical name.

And so if people don't use ENOSYS, they use EINVAL.

I *suspect* no application cares: partly because ioctl error numbers
are so random anyway, but also very much if this is a "without
overlayfs it does X, with overlayfs it does Y".

The sanest thing to do is likely to make ovl match what a non-ovl
setup would do in the same situation (_either_ of the overlaid
filesystems - there might be multiple cases).

But I'm missing the original report. It sounds like there was a
regression and we already have a case of "changing the error number
broke something". If so, that regression should be fixed.

In general, I'm perfectly happy with people fixing error numbers and
changing them.

The only thing I require is that if those cleanups and fixes are
reported to break something, people quickly revert (and preferably add
a big comment about "Use *this* error number, because while this
*other* error number would make sense, application XyZ expects AbC"..)

             Linus

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 18:29               ` Linus Torvalds
@ 2022-07-18 19:04                 ` Christian Kohlschütter
  2022-07-18 19:17                   ` Linus Torvalds
  0 siblings, 1 reply; 28+ messages in thread
From: Christian Kohlschütter @ 2022-07-18 19:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Miklos Szeredi, overlayfs, linux-kernel, linux-fsdevel

Am 18.07.2022 um 20:29 schrieb Linus Torvalds <torvalds@linux-foundation.org>:
> 
> On Mon, Jul 18, 2022 at 6:13 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>> 
>> Correct.  The question is whether any application would break in this
>> case.  I think not, but you are free to prove otherwise.
> 
> Most often, an error is "just an error", and most applications usually
> won't care.
> 
> There are exceptions: some errors are very much "do something special"
> (eg EAGAIN or EINTR _are_ often separately tested for and often mean
> "just retry"). And permission error handling is often different from
> EINVAL etc.
> 
> And ENOSYS can easily be such an error - people probing whether they
> are running on a new kernel that supports a new system call or not.
> 
> And yeah, some of our ioctl's are odd, and we have a lot of drivers
> (and driver infrastructure) that basically does "this device does not
> support this ioctl, so return ENOSYS".
> 
> I don't think that's the right thing to do, but I think it's
> understandable. The traditional error for "this device does not
> support this ioctl" is ENOTTY, which sounds so crazy to non-tty people
> that I understand why people have used ENOSYS instead.
> 
> It's sad that it's called "ENOTTY" and some (at least historical)
> strerror() implementations will indeed return "Not a tty". Never mind
> that modern ones will say "inappropriate ioctl for device" - even when
> the string has been updated, the error number isn't called
> EINAPPROPRAITEDEVICE.
> 
> But it is what it is, and so I think ENOTTY is understandably not used
> in many situations just because it's such a senseless historical name.
> 
> And so if people don't use ENOSYS, they use EINVAL.
> 
> I *suspect* no application cares: partly because ioctl error numbers
> are so random anyway, but also very much if this is a "without
> overlayfs it does X, with overlayfs it does Y".
> 
> The sanest thing to do is likely to make ovl match what a non-ovl
> setup would do in the same situation (_either_ of the overlaid
> filesystems - there might be multiple cases).
> 
> But I'm missing the original report. It sounds like there was a
> regression and we already have a case of "changing the error number
> broke something". If so, that regression should be fixed.
> 
> In general, I'm perfectly happy with people fixing error numbers and
> changing them.
> 
> The only thing I require is that if those cleanups and fixes are
> reported to break something, people quickly revert (and preferably add
> a big comment about "Use *this* error number, because while this
> *other* error number would make sense, application XyZ expects AbC"..)
> 
>             Linus

Thanks for clarifying, Linus!

The regression in question caused overlayfs to erroneously return ENOSYS when one lower filesystem (e.g., davfs2) returned this upon checking extended attributes (there were two relevant submissions triggering this somewhere around 5.15, 5.16)

My original patch: https://lore.kernel.org/lkml/4B9D76D5-C794-4A49-A76F-3D4C10385EE0@kohlschutter.com/T/

This was supposed to augment the following commit:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=5b0a414d06c3ed2097e32ef7944a4abb644b89bd

There, checking for the exact error code indeed matters, because it is supposed to swallow all conditions marking "no fileattr support" but doesn't catch fuse's ENOSYS.

You see similar code checking for ENOSYS appearing in other places, like util-linux, for example
here https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/commit/?id=f6385a6adeea6be255d68016977c5dd5eaab05da
and there's of course EOPNOTSUPP as well, as in here https://git.kernel.org/pub/scm/utils/util-linux/util-linux.git/commit/?id=7b8fda2e8e7b1752cba1fab01d7f569b5d87e661

Best,
Christian


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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 19:04                 ` Christian Kohlschütter
@ 2022-07-18 19:17                   ` Linus Torvalds
  2022-07-18 19:27                     ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2022-07-18 19:17 UTC (permalink / raw)
  To: Christian Kohlschütter
  Cc: Miklos Szeredi, overlayfs, linux-kernel, linux-fsdevel

On Mon, Jul 18, 2022 at 12:04 PM Christian Kohlschütter
<christian@kohlschutter.com> wrote:
>
> The regression in question caused overlayfs to erroneously return ENOSYS when one lower filesystem (e.g., davfs2) returned this upon checking extended attributes (there were two relevant submissions triggering this somewhere around 5.15, 5.16)

Well, if that's the case, isn't the proper fix to just fix davfs2?

If ENOSYS isn't a valid error, and has broken apps that want to just
ignore "no fattr support", then it's a davfs2 bug, and fixing it there
will presumably magically just fix the ovl case too?

Yes, yes, you point to that commit to util-linux to also accept
ENOSYS, but that's from 2021.

So it's presumably triggered by the same issue - a rare (or new) and
broken filesystem returned the wrong error code.

Let's just fix that.

I do not object to *also* doing the ovlfs "accept ENOSYS too", since
it seems harmless and understandable, but at the same time this all
does make me go "the actual *fundamental* cause of this was davfs2
being confused, it should be fixed there too.

And yes, yes, I realize that davfs2 is out-of-tree fuse filesystem,
and is not in the kernel. But have people made the bug-report to the
maintainers there?

I don't think we should *only* have a kernel-side fix for a broken
FUSE filesystem. Particularly not one to some random bystander like
ovlfs.

In fact if we do a kernel patch for this dodgy filesystem, it would
seem to me to make more sense to have FUSE notice that "ok, ENOSYS is
broken for this situation, let's translate it to the right ENOTTY",
and that would have fixed both the ovlfs case and the util-linux one.

Hmm?

              Linus

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 19:17                   ` Linus Torvalds
@ 2022-07-18 19:27                     ` Miklos Szeredi
  2022-07-18 20:12                       ` Linus Torvalds
  0 siblings, 1 reply; 28+ messages in thread
From: Miklos Szeredi @ 2022-07-18 19:27 UTC (permalink / raw)
  To: Linus Torvalds
  Cc: Christian Kohlschütter, overlayfs, linux-kernel, linux-fsdevel

On Mon, 18 Jul 2022 at 21:17, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Mon, Jul 18, 2022 at 12:04 PM Christian Kohlschütter
> <christian@kohlschutter.com> wrote:
> >
> > The regression in question caused overlayfs to erroneously return ENOSYS when one lower filesystem (e.g., davfs2) returned this upon checking extended attributes (there were two relevant submissions triggering this somewhere around 5.15, 5.16)
>
> Well, if that's the case, isn't the proper fix to just fix davfs2?
>
> If ENOSYS isn't a valid error, and has broken apps that want to just
> ignore "no fattr support", then it's a davfs2 bug, and fixing it there
> will presumably magically just fix the ovl case too?

Libfuse returns ENOSYS for requests which the filesystem doesn't
support.  Hence this is not a davfs bug, davfs knows nothing about
itoctls, the userspace filesystem is just returning the error value
that is used to mean "no support for this type of request".

So this is a bug in the kernel part of fuse, that doesn't catch and
convert ENOSYS in case of the ioctl request.

Fixing it in fuse should fix it in overlayfs as well, as you say.

Thanks,
Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 19:27                     ` Miklos Szeredi
@ 2022-07-18 20:12                       ` Linus Torvalds
  2022-07-18 20:33                         ` Christian Kohlschütter
  0 siblings, 1 reply; 28+ messages in thread
From: Linus Torvalds @ 2022-07-18 20:12 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Christian Kohlschütter, overlayfs, linux-kernel, linux-fsdevel

On Mon, Jul 18, 2022 at 12:28 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> So this is a bug in the kernel part of fuse, that doesn't catch and
> convert ENOSYS in case of the ioctl request.

Ahh, even better. No need to worry about external issues.

            Linus

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 20:12                       ` Linus Torvalds
@ 2022-07-18 20:33                         ` Christian Kohlschütter
  2023-01-18  3:41                           ` Jonathan Katz
  0 siblings, 1 reply; 28+ messages in thread
From: Christian Kohlschütter @ 2022-07-18 20:33 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Miklos Szeredi, overlayfs, linux-kernel, linux-fsdevel

> Am 18.07.2022 um 22:12 schrieb Linus Torvalds <torvalds@linux-foundation.org>:
> 
> On Mon, Jul 18, 2022 at 12:28 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>> 
>> So this is a bug in the kernel part of fuse, that doesn't catch and
>> convert ENOSYS in case of the ioctl request.
> 
> Ahh, even better. No need to worry about external issues.
> 
>            Linus

My concern was fixing it in fuse instead of ovl would leave non-fuse filesystems affected (even though I don't have proof that such filesystems exist).

I'm glad you are OK with Miklos' change; the outcome of this discussion certainly adds some nuance to the famous "don't break userspace" / error code thread from 2012.

Best,
Christian


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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2022-07-18 20:33                         ` Christian Kohlschütter
@ 2023-01-18  3:41                           ` Jonathan Katz
  2023-01-26 13:26                             ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Katz @ 2023-01-18  3:41 UTC (permalink / raw)
  To: Christian Kohlschütter, Linus Torvalds
  Cc: Miklos Szeredi, overlayfs, linux-kernel, linux-fsdevel


On 7/18/22 13:33, Christian Kohlschütter wrote:
>> Am 18.07.2022 um 22:12 schrieb Linus Torvalds <torvalds@linux-foundation.org>:
>>
>> On Mon, Jul 18, 2022 at 12:28 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
>>> So this is a bug in the kernel part of fuse, that doesn't catch and
>>> convert ENOSYS in case of the ioctl request.
>> Ahh, even better. No need to worry about external issues.
>>
>>             Linus
> My concern was fixing it in fuse instead of ovl would leave non-fuse filesystems affected (even though I don't have proof that such filesystems exist).
>
> I'm glad you are OK with Miklos' change; the outcome of this discussion certainly adds some nuance to the famous "don't break userspace" / error code thread from 2012.
>
> Best,
> Christian
>
I believe that I am still having issues occur within Ubuntu 22.10 with 
the 5.19 version of the kernel that might be associated with this 
discussion.  I apologize up front for any faux pas I make in writing 
this email.

An example error from our syslog:

kernel: [2702258.538549] overlayfs: failed to retrieve lower fileattr 
(8020 MeOHH2O 
RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/analysis.tsf, 
err=-38)

The only other related log notification I get occurs when I do the 
overlay mount:

kernel: [2702222.266404] overlayfs: null uuid detected in lower fs '/', 
falling back to xino=off,index=off,nfs_export=off.


In the following description, the error is occurring on FileServer2

Our configuration is as follows:

FileServer1 "/data" --- NFS(ro)----->  FileServer2

On FileServer2 I wish to export that /data directory via Samba so it 
appears as RW by a specific user.  I accomplish this with bindfs 
followed by overlayfs:

# bindfs -u 1001 -g 1001 /data /overlay/lowers/data-1001
# mount -t overlay overlay -o lowerdir= /overlay/lowers/data-1001,\
upperdir=/overlay/uppers/upper-1001,\
workdir=/overlay/work/work-1001,\
/overlay/mountpoints/data-1001

Then I serve this out via Samba:

FileServer2 "/overlay/mountpoints/data-1001" ------ ( SAMBA/CIFS) --->  
Win-Client


I repeat this bind/mount for several users - each with their own 
"writable" copy of the data.  This mostly works very well... but there 
are some software packages on the win client that fail mysteriously and 
my FileSystem2 log shows "err=-38" messages for various files at the 
same time.

I am guessing there is some relation between the lack of uuid (because 
it is NFS or a bindfs?) and the failure to retrieve the low fileattr, 
but, I am humbly out of my depth here.

-Jonathan




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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-01-18  3:41                           ` Jonathan Katz
@ 2023-01-26 13:26                             ` Miklos Szeredi
  2023-01-30 19:27                               ` Jonathan Katz
  0 siblings, 1 reply; 28+ messages in thread
From: Miklos Szeredi @ 2023-01-26 13:26 UTC (permalink / raw)
  To: jkatz
  Cc: Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

On Wed, 18 Jan 2023 at 04:41, Jonathan Katz <jkatz@eitmlabs.org> wrote:

> I believe that I am still having issues occur within Ubuntu 22.10 with
> the 5.19 version of the kernel that might be associated with this
> discussion.  I apologize up front for any faux pas I make in writing
> this email.

No need to apologize.   The fix in question went into v6.0 of the
upstream kernel.  So apparently it's still missing from the distro you
are using.

> An example error from our syslog:
>
> kernel: [2702258.538549] overlayfs: failed to retrieve lower fileattr
> (8020 MeOHH2O
> RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/analysis.tsf,
> err=-38)

Yep, looks like the same bug.

Thanks,
Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-01-26 13:26                             ` Miklos Szeredi
@ 2023-01-30 19:27                               ` Jonathan Katz
  2023-02-23 23:11                                 ` Jonathan Katz
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Katz @ 2023-01-30 19:27 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

On Thu, Jan 26, 2023 at 5:26 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Wed, 18 Jan 2023 at 04:41, Jonathan Katz <jkatz@eitmlabs.org> wrote:
>
> > I believe that I am still having issues occur within Ubuntu 22.10 with
> > the 5.19 version of the kernel that might be associated with this
> > discussion.  I apologize up front for any faux pas I make in writing
> > this email.
>
> No need to apologize.   The fix in question went into v6.0 of the
> upstream kernel.  So apparently it's still missing from the distro you
> are using.

Thank you for the reply! ---  I have upgraded the Kernel and it still
seems to be throwing errors.  Details follow:

Distro: Ubuntu 22.10.
Upgraded kernel using mainline (mainline --install-latest)

# uname -a
Linux instance-20220314-1510-fileserver-for-overlay
6.1.8-060108-generic #202301240742 SMP PREEMPT_DYNAMIC Tue Jan 24
08:13:53 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

On mount I still get the following notice in syslog (representative):
Jan 30 19:11:46 instance-20220314-1510-fileserver-for-overlay kernel:
[   71.613334] overlayfs: null uuid detected in lower fs '/', falling
back to xino=off,index=off,nfs_export=off.

And on access (via samba) I still see the following errors in the
syslog (representative):
Jan 30 19:19:34 instance-20220314-1510-fileserver-for-overlay kernel:
[  539.181858] overlayfs: failed to retrieve lower fileattr (8020
MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
err=-38)

And on the Windows client, the software still fails with the same symptomology.




>
> > An example error from our syslog:
> >
> > kernel: [2702258.538549] overlayfs: failed to retrieve lower fileattr
> > (8020 MeOHH2O
> > RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/analysis.tsf,
> > err=-38)
>
> Yep, looks like the same bug.
>
> Thanks,
> Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-01-30 19:27                               ` Jonathan Katz
@ 2023-02-23 23:11                                 ` Jonathan Katz
  2023-03-07  1:12                                   ` Jonathan Katz
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Katz @ 2023-02-23 23:11 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

Hi all,

Problem persists with me with 6.2.0
# mainline --install-latest
# reboot

# uname -r
6.2.0-060200-generic


Representative log messages when mounting:
Feb 23 22:50:43 instance-20220314-1510-fileserver-for-overlay kernel:
[   44.641683] overlayfs: null uuid detected in lower fs '/', falling
back to xino=off,index=off,nfs_export=off.



Representative log messages when accessing files:
eb 23 23:06:31 instance-20220314-1510-fileserver-for-overlay kernel: [
 992.505357] overlayfs: failed to retrieve lower fileattr (8020
MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
err=-38)
Feb 23 23:06:32 instance-20220314-1510-fileserver-for-overlay kernel:
[  993.523712] overlayfs: failed to retrieve lower fileattr (8020
MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
err=-38)


On Mon, Jan 30, 2023 at 11:27 AM Jonathan Katz <jkatz@eitmlabs.org> wrote:
>
> On Thu, Jan 26, 2023 at 5:26 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Wed, 18 Jan 2023 at 04:41, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> >
> > > I believe that I am still having issues occur within Ubuntu 22.10 with
> > > the 5.19 version of the kernel that might be associated with this
> > > discussion.  I apologize up front for any faux pas I make in writing
> > > this email.
> >
> > No need to apologize.   The fix in question went into v6.0 of the
> > upstream kernel.  So apparently it's still missing from the distro you
> > are using.
>
> Thank you for the reply! ---  I have upgraded the Kernel and it still
> seems to be throwing errors.  Details follow:
>
> Distro: Ubuntu 22.10.
> Upgraded kernel using mainline (mainline --install-latest)
>
> # uname -a
> Linux instance-20220314-1510-fileserver-for-overlay
> 6.1.8-060108-generic #202301240742 SMP PREEMPT_DYNAMIC Tue Jan 24
> 08:13:53 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
>
> On mount I still get the following notice in syslog (representative):
> Jan 30 19:11:46 instance-20220314-1510-fileserver-for-overlay kernel:
> [   71.613334] overlayfs: null uuid detected in lower fs '/', falling
> back to xino=off,index=off,nfs_export=off.
>
> And on access (via samba) I still see the following errors in the
> syslog (representative):
> Jan 30 19:19:34 instance-20220314-1510-fileserver-for-overlay kernel:
> [  539.181858] overlayfs: failed to retrieve lower fileattr (8020
> MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> err=-38)
>
> And on the Windows client, the software still fails with the same symptomology.
>
>
>
>
> >
> > > An example error from our syslog:
> > >
> > > kernel: [2702258.538549] overlayfs: failed to retrieve lower fileattr
> > > (8020 MeOHH2O
> > > RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/analysis.tsf,
> > > err=-38)
> >
> > Yep, looks like the same bug.
> >
> > Thanks,
> > Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-02-23 23:11                                 ` Jonathan Katz
@ 2023-03-07  1:12                                   ` Jonathan Katz
  2023-03-07  8:38                                     ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Katz @ 2023-03-07  1:12 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

Hi all,

In pursuing this issue, I downloaded the kernel source to see if I
could debug it further.  In so doing, it looks like Christian's patch
was never committed to the main source tree (sorry if my terminology
is wrong).  This is up to and including the 6.3-rc1.  I could also
find no mention of the fix in the log.

I am trying to manually apply this patch now, but, I am wondering if
there was some reason that it was not applied (e.g. it introduces some
instability?)?

Thank you,
Jonathan



On Thu, Feb 23, 2023 at 3:11 PM Jonathan Katz <jkatz@eitmlabs.org> wrote:
>
> Hi all,
>
> Problem persists with me with 6.2.0
> # mainline --install-latest
> # reboot
>
> # uname -r
> 6.2.0-060200-generic
>
>
> Representative log messages when mounting:
> Feb 23 22:50:43 instance-20220314-1510-fileserver-for-overlay kernel:
> [   44.641683] overlayfs: null uuid detected in lower fs '/', falling
> back to xino=off,index=off,nfs_export=off.
>
>
>
> Representative log messages when accessing files:
> eb 23 23:06:31 instance-20220314-1510-fileserver-for-overlay kernel: [
>  992.505357] overlayfs: failed to retrieve lower fileattr (8020
> MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> err=-38)
> Feb 23 23:06:32 instance-20220314-1510-fileserver-for-overlay kernel:
> [  993.523712] overlayfs: failed to retrieve lower fileattr (8020
> MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> err=-38)
>
>
> On Mon, Jan 30, 2023 at 11:27 AM Jonathan Katz <jkatz@eitmlabs.org> wrote:
> >
> > On Thu, Jan 26, 2023 at 5:26 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Wed, 18 Jan 2023 at 04:41, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > >
> > > > I believe that I am still having issues occur within Ubuntu 22.10 with
> > > > the 5.19 version of the kernel that might be associated with this
> > > > discussion.  I apologize up front for any faux pas I make in writing
> > > > this email.
> > >
> > > No need to apologize.   The fix in question went into v6.0 of the
> > > upstream kernel.  So apparently it's still missing from the distro you
> > > are using.
> >
> > Thank you for the reply! ---  I have upgraded the Kernel and it still
> > seems to be throwing errors.  Details follow:
> >
> > Distro: Ubuntu 22.10.
> > Upgraded kernel using mainline (mainline --install-latest)
> >
> > # uname -a
> > Linux instance-20220314-1510-fileserver-for-overlay
> > 6.1.8-060108-generic #202301240742 SMP PREEMPT_DYNAMIC Tue Jan 24
> > 08:13:53 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
> >
> > On mount I still get the following notice in syslog (representative):
> > Jan 30 19:11:46 instance-20220314-1510-fileserver-for-overlay kernel:
> > [   71.613334] overlayfs: null uuid detected in lower fs '/', falling
> > back to xino=off,index=off,nfs_export=off.
> >
> > And on access (via samba) I still see the following errors in the
> > syslog (representative):
> > Jan 30 19:19:34 instance-20220314-1510-fileserver-for-overlay kernel:
> > [  539.181858] overlayfs: failed to retrieve lower fileattr (8020
> > MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> > err=-38)
> >
> > And on the Windows client, the software still fails with the same symptomology.
> >
> >
> >
> >
> > >
> > > > An example error from our syslog:
> > > >
> > > > kernel: [2702258.538549] overlayfs: failed to retrieve lower fileattr
> > > > (8020 MeOHH2O
> > > > RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/analysis.tsf,
> > > > err=-38)
> > >
> > > Yep, looks like the same bug.
> > >
> > > Thanks,
> > > Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-03-07  1:12                                   ` Jonathan Katz
@ 2023-03-07  8:38                                     ` Miklos Szeredi
  2023-03-07 17:14                                       ` Jonathan Katz
  0 siblings, 1 reply; 28+ messages in thread
From: Miklos Szeredi @ 2023-03-07  8:38 UTC (permalink / raw)
  To: jonathan
  Cc: Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

On Tue, 7 Mar 2023 at 02:12, Jonathan Katz <jkatz@eitmlabs.org> wrote:
>
> Hi all,
>
> In pursuing this issue, I downloaded the kernel source to see if I
> could debug it further.  In so doing, it looks like Christian's patch
> was never committed to the main source tree (sorry if my terminology
> is wrong).  This is up to and including the 6.3-rc1.  I could also
> find no mention of the fix in the log.
>
> I am trying to manually apply this patch now, but, I am wondering if
> there was some reason that it was not applied (e.g. it introduces some
> instability?)?

It's fixing the bug in the wrong place, i.e. it's checking for an
-ENOSYS return from vfs_fileattr_get(), but that return value is not
valid at that point.

The right way to fix this bug is to prevent -ENOSYS from being
returned in the first place.

Commit 02c0cab8e734 ("fuse: ioctl: translate ENOSYS") fixes one of
those bugs, but of course it's possible that I missed something in
that fix.

Can you please first verify that an upstream kernel (>v6.0) can also
reproduce this issue?

Thanks,
Miklos



>
> Thank you,
> Jonathan
>
>
>
> On Thu, Feb 23, 2023 at 3:11 PM Jonathan Katz <jkatz@eitmlabs.org> wrote:
> >
> > Hi all,
> >
> > Problem persists with me with 6.2.0
> > # mainline --install-latest
> > # reboot
> >
> > # uname -r
> > 6.2.0-060200-generic
> >
> >
> > Representative log messages when mounting:
> > Feb 23 22:50:43 instance-20220314-1510-fileserver-for-overlay kernel:
> > [   44.641683] overlayfs: null uuid detected in lower fs '/', falling
> > back to xino=off,index=off,nfs_export=off.
> >
> >
> >
> > Representative log messages when accessing files:
> > eb 23 23:06:31 instance-20220314-1510-fileserver-for-overlay kernel: [
> >  992.505357] overlayfs: failed to retrieve lower fileattr (8020
> > MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> > err=-38)
> > Feb 23 23:06:32 instance-20220314-1510-fileserver-for-overlay kernel:
> > [  993.523712] overlayfs: failed to retrieve lower fileattr (8020
> > MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> > err=-38)
> >
> >
> > On Mon, Jan 30, 2023 at 11:27 AM Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > >
> > > On Thu, Jan 26, 2023 at 5:26 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > >
> > > > On Wed, 18 Jan 2023 at 04:41, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > > >
> > > > > I believe that I am still having issues occur within Ubuntu 22.10 with
> > > > > the 5.19 version of the kernel that might be associated with this
> > > > > discussion.  I apologize up front for any faux pas I make in writing
> > > > > this email.
> > > >
> > > > No need to apologize.   The fix in question went into v6.0 of the
> > > > upstream kernel.  So apparently it's still missing from the distro you
> > > > are using.
> > >
> > > Thank you for the reply! ---  I have upgraded the Kernel and it still
> > > seems to be throwing errors.  Details follow:
> > >
> > > Distro: Ubuntu 22.10.
> > > Upgraded kernel using mainline (mainline --install-latest)
> > >
> > > # uname -a
> > > Linux instance-20220314-1510-fileserver-for-overlay
> > > 6.1.8-060108-generic #202301240742 SMP PREEMPT_DYNAMIC Tue Jan 24
> > > 08:13:53 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
> > >
> > > On mount I still get the following notice in syslog (representative):
> > > Jan 30 19:11:46 instance-20220314-1510-fileserver-for-overlay kernel:
> > > [   71.613334] overlayfs: null uuid detected in lower fs '/', falling
> > > back to xino=off,index=off,nfs_export=off.
> > >
> > > And on access (via samba) I still see the following errors in the
> > > syslog (representative):
> > > Jan 30 19:19:34 instance-20220314-1510-fileserver-for-overlay kernel:
> > > [  539.181858] overlayfs: failed to retrieve lower fileattr (8020
> > > MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> > > err=-38)
> > >
> > > And on the Windows client, the software still fails with the same symptomology.
> > >
> > >
> > >
> > >
> > > >
> > > > > An example error from our syslog:
> > > > >
> > > > > kernel: [2702258.538549] overlayfs: failed to retrieve lower fileattr
> > > > > (8020 MeOHH2O
> > > > > RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/analysis.tsf,
> > > > > err=-38)
> > > >
> > > > Yep, looks like the same bug.
> > > >
> > > > Thanks,
> > > > Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-03-07  8:38                                     ` Miklos Szeredi
@ 2023-03-07 17:14                                       ` Jonathan Katz
  2023-03-09 15:31                                         ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Katz @ 2023-03-07 17:14 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: jonathan, Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

On Tue, Mar 7, 2023 at 12:38 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 7 Mar 2023 at 02:12, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> >
> > Hi all,
> >
> > In pursuing this issue, I downloaded the kernel source to see if I
> > could debug it further.  In so doing, it looks like Christian's patch
> > was never committed to the main source tree (sorry if my terminology
> > is wrong).  This is up to and including the 6.3-rc1.  I could also
> > find no mention of the fix in the log.
> >
> > I am trying to manually apply this patch now, but, I am wondering if
> > there was some reason that it was not applied (e.g. it introduces some
> > instability?)?
>
> It's fixing the bug in the wrong place, i.e. it's checking for an
> -ENOSYS return from vfs_fileattr_get(), but that return value is not
> valid at that point.
>
> The right way to fix this bug is to prevent -ENOSYS from being
> returned in the first place.
>
> Commit 02c0cab8e734 ("fuse: ioctl: translate ENOSYS") fixes one of
> those bugs, but of course it's possible that I missed something in
> that fix.
>
> Can you please first verify that an upstream kernel (>v6.0) can also
> reproduce this issue?

Got ya.  that makes a lot of sense, thank you.

I have confirmed that I continue to get the error with 6.2 .
quick summary of the lowerdir:
   server ---- NFS(ro) ---- > client "/nfs"
   client "/nfs" --- bindfs(uidmap) --- > client "/lower"






>
> Thanks,
> Miklos
>
>
>
> >
> > Thank you,
> > Jonathan
> >
> >
> >
> > On Thu, Feb 23, 2023 at 3:11 PM Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > >
> > > Hi all,
> > >
> > > Problem persists with me with 6.2.0
> > > # mainline --install-latest
> > > # reboot
> > >
> > > # uname -r
> > > 6.2.0-060200-generic
> > >
> > >
> > > Representative log messages when mounting:
> > > Feb 23 22:50:43 instance-20220314-1510-fileserver-for-overlay kernel:
> > > [   44.641683] overlayfs: null uuid detected in lower fs '/', falling
> > > back to xino=off,index=off,nfs_export=off.
> > >
> > >
> > >
> > > Representative log messages when accessing files:
> > > eb 23 23:06:31 instance-20220314-1510-fileserver-for-overlay kernel: [
> > >  992.505357] overlayfs: failed to retrieve lower fileattr (8020
> > > MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> > > err=-38)
> > > Feb 23 23:06:32 instance-20220314-1510-fileserver-for-overlay kernel:
> > > [  993.523712] overlayfs: failed to retrieve lower fileattr (8020
> > > MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> > > err=-38)
> > >
> > >
> > > On Mon, Jan 30, 2023 at 11:27 AM Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > > >
> > > > On Thu, Jan 26, 2023 at 5:26 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > > >
> > > > > On Wed, 18 Jan 2023 at 04:41, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > > > >
> > > > > > I believe that I am still having issues occur within Ubuntu 22.10 with
> > > > > > the 5.19 version of the kernel that might be associated with this
> > > > > > discussion.  I apologize up front for any faux pas I make in writing
> > > > > > this email.
> > > > >
> > > > > No need to apologize.   The fix in question went into v6.0 of the
> > > > > upstream kernel.  So apparently it's still missing from the distro you
> > > > > are using.
> > > >
> > > > Thank you for the reply! ---  I have upgraded the Kernel and it still
> > > > seems to be throwing errors.  Details follow:
> > > >
> > > > Distro: Ubuntu 22.10.
> > > > Upgraded kernel using mainline (mainline --install-latest)
> > > >
> > > > # uname -a
> > > > Linux instance-20220314-1510-fileserver-for-overlay
> > > > 6.1.8-060108-generic #202301240742 SMP PREEMPT_DYNAMIC Tue Jan 24
> > > > 08:13:53 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux
> > > >
> > > > On mount I still get the following notice in syslog (representative):
> > > > Jan 30 19:11:46 instance-20220314-1510-fileserver-for-overlay kernel:
> > > > [   71.613334] overlayfs: null uuid detected in lower fs '/', falling
> > > > back to xino=off,index=off,nfs_export=off.
> > > >
> > > > And on access (via samba) I still see the following errors in the
> > > > syslog (representative):
> > > > Jan 30 19:19:34 instance-20220314-1510-fileserver-for-overlay kernel:
> > > > [  539.181858] overlayfs: failed to retrieve lower fileattr (8020
> > > > MeOHH2O RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/Storage.mcf_idx,
> > > > err=-38)
> > > >
> > > > And on the Windows client, the software still fails with the same symptomology.
> > > >
> > > >
> > > >
> > > >
> > > > >
> > > > > > An example error from our syslog:
> > > > > >
> > > > > > kernel: [2702258.538549] overlayfs: failed to retrieve lower fileattr
> > > > > > (8020 MeOHH2O
> > > > > > RecoverySample2-20221219-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1722.d/analysis.tsf,
> > > > > > err=-38)
> > > > >
> > > > > Yep, looks like the same bug.
> > > > >
> > > > > Thanks,
> > > > > Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-03-07 17:14                                       ` Jonathan Katz
@ 2023-03-09 15:31                                         ` Miklos Szeredi
  2023-03-15  2:43                                           ` Jonathan Katz
  0 siblings, 1 reply; 28+ messages in thread
From: Miklos Szeredi @ 2023-03-09 15:31 UTC (permalink / raw)
  To: jonathan
  Cc: Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

On Tue, 7 Mar 2023 at 18:14, Jonathan Katz <jkatz@eitmlabs.org> wrote:
>
> On Tue, Mar 7, 2023 at 12:38 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Tue, 7 Mar 2023 at 02:12, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > >
> > > Hi all,
> > >
> > > In pursuing this issue, I downloaded the kernel source to see if I
> > > could debug it further.  In so doing, it looks like Christian's patch
> > > was never committed to the main source tree (sorry if my terminology
> > > is wrong).  This is up to and including the 6.3-rc1.  I could also
> > > find no mention of the fix in the log.
> > >
> > > I am trying to manually apply this patch now, but, I am wondering if
> > > there was some reason that it was not applied (e.g. it introduces some
> > > instability?)?
> >
> > It's fixing the bug in the wrong place, i.e. it's checking for an
> > -ENOSYS return from vfs_fileattr_get(), but that return value is not
> > valid at that point.
> >
> > The right way to fix this bug is to prevent -ENOSYS from being
> > returned in the first place.
> >
> > Commit 02c0cab8e734 ("fuse: ioctl: translate ENOSYS") fixes one of
> > those bugs, but of course it's possible that I missed something in
> > that fix.
> >
> > Can you please first verify that an upstream kernel (>v6.0) can also
> > reproduce this issue?
>
> Got ya.  that makes a lot of sense, thank you.
>
> I have confirmed that I continue to get the error with 6.2 .
> quick summary of the lowerdir:
>    server ---- NFS(ro) ---- > client "/nfs"
>    client "/nfs" --- bindfs(uidmap) --- > client "/lower"

Can you please run bindfs in debugging mode (-d) and send the
resulting log after reproducing the issue?

Thanks,
Miklos

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-03-09 15:31                                         ` Miklos Szeredi
@ 2023-03-15  2:43                                           ` Jonathan Katz
  2023-03-22 18:42                                             ` Jonathan Katz
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Katz @ 2023-03-15  2:43 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: jonathan, Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

On Thu, Mar 9, 2023 at 7:31 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
>
> On Tue, 7 Mar 2023 at 18:14, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> >
> > On Tue, Mar 7, 2023 at 12:38 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Tue, 7 Mar 2023 at 02:12, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > > >
> > > > Hi all,
> > > >
> > > > In pursuing this issue, I downloaded the kernel source to see if I
> > > > could debug it further.  In so doing, it looks like Christian's patch
> > > > was never committed to the main source tree (sorry if my terminology
> > > > is wrong).  This is up to and including the 6.3-rc1.  I could also
> > > > find no mention of the fix in the log.
> > > >
> > > > I am trying to manually apply this patch now, but, I am wondering if
> > > > there was some reason that it was not applied (e.g. it introduces some
> > > > instability?)?
> > >
> > > It's fixing the bug in the wrong place, i.e. it's checking for an
> > > -ENOSYS return from vfs_fileattr_get(), but that return value is not
> > > valid at that point.
> > >
> > > The right way to fix this bug is to prevent -ENOSYS from being
> > > returned in the first place.
> > >
> > > Commit 02c0cab8e734 ("fuse: ioctl: translate ENOSYS") fixes one of
> > > those bugs, but of course it's possible that I missed something in
> > > that fix.
> > >
> > > Can you please first verify that an upstream kernel (>v6.0) can also
> > > reproduce this issue?
> >
> > Got ya.  that makes a lot of sense, thank you.
> >
> > I have confirmed that I continue to get the error with 6.2 .
> > quick summary of the lowerdir:
> >    server ---- NFS(ro) ---- > client "/nfs"
> >    client "/nfs" --- bindfs(uidmap) --- > client "/lower"
>
> Can you please run bindfs in debugging mode (-d) and send the
> resulting log after reproducing the issue?
>
> Thanks,
> Miklos

OUCH -- MY LAST EMAIL WAS REJECTED FOR BEING TOO BIG
I HOPE THAT I AM SUMMARIZING THE RELEVANT INFORMATION HERE:


Hi Miklos, thank you.... I am sorry for the delay.

The log is somewhat long and was sent in a separate email.

I broke up the log into entries to try to match the chronology of actions:
   * ENTRY 1 nfs mount the external drive
   * ENTRY 2 perform the bind fs
   * ENTRY 3 perform the overlay
   * ENTRY 4 restart smb
   * ENTRY 5 mount the filesystem on a windows box
   * ENTRY 6 performing some navigation on the windows file explorer
   * ENTRY 7 attempt to open a data file with the windows application.

The only place that generated a kernel error in dmesg was at ENTRY 7.

Because the logs are so big, I tried to parse them, I may have made a
mistake or omitted information -- if you think so, as mentioned, the
full bindfs logs were sent separately


Here is my attempt to parse out the errors associated with this dmesg entry:

[ 1925.705908] overlayfs: failed to retrieve lower fileattr (8020
MeOHH2O RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data.sqlite,
err=-38)

--
unique: 1550, opcode: GETXATTR (22), nodeid: 71, insize: 73, pid: 3458
getxattr /eimstims1/deleteme2/8020 MeOHH2O
RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
trusted.overlay.metacopy 0
   unique: 1550, error: -95 (Operation not supported), outsize: 16
--
unique: 3922, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
getxattr /eimstims1/deleteme2/8020 MeOHH2O
RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
system.posix_acl_access 132
   unique: 3922, error: -95 (Operation not supported), outsize: 16
--
unique: 3954, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
getxattr /eimstims1/deleteme2/8020 MeOHH2O
RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
system.posix_acl_access 132
   unique: 3954, error: -95 (Operation not supported), outsize: 16
--
unique: 3960, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
getxattr /eimstims1/deleteme2/8020 MeOHH2O
RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
system.posix_acl_access 132
   unique: 3960, error: -95 (Operation not supported), outsize: 16


Thank you again!

-Jonathan

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-03-15  2:43                                           ` Jonathan Katz
@ 2023-03-22 18:42                                             ` Jonathan Katz
  2023-04-21 14:26                                               ` Miklos Szeredi
  0 siblings, 1 reply; 28+ messages in thread
From: Jonathan Katz @ 2023-03-22 18:42 UTC (permalink / raw)
  To: Miklos Szeredi
  Cc: jonathan, Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

Confirmed bindfs interaction:

Based on your bindfs comment I redid my configuration as follows:
ORIGINAL  (FAILS):
    FS1 - exports "/Data"  (nfs)
    FS2 - Mounts "/Data", does a bindfs, does an overlay

TEST (SUCCEEDS):
    FS1 - does a bindfs and exports a series of directories:
              # bindfs -u 5007, -g 5007 /Data /Data-jiajun
              /etc/exports:
                    /Data  machine.org(ro,sync,no_subtree_check)
                    /Data-jiajun machine.org(ro,fsid=12,sync,no_subtree_check)
     FS2 - used to do bindfs to make the lowers, but, now mounts
"/Data-jiajun" as the lower
               FS2 then does the overlay and samba share.
                It would not let me do the 2nd export if I did not
include the fsid entry....

WOOT WOOT.


Not an ideal solution as I have to make changes to 2 servers in order
to accomplish my goal :/.








On Tue, Mar 14, 2023 at 7:43 PM Jonathan Katz <jkatz@eitmlabs.org> wrote:
>
> On Thu, Mar 9, 2023 at 7:31 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> >
> > On Tue, 7 Mar 2023 at 18:14, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > >
> > > On Tue, Mar 7, 2023 at 12:38 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > >
> > > > On Tue, 7 Mar 2023 at 02:12, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > > > >
> > > > > Hi all,
> > > > >
> > > > > In pursuing this issue, I downloaded the kernel source to see if I
> > > > > could debug it further.  In so doing, it looks like Christian's patch
> > > > > was never committed to the main source tree (sorry if my terminology
> > > > > is wrong).  This is up to and including the 6.3-rc1.  I could also
> > > > > find no mention of the fix in the log.
> > > > >
> > > > > I am trying to manually apply this patch now, but, I am wondering if
> > > > > there was some reason that it was not applied (e.g. it introduces some
> > > > > instability?)?
> > > >
> > > > It's fixing the bug in the wrong place, i.e. it's checking for an
> > > > -ENOSYS return from vfs_fileattr_get(), but that return value is not
> > > > valid at that point.
> > > >
> > > > The right way to fix this bug is to prevent -ENOSYS from being
> > > > returned in the first place.
> > > >
> > > > Commit 02c0cab8e734 ("fuse: ioctl: translate ENOSYS") fixes one of
> > > > those bugs, but of course it's possible that I missed something in
> > > > that fix.
> > > >
> > > > Can you please first verify that an upstream kernel (>v6.0) can also
> > > > reproduce this issue?
> > >
> > > Got ya.  that makes a lot of sense, thank you.
> > >
> > > I have confirmed that I continue to get the error with 6.2 .
> > > quick summary of the lowerdir:
> > >    server ---- NFS(ro) ---- > client "/nfs"
> > >    client "/nfs" --- bindfs(uidmap) --- > client "/lower"
> >
> > Can you please run bindfs in debugging mode (-d) and send the
> > resulting log after reproducing the issue?
> >
> > Thanks,
> > Miklos
>
> OUCH -- MY LAST EMAIL WAS REJECTED FOR BEING TOO BIG
> I HOPE THAT I AM SUMMARIZING THE RELEVANT INFORMATION HERE:
>
>
> Hi Miklos, thank you.... I am sorry for the delay.
>
> The log is somewhat long and was sent in a separate email.
>
> I broke up the log into entries to try to match the chronology of actions:
>    * ENTRY 1 nfs mount the external drive
>    * ENTRY 2 perform the bind fs
>    * ENTRY 3 perform the overlay
>    * ENTRY 4 restart smb
>    * ENTRY 5 mount the filesystem on a windows box
>    * ENTRY 6 performing some navigation on the windows file explorer
>    * ENTRY 7 attempt to open a data file with the windows application.
>
> The only place that generated a kernel error in dmesg was at ENTRY 7.
>
> Because the logs are so big, I tried to parse them, I may have made a
> mistake or omitted information -- if you think so, as mentioned, the
> full bindfs logs were sent separately
>
>
> Here is my attempt to parse out the errors associated with this dmesg entry:
>
> [ 1925.705908] overlayfs: failed to retrieve lower fileattr (8020
> MeOHH2O RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data.sqlite,
> err=-38)
>
> --
> unique: 1550, opcode: GETXATTR (22), nodeid: 71, insize: 73, pid: 3458
> getxattr /eimstims1/deleteme2/8020 MeOHH2O
> RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> trusted.overlay.metacopy 0
>    unique: 1550, error: -95 (Operation not supported), outsize: 16
> --
> unique: 3922, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
> getxattr /eimstims1/deleteme2/8020 MeOHH2O
> RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> system.posix_acl_access 132
>    unique: 3922, error: -95 (Operation not supported), outsize: 16
> --
> unique: 3954, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
> getxattr /eimstims1/deleteme2/8020 MeOHH2O
> RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> system.posix_acl_access 132
>    unique: 3954, error: -95 (Operation not supported), outsize: 16
> --
> unique: 3960, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
> getxattr /eimstims1/deleteme2/8020 MeOHH2O
> RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> system.posix_acl_access 132
>    unique: 3960, error: -95 (Operation not supported), outsize: 16
>
>
> Thank you again!
>
> -Jonathan

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

* Re: [PATCH] [REGRESSION] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs
  2023-03-22 18:42                                             ` Jonathan Katz
@ 2023-04-21 14:26                                               ` Miklos Szeredi
  0 siblings, 0 replies; 28+ messages in thread
From: Miklos Szeredi @ 2023-04-21 14:26 UTC (permalink / raw)
  To: jonathan
  Cc: Christian Kohlschütter, Linus Torvalds, overlayfs,
	linux-kernel, linux-fsdevel

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

Hi Jonathan,

Can you please try out the attached patch?

Thanks,
Miklos


On Wed, 22 Mar 2023 at 19:42, Jonathan Katz <jkatz@eitmlabs.org> wrote:
>
> Confirmed bindfs interaction:
>
> Based on your bindfs comment I redid my configuration as follows:
> ORIGINAL  (FAILS):
>     FS1 - exports "/Data"  (nfs)
>     FS2 - Mounts "/Data", does a bindfs, does an overlay
>
> TEST (SUCCEEDS):
>     FS1 - does a bindfs and exports a series of directories:
>               # bindfs -u 5007, -g 5007 /Data /Data-jiajun
>               /etc/exports:
>                     /Data  machine.org(ro,sync,no_subtree_check)
>                     /Data-jiajun machine.org(ro,fsid=12,sync,no_subtree_check)
>      FS2 - used to do bindfs to make the lowers, but, now mounts
> "/Data-jiajun" as the lower
>                FS2 then does the overlay and samba share.
>                 It would not let me do the 2nd export if I did not
> include the fsid entry....
>
> WOOT WOOT.
>
>
> Not an ideal solution as I have to make changes to 2 servers in order
> to accomplish my goal :/.
>
>
>
>
>
>
>
>
> On Tue, Mar 14, 2023 at 7:43 PM Jonathan Katz <jkatz@eitmlabs.org> wrote:
> >
> > On Thu, Mar 9, 2023 at 7:31 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > >
> > > On Tue, 7 Mar 2023 at 18:14, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > > >
> > > > On Tue, Mar 7, 2023 at 12:38 AM Miklos Szeredi <miklos@szeredi.hu> wrote:
> > > > >
> > > > > On Tue, 7 Mar 2023 at 02:12, Jonathan Katz <jkatz@eitmlabs.org> wrote:
> > > > > >
> > > > > > Hi all,
> > > > > >
> > > > > > In pursuing this issue, I downloaded the kernel source to see if I
> > > > > > could debug it further.  In so doing, it looks like Christian's patch
> > > > > > was never committed to the main source tree (sorry if my terminology
> > > > > > is wrong).  This is up to and including the 6.3-rc1.  I could also
> > > > > > find no mention of the fix in the log.
> > > > > >
> > > > > > I am trying to manually apply this patch now, but, I am wondering if
> > > > > > there was some reason that it was not applied (e.g. it introduces some
> > > > > > instability?)?
> > > > >
> > > > > It's fixing the bug in the wrong place, i.e. it's checking for an
> > > > > -ENOSYS return from vfs_fileattr_get(), but that return value is not
> > > > > valid at that point.
> > > > >
> > > > > The right way to fix this bug is to prevent -ENOSYS from being
> > > > > returned in the first place.
> > > > >
> > > > > Commit 02c0cab8e734 ("fuse: ioctl: translate ENOSYS") fixes one of
> > > > > those bugs, but of course it's possible that I missed something in
> > > > > that fix.
> > > > >
> > > > > Can you please first verify that an upstream kernel (>v6.0) can also
> > > > > reproduce this issue?
> > > >
> > > > Got ya.  that makes a lot of sense, thank you.
> > > >
> > > > I have confirmed that I continue to get the error with 6.2 .
> > > > quick summary of the lowerdir:
> > > >    server ---- NFS(ro) ---- > client "/nfs"
> > > >    client "/nfs" --- bindfs(uidmap) --- > client "/lower"
> > >
> > > Can you please run bindfs in debugging mode (-d) and send the
> > > resulting log after reproducing the issue?
> > >
> > > Thanks,
> > > Miklos
> >
> > OUCH -- MY LAST EMAIL WAS REJECTED FOR BEING TOO BIG
> > I HOPE THAT I AM SUMMARIZING THE RELEVANT INFORMATION HERE:
> >
> >
> > Hi Miklos, thank you.... I am sorry for the delay.
> >
> > The log is somewhat long and was sent in a separate email.
> >
> > I broke up the log into entries to try to match the chronology of actions:
> >    * ENTRY 1 nfs mount the external drive
> >    * ENTRY 2 perform the bind fs
> >    * ENTRY 3 perform the overlay
> >    * ENTRY 4 restart smb
> >    * ENTRY 5 mount the filesystem on a windows box
> >    * ENTRY 6 performing some navigation on the windows file explorer
> >    * ENTRY 7 attempt to open a data file with the windows application.
> >
> > The only place that generated a kernel error in dmesg was at ENTRY 7.
> >
> > Because the logs are so big, I tried to parse them, I may have made a
> > mistake or omitted information -- if you think so, as mentioned, the
> > full bindfs logs were sent separately
> >
> >
> > Here is my attempt to parse out the errors associated with this dmesg entry:
> >
> > [ 1925.705908] overlayfs: failed to retrieve lower fileattr (8020
> > MeOHH2O RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data.sqlite,
> > err=-38)
> >
> > --
> > unique: 1550, opcode: GETXATTR (22), nodeid: 71, insize: 73, pid: 3458
> > getxattr /eimstims1/deleteme2/8020 MeOHH2O
> > RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> > trusted.overlay.metacopy 0
> >    unique: 1550, error: -95 (Operation not supported), outsize: 16
> > --
> > unique: 3922, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
> > getxattr /eimstims1/deleteme2/8020 MeOHH2O
> > RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> > system.posix_acl_access 132
> >    unique: 3922, error: -95 (Operation not supported), outsize: 16
> > --
> > unique: 3954, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
> > getxattr /eimstims1/deleteme2/8020 MeOHH2O
> > RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> > system.posix_acl_access 132
> >    unique: 3954, error: -95 (Operation not supported), outsize: 16
> > --
> > unique: 3960, opcode: GETXATTR (22), nodeid: 71, insize: 72, pid: 3458
> > getxattr /eimstims1/deleteme2/8020 MeOHH2O
> > RecoverySample1-20221216-A-JJL-WebinarHilic10C-TOF-TT54-Neg-1632.d/chromatography-data-pre.sqlite
> > system.posix_acl_access 132
> >    unique: 3960, error: -95 (Operation not supported), outsize: 16
> >
> >
> > Thank you again!
> >
> > -Jonathan

[-- Attachment #2: fuse-ioctl-translate-enosys-in-outarg.patch --]
[-- Type: application/x-patch, Size: 2388 bytes --]

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

end of thread, other threads:[~2023-04-21 14:27 UTC | newest]

Thread overview: 28+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-04 18:36 [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs Christian Kohlschütter
2022-07-18  9:14 ` Miklos Szeredi
2022-07-18 10:10   ` Christian Kohlschütter
2022-07-18 10:31     ` Miklos Szeredi
2022-07-18 10:56       ` Christian Kohlschütter
2022-07-18 12:21         ` Miklos Szeredi
2022-07-18 13:03           ` [PATCH] [REGRESSION] " Christian Kohlschütter
2022-07-18 13:13             ` Miklos Szeredi
2022-07-18 14:25               ` Christian Kohlschütter
2022-07-18 15:02                 ` Antonio SJ Musumeci
2022-07-18 17:23                 ` Miklos Szeredi
2022-07-18 18:29               ` Linus Torvalds
2022-07-18 19:04                 ` Christian Kohlschütter
2022-07-18 19:17                   ` Linus Torvalds
2022-07-18 19:27                     ` Miklos Szeredi
2022-07-18 20:12                       ` Linus Torvalds
2022-07-18 20:33                         ` Christian Kohlschütter
2023-01-18  3:41                           ` Jonathan Katz
2023-01-26 13:26                             ` Miklos Szeredi
2023-01-30 19:27                               ` Jonathan Katz
2023-02-23 23:11                                 ` Jonathan Katz
2023-03-07  1:12                                   ` Jonathan Katz
2023-03-07  8:38                                     ` Miklos Szeredi
2023-03-07 17:14                                       ` Jonathan Katz
2023-03-09 15:31                                         ` Miklos Szeredi
2023-03-15  2:43                                           ` Jonathan Katz
2023-03-22 18:42                                             ` Jonathan Katz
2023-04-21 14:26                                               ` Miklos Szeredi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).