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

[Jeff Layton]

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@xxxxxxxxxx>
> > ---
> >  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@xxxxxxxxxx>