On Mon, 2018-11-26 at 09:58 +0800, Pan Bian wrote: > The function autofs_expire_run calls dput(dentry) to drop the reference > count of dentry. However, dentry is read via autofs_dentry_ino(dentry) > after that. This may result in a use-free-bug. The patch drops the > reference count of dentry only when it is never used. Yes, I agree this is a bug and it should be fixed. The autofs_expire_run() function is used for autofs v3 which is very old now so it's not likely to be called. But I think you are correct, if it is called the copy to user space should trigger a umount and (likley) remove the mount point directory, maybe I broke this at some point without realising it ... So thanks, I'll have a closer look but even if the ref counting isn't quite what either of us expect this is probably still worth while. If I don't see any reason to not do this I'll forward the patch to Andrew. Ian > > Signed-off-by: Pan Bian <bianpan2016@xxxxxxx> > --- > fs/autofs/expire.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/autofs/expire.c b/fs/autofs/expire.c > index d441244..28d9c2b 100644 > --- a/fs/autofs/expire.c > +++ b/fs/autofs/expire.c > @@ -596,7 +596,6 @@ int autofs_expire_run(struct super_block *sb, > pkt.len = dentry->d_name.len; > memcpy(pkt.name, dentry->d_name.name, pkt.len); > pkt.name[pkt.len] = '\0'; > - dput(dentry); > > if (copy_to_user(pkt_p, &pkt, sizeof(struct autofs_packet_expire))) > ret = -EFAULT; > @@ -609,6 +608,8 @@ int autofs_expire_run(struct super_block *sb, > complete_all(&ino->expire_complete); > spin_unlock(&sbi->fs_lock); > > + dput(dentry); > + > return ret; > } >
