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

Jeff Layton posted 13 patches 2 months, 1 week ago
There is a newer version of this series
[PATCH 01/13] filelock: push the S_ISREG check down to ->setlease handlers
Posted by Jeff Layton 2 months, 1 week ago
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.

Signed-off-by: Jeff Layton <jlayton@kernel.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
Re: [PATCH 01/13] filelock: push the S_ISREG check down to ->setlease handlers
Posted by NeilBrown 2 months ago
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?

Thanks,
NeilBrown


> 
> Signed-off-by: Jeff Layton <jlayton@kernel.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
> 
> 
Re: [PATCH 01/13] filelock: push the S_ISREG check down to ->setlease handlers
Posted by Jeff Layton 2 months ago
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@kernel.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@kernel.org>