linux-kernel - Re: [PATCH v3 00/24] Convert vfs.txt to vfs.rst

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: <894091e9742896e4bc810458a9f71f3f59a48860.camel@themaw.net>
Date:   Thu, 04 Apr 2019 07:28:52 +0800
From:   Ian Kent <raven@...maw.net>
To:     NeilBrown <neilb@...e.com>, Al Viro <viro@...iv.linux.org.uk>,
        Jonathan Corbet <corbet@....net>
Cc:     "Tobin C. Harding" <tobin@...nel.org>,
        Mauro Carvalho Chehab <mchehab@...pensource.com>,
        Randy Dunlap <rdunlap@...radead.org>,
        linux-doc@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 00/24] Convert vfs.txt to vfs.rst

On Wed, 2019-04-03 at 11:55 +1100, NeilBrown wrote:
> On Wed, Apr 03 2019, Ian Kent wrote:
> 
> > On Tue, 2019-04-02 at 20:08 +0100, Al Viro wrote:
> > > On Tue, Apr 02, 2019 at 06:54:01PM +0100, Al Viro wrote:
> > > > static void autofs_dentry_release(struct dentry *de)
> > > > {
> > > >         struct autofs_info *ino = autofs_dentry_ino(de);
> > > >         struct autofs_sb_info *sbi = autofs_sbi(de->d_sb);
> > > > 
> > > >         pr_debug("releasing %p\n", de);
> > > > 
> > > >         if (!ino)
> > > >                 return;
> > > > ...
> > > >         autofs_free_ino(ino);
> > > > }
> > > > with autofs_free_ino() being straight kfree().  Which means
> > > > that the lockless case of autofs_d_manage() can run into
> > > > autofs_dentry_ino(dentry) getting freed right under it.
> > > > 
> > > > And there we do have this reachable:
> > > > int autofs_expire_wait(const struct path *path, int rcu_walk)
> > > > {
> > > >         struct dentry *dentry = path->dentry;
> > > >         struct autofs_sb_info *sbi = autofs_sbi(dentry->d_sb);
> > > >         struct autofs_info *ino = autofs_dentry_ino(dentry);
> > > >         int status;
> > > >         int state;
> > > > 
> > > >         /* Block on any pending expire */
> > > >         if (!(ino->flags & AUTOFS_INF_WANT_EXPIRE))
> > 
> > Oh yes, this is saying the dentry hasn't been selected
> > for expire on the first pass, there's a second pass at
> > expire selection so there's a delay there and both flags
> > (this one and the expiring flag) are kept throughout the
> > expire operation if dentry is selected.
> > 
> > That might be partly why an oops has never been seen but
> > path walks can occur at any time so it's a bit puzzling.
> > 
> > LOL, and Neil probably can't remember the deeper detail
> > on what he did there now either.
> 
> It seems very likely that this was just a subtlety that I missed.
> I doesn't help that "ino" isn't actually and inode and isn't freed like
> an inode, but that is no excuse.

I've become accustom to the naming so that doesn't occur to
me, ;)

> 
> When we add the rcu_head linkage to 'struct autofs_info', we might as
> well remove the 'struct inode' from there - it doesn't seem to have been
> used for years.

That's a good point, I've thought about doing so several
times but haven't got around to it.

Ian