linux-kernel - Re: [PATCH 01/13] filelock: push the S_ISREG check down to ->setlease handlers

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: <9a8d3d9e1ac6c5cb12d880410003fd0601e0abc2.camel@kernel.org>
Date: Tue, 14 Oct 2025 07:07:14 -0400
From: Jeff Layton <jlayton@...nel.org>
To: NeilBrown <neil@...wn.name>
Cc: Miklos Szeredi <miklos@...redi.hu>, Alexander Viro
 <viro@...iv.linux.org.uk>,  Christian Brauner	 <brauner@...nel.org>, Jan
 Kara <jack@...e.cz>, Chuck Lever	 <chuck.lever@...cle.com>, Alexander Aring
 <alex.aring@...il.com>, Trond Myklebust <trondmy@...nel.org>, Anna
 Schumaker <anna@...nel.org>, Steve French <sfrench@...ba.org>,  Paulo
 Alcantara	 <pc@...guebit.org>, Ronnie Sahlberg <ronniesahlberg@...il.com>,
 Shyam Prasad N	 <sprasad@...rosoft.com>, Tom Talpey <tom@...pey.com>,
 Bharath SM	 <bharathsm@...rosoft.com>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>,  "Rafael J. Wysocki"	 <rafael@...nel.org>,
 Danilo Krummrich <dakr@...nel.org>, David Howells	 <dhowells@...hat.com>,
 Tyler Hicks <code@...icks.com>, Olga Kornievskaia	 <okorniev@...hat.com>,
 Dai Ngo <Dai.Ngo@...cle.com>, Amir Goldstein	 <amir73il@...il.com>, Namjae
 Jeon <linkinjeon@...nel.org>, Steve French	 <smfrench@...il.com>, Sergey
 Senozhatsky <senozhatsky@...omium.org>, Carlos Maiolino <cem@...nel.org>,
 Kuniyuki Iwashima <kuniyu@...gle.com>, "David S. Miller"	
 <davem@...emloft.net>, Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski	
 <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>, Simon Horman	
 <horms@...nel.org>, linux-fsdevel@...r.kernel.org,
 linux-kernel@...r.kernel.org, 	linux-nfs@...r.kernel.org,
 linux-cifs@...r.kernel.org, 	samba-technical@...ts.samba.org,
 netfs@...ts.linux.dev, ecryptfs@...r.kernel.org, 
	linux-unionfs@...r.kernel.org, linux-xfs@...r.kernel.org,
 netdev@...r.kernel.org
Subject: Re: [PATCH 01/13] filelock: push the S_ISREG check down to
 ->setlease handlers

On Tue, 2025-10-14 at 16:37 +1100, NeilBrown wrote:
> On Tue, 14 Oct 2025, Jeff Layton wrote:
> > When nfsd starts requesting directory delegations, setlease handlers may
> > see requests for leases on directories. Push the !S_ISREG check down
> > into the non-trivial setlease handlers, so we can selectively enable
> > them where they're supported.
> > 
> > FUSE is special: It's the only filesystem that supports atomic_open and
> > allows kernel-internal leases. Ensure that we don't allow directory
> > leases by default going forward by explicitly disabling them there.
> 
> What is special about atomic_open w.r.t leases?
> 

Good question:

We want to break the parent's lease when creating new files, but
opening an existing file should not. Using atomic_open implies that the
VFS doesn't know the state of the dentry yet. If it doesn't exist and
the file is created during the atomic_open, it'll be too late for the
VFS to break the lease. So, if your filesystem supports atomic_open,
but uses the standard kernel-internal lease implementation (like FUSE
does), then it can't properly handle directory leases.

This could probably be fixed by FUSE implementing its own ->setlease
method that handles them properly, but that's beyond the scope of this
work (and would probably require plumbing in libfuse and the underlying
filesystems). Since directory leases are brand-new, the simplest way
around this is to just blanket deny them on FUSE for now.

I'll plan to write something along these lines for the changelog.

> 
> > 
> > Signed-off-by: Jeff Layton <jlayton@...nel.org>
> > ---
> >  fs/fuse/dir.c          | 1 +
> >  fs/locks.c             | 5 +++--
> >  fs/nfs/nfs4file.c      | 2 ++
> >  fs/smb/client/cifsfs.c | 3 +++
> >  4 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
> > index ecaec0fea3a132e7cbb88121e7db7fb504d57d3c..667774cc72a1d49796f531fcb342d2e4878beb85 100644
> > --- a/fs/fuse/dir.c
> > +++ b/fs/fuse/dir.c
> > @@ -2230,6 +2230,7 @@ static const struct file_operations fuse_dir_operations = {
> >  	.fsync		= fuse_dir_fsync,
> >  	.unlocked_ioctl	= fuse_dir_ioctl,
> >  	.compat_ioctl	= fuse_dir_compat_ioctl,
> > +	.setlease	= simple_nosetlease,
> >  };
> >  
> >  static const struct inode_operations fuse_common_inode_operations = {
> > diff --git a/fs/locks.c b/fs/locks.c
> > index 04a3f0e2072461b6e2d3d1cd12f2b089d69a7db3..0b16921fb52e602ea2e0c3de39d9d772af98ba7d 100644
> > --- a/fs/locks.c
> > +++ b/fs/locks.c
> > @@ -1929,6 +1929,9 @@ static int generic_delete_lease(struct file *filp, void *owner)
> >  int generic_setlease(struct file *filp, int arg, struct file_lease **flp,
> >  			void **priv)
> >  {
> > +	if (!S_ISREG(file_inode(filp)->i_mode))
> > +		return -EINVAL;
> > +
> >  	switch (arg) {
> >  	case F_UNLCK:
> >  		return generic_delete_lease(filp, *priv);
> > @@ -2018,8 +2021,6 @@ vfs_setlease(struct file *filp, int arg, struct file_lease **lease, void **priv)
> >  
> >  	if ((!vfsuid_eq_kuid(vfsuid, current_fsuid())) && !capable(CAP_LEASE))
> >  		return -EACCES;
> > -	if (!S_ISREG(inode->i_mode))
> > -		return -EINVAL;
> >  	error = security_file_lock(filp, arg);
> >  	if (error)
> >  		return error;
> > diff --git a/fs/nfs/nfs4file.c b/fs/nfs/nfs4file.c
> > index 7f43e890d3564a000dab9365048a3e17dc96395c..7317f26892c5782a39660cae87ec1afea24e36c0 100644
> > --- a/fs/nfs/nfs4file.c
> > +++ b/fs/nfs/nfs4file.c
> > @@ -431,6 +431,8 @@ void nfs42_ssc_unregister_ops(void)
> >  static int nfs4_setlease(struct file *file, int arg, struct file_lease **lease,
> >  			 void **priv)
> >  {
> > +	if (!S_ISREG(file_inode(file)->i_mode))
> > +		return -EINVAL;
> >  	return nfs4_proc_setlease(file, arg, lease, priv);
> >  }
> >  
> > diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
> > index 05b1fa76e8ccf1e86f0c174593cd6e1acb84608d..03c44c1d9bb631b87a8b67aa16e481d6bb3c7d14 100644
> > --- a/fs/smb/client/cifsfs.c
> > +++ b/fs/smb/client/cifsfs.c
> > @@ -1149,6 +1149,9 @@ cifs_setlease(struct file *file, int arg, struct file_lease **lease, void **priv
> >  	struct inode *inode = file_inode(file);
> >  	struct cifsFileInfo *cfile = file->private_data;
> >  
> > +	if (!S_ISREG(inode->i_mode))
> > +		return -EINVAL;
> > +
> >  	/* Check if file is oplocked if this is request for new lease */
> >  	if (arg == F_UNLCK ||
> >  	    ((arg == F_RDLCK) && CIFS_CACHE_READ(CIFS_I(inode))) ||
> > 
> > -- 
> > 2.51.0
> > 
> > 

-- 
Jeff Layton <jlayton@...nel.org>