linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miklos Szeredi <miklos@szeredi.hu>
To: Amir Goldstein <amir73il@gmail.com>
Cc: Miklos Szeredi <mszeredi@redhat.com>,
	overlayfs <linux-unionfs@vger.kernel.org>,
	linux-fsdevel <linux-fsdevel@vger.kernel.org>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 07/11] ovl: make ovl_create_real() cope with vfs_mkdir() safely
Date: Wed, 30 May 2018 10:18:03 +0200	[thread overview]
Message-ID: <CAJfpegv1HLghFSczd1oZbrpOaz39r8VD5M47aH1eVJFQxs=keg@mail.gmail.com> (raw)
In-Reply-To: <CAOQ4uxg6hVd2k5T20N1u72Of9Eb3O_EAasC0MhDoLY3-o2UrGw@mail.gmail.com>

On Tue, May 29, 2018 at 5:29 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, May 29, 2018 at 5:41 PM, Miklos Szeredi <mszeredi@redhat.com> wrote:
>> From: Amir Goldstein <amir73il@gmail.com>
>>
>> vfs_mkdir() may succeed and leave the dentry passed to it unhashed and
>> negative.  ovl_create_real() is the last caller breaking when that
>> happens.
>>
>> [amir: split re-factoring of ovl_create_temp() to prep patch
>>        add comment about unhashed dir after mkdir
>>        add pr_warn() if mkdir succeeds and lookup fails]
>>
>> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
>> Signed-off-by: Amir Goldstein <amir73il@gmail.com>
>> Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
>> ---
>>  fs/overlayfs/dir.c | 35 +++++++++++++++++++++++++++++++++--
>>  1 file changed, 33 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
>> index 1b181292a624..b33d37d1a87c 100644
>> --- a/fs/overlayfs/dir.c
>> +++ b/fs/overlayfs/dir.c
>> @@ -114,6 +114,37 @@ int ovl_cleanup_and_whiteout(struct dentry *workdir, struct inode *dir,
>>         goto out;
>>  }
>>
>> +static struct dentry *ovl_mkdir_real(struct inode *dir, struct dentry *dentry,
>> +                                    umode_t mode)
>> +{
>> +       int err;
>> +
>> +       err = ovl_do_mkdir(dir, dentry, mode);
>> +       if (err) {
>> +               dput(dentry);
>> +               return ERR_PTR(err);
>> +       }
>> +
>> +       /*
>> +        * vfs_mkdir() may succeed and leave the dentry passed
>> +        * to it unhashed and negative. If that happens, try to
>> +        * lookup a new hashed and positive dentry.
>> +        */
>> +       if (unlikely(d_unhashed(dentry))) {
>> +               struct dentry *d;
>> +
>> +               d = lookup_one_len(dentry->d_name.name, dentry->d_parent,
>> +                                  dentry->d_name.len);
>> +               if (IS_ERR(d)) {
>> +                       pr_warn("overlayfs: failed lookup after mkdir (%pd2, err=%i).\n",
>> +                               dentry, err);
>> +               }
>> +               dput(dentry);
>> +               dentry = d;
>> +       }
>> +       return dentry;
>> +}
>> +
>>  struct dentry *ovl_create_real(struct inode *dir, struct dentry *newdentry,
>>                                struct ovl_cattr *attr)
>>  {
>> @@ -135,8 +166,8 @@ struct dentry *ovl_create_real(struct inode *dir, struct dentry *newdentry,
>>                         break;
>>
>>                 case S_IFDIR:
>> -                       err = ovl_do_mkdir(dir, newdentry, attr->mode);
>> -                       break;
>> +                       /* mkdir is special... */
>> +                       return ovl_mkdir_real(dir, newdentry, attr->mode);
>>
>
> I like your version with the helper.
> So do we not care about the WARN_ON below on non-instantiated dentry
> for directories? or we don't need it at all?

I think we do.  Filesystems do all sorts of weird an unexpected
things.   We should definitely not assume underlying filesystem
returns in any way sane result.

Fixed, thanks for spotting.

Miklos

  reply	other threads:[~2018-05-30  8:18 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-29 14:41 [PATCH 00/11] overlayfs fixes Miklos Szeredi
2018-05-29 14:41 ` [PATCH 01/11] ovl: update documentation for unionmount-testsuite Miklos Szeredi
2018-05-29 14:41 ` [PATCH 02/11] ovl: remove WARN_ON() real inode attributes mismatch Miklos Szeredi
2018-05-29 14:41 ` [PATCH 03/11] ovl: strip debug argument from ovl_do_ helpers Miklos Szeredi
2018-05-29 14:41 ` [PATCH 04/11] ovl: struct cattr cleanups Miklos Szeredi
2018-05-29 14:41 ` [PATCH 05/11] ovl: return dentry from ovl_create_real() Miklos Szeredi
2018-05-29 15:24   ` Amir Goldstein
2018-05-30  8:28     ` Miklos Szeredi
2018-05-29 14:41 ` [PATCH 06/11] ovl: create helper ovl_create_temp() Miklos Szeredi
2018-05-29 14:41 ` [PATCH 07/11] ovl: make ovl_create_real() cope with vfs_mkdir() safely Miklos Szeredi
2018-05-29 15:29   ` Amir Goldstein
2018-05-30  8:18     ` Miklos Szeredi [this message]
2018-05-29 14:41 ` [PATCH 08/11] ovl: clean up copy-up error paths Miklos Szeredi
2018-05-29 14:41 ` [PATCH 09/11] vfs: factor out inode_insert5() Miklos Szeredi
2018-06-10  5:49   ` Al Viro
2018-06-10  6:02     ` Al Viro
2018-06-11  9:15       ` Miklos Szeredi
2018-06-11 11:32         ` Miklos Szeredi
2018-06-11 16:43           ` Al Viro
2018-06-12 12:38             ` Miklos Szeredi
2018-05-29 14:41 ` [PATCH 10/11] ovl: Pass argument to ovl_get_inode() in a structure Miklos Szeredi
2018-05-29 14:41 ` [PATCH 11/11] ovl: use inode_insert5() to hash a newly created inode Miklos Szeredi

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='CAJfpegv1HLghFSczd1oZbrpOaz39r8VD5M47aH1eVJFQxs=keg@mail.gmail.com' \
    --to=miklos@szeredi.hu \
    --cc=amir73il@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-unionfs@vger.kernel.org \
    --cc=mszeredi@redhat.com \
    --subject='Re: [PATCH 07/11] ovl: make ovl_create_real() cope with vfs_mkdir() safely' \
    /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

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).