linux-kernel - Re: [PATCH] ovl: Handle ENOSYS when fileattr support is missing in lower/upper fs

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <FFA26FD1-60EF-457E-B914-E1978CCC7B57@kohlschutter.com>
Date:   Mon, 18 Jul 2022 12:56:19 +0200
From:   Christian Kohlschütter 
        <christian@...lschutter.com>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     overlayfs <linux-unionfs@...r.kernel.org>,
        linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org
Subject: Re: [PATCH] ovl: Handle ENOSYS when fileattr support is missing in
 lower/upper fs


-- 
Dr. Christian Kohlschütter



> Am 18.07.2022 um 12:31 schrieb Miklos Szeredi <miklos@...redi.hu>:
> 
> On Mon, 18 Jul 2022 at 12:10, Christian Kohlschütter
> <christian@...lschutter.com> wrote:
>> 
>>> Am 18.07.2022 um 11:14 schrieb Miklos Szeredi <miklos@...redi.hu>:
>>> 
>>> On Mon, 4 Jul 2022 at 20:36, Christian Kohlschütter
>>> <christian@...lschutter.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.