linux-kernel - Re: [PATCH 4/6] fuse: return correct dentry for ->mkdir

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: <174036397835.74271.9038146946135155196@noble.neil.brown.name>
Date: Mon, 24 Feb 2025 13:26:18 +1100
From: "NeilBrown" <neilb@...e.de>
To: "Al Viro" <viro@...iv.linux.org.uk>
Cc: "Christian Brauner" <brauner@...nel.org>, "Jan Kara" <jack@...e.cz>,
 "Miklos Szeredi" <miklos@...redi.hu>, "Xiubo Li" <xiubli@...hat.com>,
 "Ilya Dryomov" <idryomov@...il.com>, "Richard Weinberger" <richard@....at>,
 "Anton Ivanov" <anton.ivanov@...bridgegreys.com>,
 "Johannes Berg" <johannes@...solutions.net>,
 "Trond Myklebust" <trondmy@...nel.org>, "Anna Schumaker" <anna@...nel.org>,
 "Chuck Lever" <chuck.lever@...cle.com>, "Jeff Layton" <jlayton@...nel.org>,
 "Olga Kornievskaia" <okorniev@...hat.com>, "Dai Ngo" <Dai.Ngo@...cle.com>,
 "Tom Talpey" <tom@...pey.com>,
 "Sergey Senozhatsky" <senozhatsky@...omium.org>,
 linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
 linux-cifs@...r.kernel.org, linux-nfs@...r.kernel.org,
 linux-um@...ts.infradead.org, ceph-devel@...r.kernel.org,
 netfs@...ts.linux.dev
Subject: Re: [PATCH 4/6] fuse: return correct dentry for ->mkdir

On Sat, 22 Feb 2025, Al Viro wrote:
> On Fri, Feb 21, 2025 at 10:36:33AM +1100, NeilBrown wrote:
> 
> > @@ -871,7 +870,12 @@ static int fuse_mknod(struct mnt_idmap *idmap, struct inode *dir,
> >  	args.in_args[0].value = &inarg;
> >  	args.in_args[1].size = entry->d_name.len + 1;
> >  	args.in_args[1].value = entry->d_name.name;
> > -	return create_new_entry(idmap, fm, &args, dir, entry, mode);
> > +	de = create_new_entry(idmap, fm, &args, dir, entry, mode);
> > +	if (IS_ERR(de))
> > +		return PTR_ERR(de);
> > +	if (de)
> > +		dput(de);
> > +	return 0;
> 
> Can that really happen?

Probably now.  It would require S_IFDIR to be passed in the mode to
vfs_mknod().  I don't think any current callers do that, but I don't see
any code in vfs_mknod() to prevent it.

> 
> > @@ -934,7 +939,12 @@ static int fuse_symlink(struct mnt_idmap *idmap, struct inode *dir,
> >  	args.in_args[1].value = entry->d_name.name;
> >  	args.in_args[2].size = len;
> >  	args.in_args[2].value = link;
> > -	return create_new_entry(idmap, fm, &args, dir, entry, S_IFLNK);
> > +	de = create_new_entry(idmap, fm, &args, dir, entry, S_IFLNK);
> > +	if (IS_ERR(de))
> > +		return PTR_ERR(de);
> > +	if (de)
> > +		dput(de);
> > +	return 0;
> 
> Same question.

That definitely cannot happen. - because we *know* that d_splice_alias()
never returns a dentry for any but an S_IFDIR inode (how might we
explain that to the rust type system I wonder :-).

I was going for "obviously correct" without try to optimise, but you are
correct that testing for a non-NULL non-ERR dentry should be optimsed
away as impossible in all cases except mkdir.

Thanks,
NeilBrown


> 
> > +	de = create_new_entry(&invalid_mnt_idmap, fm, &args, newdir, newent, inode->i_mode);
> > +	if (!IS_ERR(de)) {
> > +		if (de)
> > +			dput(de);
> > +		de = NULL;
> 
> Whoa...  Details, please.  What's going on here?
>