linux-kernel - Re: [GIT PULL] overlayfs fix for 4.8-rc5

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]

Date:   Fri, 9 Sep 2016 13:36:16 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        "linux-unionfs@...r.kernel.org" <linux-unionfs@...r.kernel.org>
Subject: Re: [GIT PULL] overlayfs fix for 4.8-rc5

On Fri, Sep 9, 2016 at 1:08 PM, Miklos Szeredi <miklos@...redi.hu> wrote:
>
> That code tries to remove ACL from a directory, and there are several cases:
>
> 1) success: that's good obviously
> 2) error: no ACL was found: that's also good
> 3) error: ACL's are not supported by the filesystem: this is also good
> 4) error: ACL was there but we failed to remove it for some other
> reason: this is not good
>
> The patch adds handling of case 3.

I'm not convinced your explanation is correct.

The thing is, you added a test for -EOPNOTSUPP, and that is in fact at
least partly case (2) (eg xattr_resolve_name())

And EOPNOTSUPP actually seems to be the _clear_ case. The ENODATA case
is the one that is hard to actually verify. I tried to see that "yes,
all filesystems return ENODATA", but it wasn't obvious at all (p9fs?)
If I read the cifs code right, it returns EOPNOTSUPP for the "not
found" case too.

And ext2/ext4 returns ERANGE for some "we don't support that" cases,
while gfs2 seems to return EINVAL for those cases. Those are obviously
also cases of (2), but the fuse code doesn't test for it.

So the error list seems to be rather random, and no, ENODATA and
EOPNOTSUPP do not seem to be the only errors that would match the
above at all.

I dunno. I guess this is a corner case that really doesn't matter in
practice, but the whole "let's test a few special cases" approach
fails the smell test to me, and doesn't actually seem to match your
cases above very well.

              Linus