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.
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.
Thanks,
NeilBrown
Attachment:
signature.asc
Description: PGP signature
