linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-fsdevel@vger.kernel.org, overlayfs <linux-unionfs@vger.kernel.org>
Subject: Re: [RFC][PATCH] ovl_create_index(): vfs_mkdir() might succeed leaving dentry negative unhashed
Date: Mon, 14 May 2018 16:39:00 +0100	[thread overview]
Message-ID: <20180514153900.GG30522@ZenIV.linux.org.uk> (raw)
In-Reply-To: <CAJfpegvU9BM+BL7CyxERNnYNpdG=hpXSZSNL7KcZ4-qR3XUbcA@mail.gmail.com>

On Mon, May 14, 2018 at 05:04:44PM +0200, Miklos Szeredi wrote:
> On Sat, May 12, 2018 at 3:25 AM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> > [same story as with the previous two patches]
> >
> > Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> > ---
> > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> > index 8bede0742619..cdd8f8816d2a 100644
> > --- a/fs/overlayfs/copy_up.c
> > +++ b/fs/overlayfs/copy_up.c
> > @@ -373,6 +373,22 @@ static int ovl_create_index(struct dentry *dentry, struct dentry *origin,
> >         if (err)
> >                 goto out;
> >
> > +       if (unlikely(d_unhashed(temp))) {
> > +               struct dentry *d = lookup_one_len(temp->d_name.name,
> > +                                                 temp->d_parent,
> > +                                                 temp->d_name.len);
> > +               if (IS_ERR(d)) {
> > +                       err = PTR_ERR(d);
> > +                       goto out;
> 
> This violates the "If -1 is returned, no directory shall be created" rule.

So does NFS, for that matter...  If you use weird filesystems as layers,
you get corner cases.  Note that on NFS we'll probably have to go for
that possibility (== argument unhashed negative after success) on mkdir,
in case when it's reexported and somebody is playing silly buggers with
races.

> lookup_one_len() does various permission checks.  The normal DAC check
> is not a worry, because of the lock on the parent. But is it
> guaranteed that MAC allows lookup if it allowed mkdir?

*shrug*

Or it can be set so that rmdir won't be allowed afterwards, or...
There are all kinds of ways to set idiotic LSM that would screw overlayfs.
Doctor, it hurts when I do it...  But then you know what I think of
Linux S&M community in general...

Right now you code would fail with -EIO in all those cases, AFAICS.  So you
might get a stray -EPERM...

      reply	other threads:[~2018-05-14 15:39 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-12  1:25 [RFC][PATCH] ovl_create_index(): vfs_mkdir() might succeed leaving dentry negative unhashed Al Viro
2018-05-12  1:29 ` [PATCH][RFC] ovl_create_real(): make it cope with vfs_mkdir() safely Al Viro
2018-05-12  5:13   ` Amir Goldstein
2018-05-12  8:55     ` Amir Goldstein
2018-05-12  4:49 ` [RFC][PATCH] ovl_create_index(): vfs_mkdir() might succeed leaving dentry negative unhashed Amir Goldstein
2018-05-13 22:50   ` Al Viro
2018-05-14 15:04 ` Miklos Szeredi
2018-05-14 15:39   ` Al Viro [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180514153900.GG30522@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).