diff mbox series

nfsd: disallow file locking and delegations for NFSv4 proxy

Message ID 20241023145436.63240-1-snitzer@kernel.org (mailing list archive)
State New
Headers show
Series nfsd: disallow file locking and delegations for NFSv4 proxy | expand

Commit Message

Mike Snitzer Oct. 23, 2024, 2:54 p.m. UTC 
We do not and cannot support NFS proxy server file locking over
NFSv4.x for the same reason we don't do it for NFSv3: NFS proxy server
reboot cannot allow clients to recover locks because the source NFS
server has not rebooted, and so it is not in grace.  Since the source
NFS server is not in grace, it cannot offer any guarantees that the
file won't have been changed between the locks getting lost and any
attempt to recover/reclaim them.  The same applies to delegations and
any associated locks, so disallow them too.

Add EXPORT_OP_NOLOCKSUPPORT and exportfs_lock_op_is_unsupported(), set
EXPORT_OP_NOLOCKSUPPORT in nfs_export_ops and check for it in
nfsd4_lock(), nfsd4_locku() and nfs4_set_delegation().

Signed-off-by: Mike Snitzer <snitzer@kernel.org>
---
 fs/nfs/export.c          |  3 ++-
 fs/nfsd/nfs4state.c      | 20 ++++++++++++++++++++
 include/linux/exportfs.h | 14 ++++++++++++++
 3 files changed, 36 insertions(+), 1 deletion(-)

Comments

Chuck Lever Oct. 23, 2024, 3:03 p.m. UTC | #1 
On Wed, Oct 23, 2024 at 10:54:36AM -0400, Mike Snitzer wrote:
> We do not and cannot support NFS proxy server file locking over
> NFSv4.x for the same reason we don't do it for NFSv3: NFS proxy server
> reboot cannot allow clients to recover locks because the source NFS
> server has not rebooted, and so it is not in grace.  Since the source
> NFS server is not in grace, it cannot offer any guarantees that the
> file won't have been changed between the locks getting lost and any
> attempt to recover/reclaim them.  The same applies to delegations and
> any associated locks, so disallow them too.
> 
> Add EXPORT_OP_NOLOCKSUPPORT and exportfs_lock_op_is_unsupported(), set
> EXPORT_OP_NOLOCKSUPPORT in nfs_export_ops and check for it in
> nfsd4_lock(), nfsd4_locku() and nfs4_set_delegation().
> 
> Signed-off-by: Mike Snitzer <snitzer@kernel.org>
> ---
>  fs/nfs/export.c          |  3 ++-
>  fs/nfsd/nfs4state.c      | 20 ++++++++++++++++++++
>  include/linux/exportfs.h | 14 ++++++++++++++
>  3 files changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/nfs/export.c b/fs/nfs/export.c
> index be686b8e0c54..2f001a0273bc 100644
> --- a/fs/nfs/export.c
> +++ b/fs/nfs/export.c
> @@ -154,5 +154,6 @@ const struct export_operations nfs_export_ops = {
>  		 EXPORT_OP_CLOSE_BEFORE_UNLINK	|
>  		 EXPORT_OP_REMOTE_FS		|
>  		 EXPORT_OP_NOATOMIC_ATTR	|
> -		 EXPORT_OP_FLUSH_ON_CLOSE,
> +		 EXPORT_OP_FLUSH_ON_CLOSE	|
> +		 EXPORT_OP_NOLOCKSUPPORT,
>  };
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index ac1859c7cc9d..63297ea82e4e 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -5813,6 +5813,15 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  	if (!nf)
>  		return ERR_PTR(-EAGAIN);
>  
> +	/*
> +	 * File delegations and associated locks cannot be recovered if
> +	 * export is from NFS proxy server.
> +	 */
> +	if (exportfs_lock_op_is_unsupported(nf->nf_file->f_path.mnt->mnt_sb->s_export_op)) {
> +		nfsd_file_put(nf);
> +		return ERR_PTR(-EOPNOTSUPP);
> +	}
> +
>  	spin_lock(&state_lock);
>  	spin_lock(&fp->fi_lock);
>  	if (nfs4_delegation_exists(clp, fp))
> @@ -7917,6 +7926,11 @@ nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	}
>  	sb = cstate->current_fh.fh_dentry->d_sb;
>  
> +	if (exportfs_lock_op_is_unsupported(sb->s_export_op)) {
> +		status = nfserr_notsupp;
> +		goto out;
> +	}
> +
>  	if (lock->lk_is_new) {
>  		if (nfsd4_has_session(cstate))
>  			/* See rfc 5661 18.10.3: given clientid is ignored: */
> @@ -8266,6 +8280,12 @@ nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  		status = nfserr_lock_range;
>  		goto put_stateid;
>  	}
> +
> +	if (exportfs_lock_op_is_unsupported(nf->nf_file->f_path.mnt->mnt_sb->s_export_op)) {
> +		status = nfserr_notsupp;
> +		goto put_file;
> +	}
> +
>  	file_lock = locks_alloc_lock();
>  	if (!file_lock) {
>  		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
> index 893a1d21dc1c..106fd590d323 100644
> --- a/include/linux/exportfs.h
> +++ b/include/linux/exportfs.h
> @@ -247,6 +247,7 @@ struct export_operations {
>  						*/
>  #define EXPORT_OP_FLUSH_ON_CLOSE	(0x20) /* fs flushes file data on close */
>  #define EXPORT_OP_ASYNC_LOCK		(0x40) /* fs can do async lock request */
> +#define EXPORT_OP_NOLOCKSUPPORT		(0x80) /* no file locking support */
>  	unsigned long	flags;
>  };
>  
> @@ -263,6 +264,19 @@ exportfs_lock_op_is_async(const struct export_operations *export_ops)
>  	return export_ops->flags & EXPORT_OP_ASYNC_LOCK;
>  }
>  
> +/**
> + * exportfs_lock_op_is_unsupported() - export does not support file locking
> + * @export_ops:	the nfs export operations to check
> + *
> + * Returns true if the nfs export_operations structure has
> + * EXPORT_OP_NOLOCKSUPPORT in their flags set
> + */
> +static inline bool
> +exportfs_lock_op_is_unsupported(const struct export_operations *export_ops)
> +{
> +	return export_ops->flags & EXPORT_OP_NOLOCKSUPPORT;
> +}
> +
>  extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
>  				    int *max_len, struct inode *parent,
>  				    int flags);
> -- 
> 2.44.0
> 

Re: this patch:

I expect that this behavior will be surprising to users and
administrators. It would be great to have some documentation for NFS
re-export / proxy somewhere prominent that explains this (and other)
limitations for NFS proxy. Any thoughts, please share.

General comment:

We have talked about adding NFS re-export to our kdevops CI for
upstream NFSD, but that hasn't happened yet. I'm wondering if there
is any other regular testing of re-export.
diff mbox series

Patch

diff --git a/fs/nfs/export.c b/fs/nfs/export.c
index be686b8e0c54..2f001a0273bc 100644
--- a/fs/nfs/export.c
+++ b/fs/nfs/export.c
@@ -154,5 +154,6 @@  const struct export_operations nfs_export_ops = {
 		 EXPORT_OP_CLOSE_BEFORE_UNLINK	|
 		 EXPORT_OP_REMOTE_FS		|
 		 EXPORT_OP_NOATOMIC_ATTR	|
-		 EXPORT_OP_FLUSH_ON_CLOSE,
+		 EXPORT_OP_FLUSH_ON_CLOSE	|
+		 EXPORT_OP_NOLOCKSUPPORT,
 };
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ac1859c7cc9d..63297ea82e4e 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5813,6 +5813,15 @@  nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	if (!nf)
 		return ERR_PTR(-EAGAIN);
 
+	/*
+	 * File delegations and associated locks cannot be recovered if
+	 * export is from NFS proxy server.
+	 */
+	if (exportfs_lock_op_is_unsupported(nf->nf_file->f_path.mnt->mnt_sb->s_export_op)) {
+		nfsd_file_put(nf);
+		return ERR_PTR(-EOPNOTSUPP);
+	}
+
 	spin_lock(&state_lock);
 	spin_lock(&fp->fi_lock);
 	if (nfs4_delegation_exists(clp, fp))
@@ -7917,6 +7926,11 @@  nfsd4_lock(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	}
 	sb = cstate->current_fh.fh_dentry->d_sb;
 
+	if (exportfs_lock_op_is_unsupported(sb->s_export_op)) {
+		status = nfserr_notsupp;
+		goto out;
+	}
+
 	if (lock->lk_is_new) {
 		if (nfsd4_has_session(cstate))
 			/* See rfc 5661 18.10.3: given clientid is ignored: */
@@ -8266,6 +8280,12 @@  nfsd4_locku(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 		status = nfserr_lock_range;
 		goto put_stateid;
 	}
+
+	if (exportfs_lock_op_is_unsupported(nf->nf_file->f_path.mnt->mnt_sb->s_export_op)) {
+		status = nfserr_notsupp;
+		goto put_file;
+	}
+
 	file_lock = locks_alloc_lock();
 	if (!file_lock) {
 		dprintk("NFSD: %s: unable to allocate lock!\n", __func__);
diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
index 893a1d21dc1c..106fd590d323 100644
--- a/include/linux/exportfs.h
+++ b/include/linux/exportfs.h
@@ -247,6 +247,7 @@  struct export_operations {
 						*/
 #define EXPORT_OP_FLUSH_ON_CLOSE	(0x20) /* fs flushes file data on close */
 #define EXPORT_OP_ASYNC_LOCK		(0x40) /* fs can do async lock request */
+#define EXPORT_OP_NOLOCKSUPPORT		(0x80) /* no file locking support */
 	unsigned long	flags;
 };
 
@@ -263,6 +264,19 @@  exportfs_lock_op_is_async(const struct export_operations *export_ops)
 	return export_ops->flags & EXPORT_OP_ASYNC_LOCK;
 }
 
+/**
+ * exportfs_lock_op_is_unsupported() - export does not support file locking
+ * @export_ops:	the nfs export operations to check
+ *
+ * Returns true if the nfs export_operations structure has
+ * EXPORT_OP_NOLOCKSUPPORT in their flags set
+ */
+static inline bool
+exportfs_lock_op_is_unsupported(const struct export_operations *export_ops)
+{
+	return export_ops->flags & EXPORT_OP_NOLOCKSUPPORT;
+}
+
 extern int exportfs_encode_inode_fh(struct inode *inode, struct fid *fid,
 				    int *max_len, struct inode *parent,
 				    int flags);