linux-kernel - Re: [GIT PULL] overlayfs update for 4.14

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:   Thu, 14 Sep 2017 08:46:57 +0200
From:   Miklos Szeredi <miklos@...redi.hu>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Al Viro <viro@...iv.linux.org.uk>,
        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 update for 4.14

On Wed, Sep 13, 2017 at 6:25 PM, Linus Torvalds
<torvalds@...ux-foundation.org> wrote:
> On Wed, Sep 13, 2017 at 7:05 AM, Miklos Szeredi <miklos@...redi.hu> wrote:
>>
>> There are also some bug fixes, one in particular (random ioctl's shouldn't be
>> able to modify lower layers) that touches some vfs code, but of course no-op for
>> non-overlay fs.
>
> Argh. I _despise_ those changes to 'd_real()'.
>
> I reall ythink you should have made it a bit in the f_flags argument
> instead. The whole "pick the upper file" is _very_ similar to the
> file->f_flags bits conceptually - those change how accesses are done
> in other ways (eg O_DIRECT etc), and it's entirely possible that some
> day maybe you'd even want to expose it to user space as a O_UPPER flag
> to open (which would then only succeed if the file is in the upper
> overlay, and never open the lower filesystem).
>
> So adding _another_ field that is only for overlayfs and makes
> absolutely no sense in any other context is nasty. It's wrong. That's
> not a "VFS layer interface" any more. It's just an ugly hack that
> makes no sense outside of overlayfs.

d_real() is currently is *the* overlayfs-op:

$ git grep -l "\.d_real"
fs/overlayfs/super.c

It might find other uses in the future, but for now that's the fact.

I could have merged D_REAL_UPPER into f_flags, but it would have
polluted the O_ namespace (which is a uapi)  with an internal flag.
Look at the MS_ flags mess to see how mixing internal and external
flags is a really bad idea.

Changing internal interfaces is so much easier than userspace
interfaces.  So this is not set in stone, and since currently
overlayfs is the only user of the d_real api, it's really trivial to
change around if we find some other use for it.

But if we try to think up some interface  (e.g. the O_UPPER that you
suggested) then we'll be stuck with it even if it makes no sense, or
worse, causes problems.  So let's just wait until the need for
something comes up, then we can massage the internal interface to our
liking.

Thanks,
Miklos