[PATCH V2 0/2] NFSD: offer write delegation for OPEN with OPEN4_SHARE_ACCESS only

[PATCH V2 0/2] NFSD: offer write delegation for OPEN with OPEN4_SHARE_ACCESS only

[Dai Ngo]

From RFC8881 does not explicitly state that server must grant write
delegation to OPEN with OPEN4_SHARE_ACCESS_WRITE only. However there
are text in the RFC that implies it is up to the server implementation
to offer write delegation for OPEN with OPEN4_SHARE_ACCESS_WRITE only.

Section 9.1.2:

  "In the case of READ, the server may perform the corresponding
   check on the access mode, or it may choose to allow READ for
   OPEN4_SHARE_ACCESS_WRITE, to accommodate clients whose WRITE
   implementation may unavoidably do (e.g., due to buffer cache
   constraints)."

Also in section 10.4.1

  "Similarly, when closing a file opened for OPEN4_SHARE_ACCESS_WRITE/
   OPEN4_SHARE_ACCESS_BOTH and if an OPEN_DELEGATE_WRITE delegation
   is in effect"

This patch series offers write delegation for OPEN with OPEN4_SHARE_ACCESS_WRITE
only. The file struct, nfs4_file and nfs4_ol_stateid are upgraded
accordingly from write only access to read/write access. When the
delegation is returned, the file struct, nfs4_file and nfs4_ol_stateid
are downgraded according to remove the read access.

-- changes from v1:
0002: The file access mode is upgraded to include read access at the time
      the delegation is granted and read access is removed when the delegation
      is returned.

 fs/nfsd/nfs4state.c | 96 +++++++++++++++++++++++++++++++++++++-----------
 fs/nfsd/state.h     |  2 +
 2 files changed, 77 insertions(+), 21 deletions(-)

[PATCH V2 1/2] NFSD: Offer write delegation for OPEN with OPEN4_SHARE_ACCESS_WRITE

[Dai Ngo]

RFC8881, section 9.1.2 says:

  "In the case of READ, the server may perform the corresponding
   check on the access mode, or it may choose to allow READ for
   OPEN4_SHARE_ACCESS_WRITE, to accommodate clients whose WRITE
   implementation may unavoidably do (e.g., due to buffer cache
   constraints)."

and in section 10.4.1:
   "Similarly, when closing a file opened for OPEN4_SHARE_ACCESS_WRITE/
   OPEN4_SHARE_ACCESS_BOTH and if an OPEN_DELEGATE_WRITE delegation
   is in effect"

This patch offers write delegation for OPEN with OPEN4_SHARE_ACCESS_WRITE
only. Also deleted no longer use find_rw_file().

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfsd/nfs4state.c | 34 +++++++++++++---------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0f97f2c62b3a..b533225e57cf 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -633,18 +633,6 @@ find_readable_file(struct nfs4_file *f)
 	return ret;
 }
 
-static struct nfsd_file *
-find_rw_file(struct nfs4_file *f)
-{
-	struct nfsd_file *ret;
-
-	spin_lock(&f->fi_lock);
-	ret = nfsd_file_get(f->fi_fds[O_RDWR]);
-	spin_unlock(&f->fi_lock);
-
-	return ret;
-}
-
 struct nfsd_file *
 find_any_file(struct nfs4_file *f)
 {
@@ -5382,7 +5370,6 @@ static int nfsd4_cb_recall_done(struct nfsd4_callback *cb,
 	if (dp->dl_stid.sc_status)
 		/* CLOSED or REVOKED */
 		return 1;
-
 	switch (task->tk_status) {
 	case 0:
 		return 1;
@@ -5987,14 +5974,19 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 	 *  "An OPEN_DELEGATE_WRITE delegation allows the client to handle,
 	 *   on its own, all opens."
 	 *
-	 * Furthermore the client can use a write delegation for most READ
-	 * operations as well, so we require a O_RDWR file here.
+	 * Furthermore, section 9.1.2 says:
+	 *
+	 *  "In the case of READ, the server may perform the corresponding
+	 *  check on the access mode, or it may choose to allow READ for
+	 *  OPEN4_SHARE_ACCESS_WRITE, to accommodate clients whose WRITE
+	 *  implementation may unavoidably do (e.g., due to buffer cache
+	 *  constraints)."
 	 *
-	 * Offer a write delegation in the case of a BOTH open, and ensure
-	 * we get the O_RDWR descriptor.
+	 *  We choose to offer a write delegation for OPEN with the
+	 *  OPEN4_SHARE_ACCESS_WRITE access mode to accommodate such clients.
 	 */
-	if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_BOTH) {
-		nf = find_rw_file(fp);
+	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
+		nf = find_writeable_file(fp);
 		dl_type = deleg_ts ? OPEN_DELEGATE_WRITE_ATTRS_DELEG : OPEN_DELEGATE_WRITE;
 	}
 
@@ -6116,7 +6108,7 @@ static bool
 nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
 		     struct kstat *stat)
 {
-	struct nfsd_file *nf = find_rw_file(dp->dl_stid.sc_file);
+	struct nfsd_file *nf = find_writeable_file(dp->dl_stid.sc_file);
 	struct path path;
 	int rc;
 
@@ -7063,7 +7055,7 @@ nfsd4_lookup_stateid(struct nfsd4_compound_state *cstate,
 		return_revoked = true;
 	if (typemask & SC_TYPE_DELEG)
 		/* Always allow REVOKED for DELEG so we can
-		 * retturn the appropriate error.
+		 * return the appropriate error.
 		 */
 		statusmask |= SC_STATUS_REVOKED;
 
-- 
2.43.5

[PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Dai Ngo]

Allow READ using write delegation stateid granted on OPENs with
OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
implementation may unavoidably do (e.g., due to buffer cache
constraints).

When the server offers a write delegation for an OPEN with
OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
and nfs4_ol_stateid are upgraded as if the OPEN was sent with
OPEN4_SHARE_ACCESS_BOTH.

When this delegation is returned or revoked, the corresponding open
stateid is looked up and if it's found then the file access mode,
the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
access.

Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
---
 fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
 fs/nfsd/state.h     |  2 ++
 2 files changed, 64 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index b533225e57cf..0c14f902c54c 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
 	return rc == 0;
 }
 
+/*
+ * Upgrade file access mode to include FMODE_READ. This is called only when
+ * a write delegation is granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE.
+ */
+static void
+nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
+{
+	struct nfs4_file *fp = stp->st_stid.sc_file;
+	struct nfsd_file *nflp;
+	struct file *file;
+
+	spin_lock(&fp->fi_lock);
+	nflp = fp->fi_fds[O_WRONLY];
+	file = nflp->nf_file;
+	file->f_mode |= FMODE_READ;
+	swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
+	clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
+	set_access(NFS4_SHARE_ACCESS_BOTH, stp);
+	__nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);	/* incr fi_access[O_RDONLY] */
+	spin_unlock(&fp->fi_lock);
+}
+
+/*
+ * Downgrade file access mode to remove FMODE_READ. This is called when
+ * a write delegation, granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE,
+ * is returned.
+ */
+static void
+nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
+{
+	struct nfs4_file *fp = stp->st_stid.sc_file;
+	struct nfsd_file *nflp;
+	struct file *file;
+
+	spin_lock(&fp->fi_lock);
+	nflp = fp->fi_fds[O_RDWR];
+	file = nflp->nf_file;
+	file->f_mode &= ~FMODE_READ;
+	swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
+	clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
+	set_access(NFS4_SHARE_ACCESS_WRITE, stp);
+	spin_unlock(&fp->fi_lock);
+	nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);	/* decr. fi_access[O_RDONLY] */
+}
+
 /*
  * The Linux NFS server does not offer write delegations to NFSv4.0
  * clients in order to avoid conflicts between write delegations and
@@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
 		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
 		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
+
+		if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_WRITE) {
+			dp->dl_stateid = stp->st_stid.sc_stateid;
+			nfs4_upgrade_rdwr_file_access(stp);
+		}
 	} else {
 		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
 						    OPEN_DELEGATE_READ;
@@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 	struct nfs4_stid *s;
 	__be32 status;
 	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
+	struct nfs4_ol_stateid *stp;
+	struct nfs4_stid *stid;
 
 	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
 		return status;
@@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
 
 	trace_nfsd_deleg_return(stateid);
 	destroy_delegation(dp);
+
+	if (dp->dl_stateid.si_generation && dp->dl_stateid.si_opaque.so_id) {
+		if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
+				SC_TYPE_OPEN, 0, &stid, nn)) {
+			stp = openlockstateid(stid);
+			nfs4_downgrade_wronly_file_access(stp);
+			nfs4_put_stid(stid);
+		}
+	}
+
 	smp_mb__after_atomic();
 	wake_up_var(d_inode(cstate->current_fh.fh_dentry));
 put_stateid:
diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 74d2d7b42676..3f2f1b92db66 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -207,6 +207,8 @@ struct nfs4_delegation {
 
 	/* for CB_GETATTR */
 	struct nfs4_cb_fattr    dl_cb_fattr;
+
+	stateid_t		dl_stateid;  /* open stateid */
 };
 
 static inline bool deleg_is_read(u32 dl_type)
-- 
2.43.5

Re: [PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Chuck Lever]

On 2/21/25 6:42 PM, Dai Ngo wrote:
> Allow READ using write delegation stateid granted on OPENs with
> OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
> implementation may unavoidably do (e.g., due to buffer cache
> constraints).
> 
> When the server offers a write delegation for an OPEN with
> OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
> and nfs4_ol_stateid are upgraded as if the OPEN was sent with
> OPEN4_SHARE_ACCESS_BOTH.
> 
> When this delegation is returned or revoked, the corresponding open
> stateid is looked up and if it's found then the file access mode,
> the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
> access.

I probably don't understand something. The patch description seems to
suggest that a WR_ONLY OPEN state ID is also granted read in this case?


> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/state.h     |  2 ++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b533225e57cf..0c14f902c54c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
>  	return rc == 0;
>  }
>  
> +/*
> + * Upgrade file access mode to include FMODE_READ. This is called only when
> + * a write delegation is granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE.
> + */
> +static void
> +nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
> +{
> +	struct nfs4_file *fp = stp->st_stid.sc_file;
> +	struct nfsd_file *nflp;
> +	struct file *file;
> +
> +	spin_lock(&fp->fi_lock);
> +	nflp = fp->fi_fds[O_WRONLY];
> +	file = nflp->nf_file;
> +	file->f_mode |= FMODE_READ;
> +	swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
> +	clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
> +	set_access(NFS4_SHARE_ACCESS_BOTH, stp);
> +	__nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);	/* incr fi_access[O_RDONLY] */
> +	spin_unlock(&fp->fi_lock);
> +}
> +
> +/*
> + * Downgrade file access mode to remove FMODE_READ. This is called when
> + * a write delegation, granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE,
> + * is returned.
> + */
> +static void
> +nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
> +{
> +	struct nfs4_file *fp = stp->st_stid.sc_file;
> +	struct nfsd_file *nflp;
> +	struct file *file;
> +
> +	spin_lock(&fp->fi_lock);
> +	nflp = fp->fi_fds[O_RDWR];
> +	file = nflp->nf_file;
> +	file->f_mode &= ~FMODE_READ;
> +	swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
> +	clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
> +	set_access(NFS4_SHARE_ACCESS_WRITE, stp);
> +	spin_unlock(&fp->fi_lock);
> +	nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);	/* decr. fi_access[O_RDONLY] */
> +}
> +
>  /*
>   * The Linux NFS server does not offer write delegations to NFSv4.0
>   * clients in order to avoid conflicts between write delegations and
> @@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>  		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
>  		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> +
> +		if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_WRITE) {
> +			dp->dl_stateid = stp->st_stid.sc_stateid;
> +			nfs4_upgrade_rdwr_file_access(stp);
> +		}
>  	} else {
>  		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
>  						    OPEN_DELEGATE_READ;
> @@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	struct nfs4_stid *s;
>  	__be32 status;
>  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> +	struct nfs4_ol_stateid *stp;
> +	struct nfs4_stid *stid;
>  
>  	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
>  		return status;
> @@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	trace_nfsd_deleg_return(stateid);
>  	destroy_delegation(dp);
> +
> +	if (dp->dl_stateid.si_generation && dp->dl_stateid.si_opaque.so_id) {
> +		if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
> +				SC_TYPE_OPEN, 0, &stid, nn)) {
> +			stp = openlockstateid(stid);
> +			nfs4_downgrade_wronly_file_access(stp);
> +			nfs4_put_stid(stid);
> +		}
> +	}
> +
>  	smp_mb__after_atomic();
>  	wake_up_var(d_inode(cstate->current_fh.fh_dentry));
>  put_stateid:
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 74d2d7b42676..3f2f1b92db66 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -207,6 +207,8 @@ struct nfs4_delegation {
>  
>  	/* for CB_GETATTR */
>  	struct nfs4_cb_fattr    dl_cb_fattr;
> +
> +	stateid_t		dl_stateid;  /* open stateid */
>  };
>  
>  static inline bool deleg_is_read(u32 dl_type)


-- 
Chuck Lever

Re: [PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Dai Ngo]

On 2/24/25 6:48 AM, Chuck Lever wrote:
> On 2/21/25 6:42 PM, Dai Ngo wrote:
>> Allow READ using write delegation stateid granted on OPENs with
>> OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
>> implementation may unavoidably do (e.g., due to buffer cache
>> constraints).
>>
>> When the server offers a write delegation for an OPEN with
>> OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
>> and nfs4_ol_stateid are upgraded as if the OPEN was sent with
>> OPEN4_SHARE_ACCESS_BOTH.
>>
>> When this delegation is returned or revoked, the corresponding open
>> stateid is looked up and if it's found then the file access mode,
>> the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
>> access.
> I probably don't understand something. The patch description seems to
> suggest that a WR_ONLY OPEN state ID is also granted read in this case?

Currently nfsd allows a WR_ONLY OPEN state ID to do READ. The access check
is done in access_permit_read:

static inline int
access_permit_read(struct nfs4_ol_stateid *stp)
{
         return test_access(NFS4_SHARE_ACCESS_READ, stp) ||
                 test_access(NFS4_SHARE_ACCESS_BOTH, stp) ||
                 test_access(NFS4_SHARE_ACCESS_WRITE, stp);       <<====
}

Is this behavior intentional or is it a bug?

-Dai

>
>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>>   fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
>>   fs/nfsd/state.h     |  2 ++
>>   2 files changed, 64 insertions(+)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index b533225e57cf..0c14f902c54c 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
>>   	return rc == 0;
>>   }
>>   
>> +/*
>> + * Upgrade file access mode to include FMODE_READ. This is called only when
>> + * a write delegation is granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE.
>> + */
>> +static void
>> +nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
>> +{
>> +	struct nfs4_file *fp = stp->st_stid.sc_file;
>> +	struct nfsd_file *nflp;
>> +	struct file *file;
>> +
>> +	spin_lock(&fp->fi_lock);
>> +	nflp = fp->fi_fds[O_WRONLY];
>> +	file = nflp->nf_file;
>> +	file->f_mode |= FMODE_READ;
>> +	swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
>> +	clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
>> +	set_access(NFS4_SHARE_ACCESS_BOTH, stp);
>> +	__nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);	/* incr fi_access[O_RDONLY] */
>> +	spin_unlock(&fp->fi_lock);
>> +}
>> +
>> +/*
>> + * Downgrade file access mode to remove FMODE_READ. This is called when
>> + * a write delegation, granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE,
>> + * is returned.
>> + */
>> +static void
>> +nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
>> +{
>> +	struct nfs4_file *fp = stp->st_stid.sc_file;
>> +	struct nfsd_file *nflp;
>> +	struct file *file;
>> +
>> +	spin_lock(&fp->fi_lock);
>> +	nflp = fp->fi_fds[O_RDWR];
>> +	file = nflp->nf_file;
>> +	file->f_mode &= ~FMODE_READ;
>> +	swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
>> +	clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
>> +	set_access(NFS4_SHARE_ACCESS_WRITE, stp);
>> +	spin_unlock(&fp->fi_lock);
>> +	nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);	/* decr. fi_access[O_RDONLY] */
>> +}
>> +
>>   /*
>>    * The Linux NFS server does not offer write delegations to NFSv4.0
>>    * clients in order to avoid conflicts between write delegations and
>> @@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>>   		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>>   		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
>>   		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>> +
>> +		if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_WRITE) {
>> +			dp->dl_stateid = stp->st_stid.sc_stateid;
>> +			nfs4_upgrade_rdwr_file_access(stp);
>> +		}
>>   	} else {
>>   		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
>>   						    OPEN_DELEGATE_READ;
>> @@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>   	struct nfs4_stid *s;
>>   	__be32 status;
>>   	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>> +	struct nfs4_ol_stateid *stp;
>> +	struct nfs4_stid *stid;
>>   
>>   	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
>>   		return status;
>> @@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>   
>>   	trace_nfsd_deleg_return(stateid);
>>   	destroy_delegation(dp);
>> +
>> +	if (dp->dl_stateid.si_generation && dp->dl_stateid.si_opaque.so_id) {
>> +		if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
>> +				SC_TYPE_OPEN, 0, &stid, nn)) {
>> +			stp = openlockstateid(stid);
>> +			nfs4_downgrade_wronly_file_access(stp);
>> +			nfs4_put_stid(stid);
>> +		}
>> +	}
>> +
>>   	smp_mb__after_atomic();
>>   	wake_up_var(d_inode(cstate->current_fh.fh_dentry));
>>   put_stateid:
>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>> index 74d2d7b42676..3f2f1b92db66 100644
>> --- a/fs/nfsd/state.h
>> +++ b/fs/nfsd/state.h
>> @@ -207,6 +207,8 @@ struct nfs4_delegation {
>>   
>>   	/* for CB_GETATTR */
>>   	struct nfs4_cb_fattr    dl_cb_fattr;
>> +
>> +	stateid_t		dl_stateid;  /* open stateid */
>>   };
>>   
>>   static inline bool deleg_is_read(u32 dl_type)
>

Re: [PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Chuck Lever]

On 2/24/25 4:11 PM, Dai Ngo wrote:
> 
> On 2/24/25 6:48 AM, Chuck Lever wrote:
>> On 2/21/25 6:42 PM, Dai Ngo wrote:
>>> Allow READ using write delegation stateid granted on OPENs with
>>> OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
>>> implementation may unavoidably do (e.g., due to buffer cache
>>> constraints).
>>>
>>> When the server offers a write delegation for an OPEN with
>>> OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
>>> and nfs4_ol_stateid are upgraded as if the OPEN was sent with
>>> OPEN4_SHARE_ACCESS_BOTH.
>>>
>>> When this delegation is returned or revoked, the corresponding open
>>> stateid is looked up and if it's found then the file access mode,
>>> the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
>>> access.
>> I probably don't understand something. The patch description seems to
>> suggest that a WR_ONLY OPEN state ID is also granted read in this case?
> 
> Currently nfsd allows a WR_ONLY OPEN state ID to do READ. The access check
> is done in access_permit_read:
> 
> static inline int
> access_permit_read(struct nfs4_ol_stateid *stp)
> {
>         return test_access(NFS4_SHARE_ACCESS_READ, stp) ||
>                 test_access(NFS4_SHARE_ACCESS_BOTH, stp) ||
>                 test_access(NFS4_SHARE_ACCESS_WRITE, stp);       <<====
> }
> 
> Is this behavior intentional or is it a bug?

RFC 8881 Section 9.1.2 makes an exception for this case, so not a bug.

One assumes this is to permit clients to perform RMW, but a comment
above this helper would have alleviated some confusion amongst us
software historians.


> -Dai
> 
>>
>>
>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>>> ---
>>>   fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
>>>   fs/nfsd/state.h     |  2 ++
>>>   2 files changed, 64 insertions(+)
>>>
>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>> index b533225e57cf..0c14f902c54c 100644
>>> --- a/fs/nfsd/nfs4state.c
>>> +++ b/fs/nfsd/nfs4state.c
>>> @@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation
>>> *dp, struct svc_fh *currentfh,
>>>       return rc == 0;
>>>   }
>>>   +/*
>>> + * Upgrade file access mode to include FMODE_READ. This is called
>>> only when
>>> + * a write delegation is granted for an OPEN with
>>> OPEN4_SHARE_ACCESS_WRITE.
>>> + */
>>> +static void
>>> +nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
>>> +{
>>> +    struct nfs4_file *fp = stp->st_stid.sc_file;
>>> +    struct nfsd_file *nflp;
>>> +    struct file *file;
>>> +
>>> +    spin_lock(&fp->fi_lock);
>>> +    nflp = fp->fi_fds[O_WRONLY];
>>> +    file = nflp->nf_file;
>>> +    file->f_mode |= FMODE_READ;
>>> +    swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
>>> +    clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
>>> +    set_access(NFS4_SHARE_ACCESS_BOTH, stp);
>>> +    __nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);    /* incr
>>> fi_access[O_RDONLY] */
>>> +    spin_unlock(&fp->fi_lock);
>>> +}
>>> +
>>> +/*
>>> + * Downgrade file access mode to remove FMODE_READ. This is called when
>>> + * a write delegation, granted for an OPEN with
>>> OPEN4_SHARE_ACCESS_WRITE,
>>> + * is returned.
>>> + */
>>> +static void
>>> +nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
>>> +{
>>> +    struct nfs4_file *fp = stp->st_stid.sc_file;
>>> +    struct nfsd_file *nflp;
>>> +    struct file *file;
>>> +
>>> +    spin_lock(&fp->fi_lock);
>>> +    nflp = fp->fi_fds[O_RDWR];
>>> +    file = nflp->nf_file;
>>> +    file->f_mode &= ~FMODE_READ;
>>> +    swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
>>> +    clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
>>> +    set_access(NFS4_SHARE_ACCESS_WRITE, stp);
>>> +    spin_unlock(&fp->fi_lock);
>>> +    nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);    /* decr.
>>> fi_access[O_RDONLY] */
>>> +}
>>> +
>>>   /*
>>>    * The Linux NFS server does not offer write delegations to NFSv4.0
>>>    * clients in order to avoid conflicts between write delegations and
>>> @@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open,
>>> struct nfs4_ol_stateid *stp,
>>>           dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>>>           dp->dl_cb_fattr.ncf_initial_cinfo =
>>> nfsd4_change_attribute(&stat);
>>>           trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>>> +
>>> +        if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) ==
>>> NFS4_SHARE_ACCESS_WRITE) {
>>> +            dp->dl_stateid = stp->st_stid.sc_stateid;
>>> +            nfs4_upgrade_rdwr_file_access(stp);
>>> +        }
>>>       } else {
>>>           open->op_delegate_type = deleg_ts ?
>>> OPEN_DELEGATE_READ_ATTRS_DELEG :
>>>                               OPEN_DELEGATE_READ;
>>> @@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp,
>>> struct nfsd4_compound_state *cstate,
>>>       struct nfs4_stid *s;
>>>       __be32 status;
>>>       struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>>> +    struct nfs4_ol_stateid *stp;
>>> +    struct nfs4_stid *stid;
>>>         if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG,
>>> 0)))
>>>           return status;
>>> @@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp,
>>> struct nfsd4_compound_state *cstate,
>>>         trace_nfsd_deleg_return(stateid);
>>>       destroy_delegation(dp);
>>> +
>>> +    if (dp->dl_stateid.si_generation && dp-
>>> >dl_stateid.si_opaque.so_id) {
>>> +        if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
>>> +                SC_TYPE_OPEN, 0, &stid, nn)) {
>>> +            stp = openlockstateid(stid);
>>> +            nfs4_downgrade_wronly_file_access(stp);
>>> +            nfs4_put_stid(stid);
>>> +        }
>>> +    }
>>> +
>>>       smp_mb__after_atomic();
>>>       wake_up_var(d_inode(cstate->current_fh.fh_dentry));
>>>   put_stateid:
>>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>>> index 74d2d7b42676..3f2f1b92db66 100644
>>> --- a/fs/nfsd/state.h
>>> +++ b/fs/nfsd/state.h
>>> @@ -207,6 +207,8 @@ struct nfs4_delegation {
>>>         /* for CB_GETATTR */
>>>       struct nfs4_cb_fattr    dl_cb_fattr;
>>> +
>>> +    stateid_t        dl_stateid;  /* open stateid */
>>>   };
>>>     static inline bool deleg_is_read(u32 dl_type)
>>


-- 
Chuck Lever

Re: [PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Jeff Layton]

On Fri, 2025-02-21 at 15:42 -0800, Dai Ngo wrote:
> Allow READ using write delegation stateid granted on OPENs with
> OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
> implementation may unavoidably do (e.g., due to buffer cache
> constraints).
> 
> When the server offers a write delegation for an OPEN with
> OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
> and nfs4_ol_stateid are upgraded as if the OPEN was sent with
> OPEN4_SHARE_ACCESS_BOTH.
> 
> When this delegation is returned or revoked, the corresponding open
> stateid is looked up and if it's found then the file access mode,
> the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
> access.
> 
> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> ---
>  fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
>  fs/nfsd/state.h     |  2 ++
>  2 files changed, 64 insertions(+)
> 
> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> index b533225e57cf..0c14f902c54c 100644
> --- a/fs/nfsd/nfs4state.c
> +++ b/fs/nfsd/nfs4state.c
> @@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
>  	return rc == 0;
>  }
>  
> +/*
> + * Upgrade file access mode to include FMODE_READ. This is called only when
> + * a write delegation is granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE.
> + */
> +static void
> +nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
> +{
> +	struct nfs4_file *fp = stp->st_stid.sc_file;
> +	struct nfsd_file *nflp;
> +	struct file *file;
> +
> +	spin_lock(&fp->fi_lock);
> +	nflp = fp->fi_fds[O_WRONLY];
> +	file = nflp->nf_file;
> +	file->f_mode |= FMODE_READ;

You can't just do this. Open upgrade/downgrade doesn't exist at the VFS
layer currently. It might work with most local filesystems, but more
complex filesystems have significant context attached to a file. Just
because you've changed it here, doesn't mean that you will _actually_
be able to do reads using it.

This might even be actively unsafe, as you're bypassing permissions
checks here. You never checked whether the file is readable. What if
it's write only? Same clients will do an ACCESS check before allowing
it, but a hostile actor might be able to exploit this.

I think you need to acquire a R/W open from the get-go instead when you
intend to grant a delegation, and just fall back to doing a O_WRONLY
open if that fails (and don't grant one).

> +	swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
> +	clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
> +	set_access(NFS4_SHARE_ACCESS_BOTH, stp);
> +	__nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);	/* incr fi_access[O_RDONLY] */
> +	spin_unlock(&fp->fi_lock);
> +}
> +
> +/*
> + * Downgrade file access mode to remove FMODE_READ. This is called when
> + * a write delegation, granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE,
> + * is returned.
> + */
> +static void
> +nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
> +{
> +	struct nfs4_file *fp = stp->st_stid.sc_file;
> +	struct nfsd_file *nflp;
> +	struct file *file;
> +
> +	spin_lock(&fp->fi_lock);
> +	nflp = fp->fi_fds[O_RDWR];
> +	file = nflp->nf_file;
> +	file->f_mode &= ~FMODE_READ;
> +	swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
> +	clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
> +	set_access(NFS4_SHARE_ACCESS_WRITE, stp);
> +	spin_unlock(&fp->fi_lock);
> +	nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);	/* decr. fi_access[O_RDONLY] */
> +}
> +
>  /*
>   * The Linux NFS server does not offer write delegations to NFSv4.0
>   * clients in order to avoid conflicts between write delegations and
> @@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>  		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>  		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
>  		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> +
> +		if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_WRITE) {
> +			dp->dl_stateid = stp->st_stid.sc_stateid;
> +			nfs4_upgrade_rdwr_file_access(stp);
> +		}
>  	} else {
>  		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
>  						    OPEN_DELEGATE_READ;
> @@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  	struct nfs4_stid *s;
>  	__be32 status;
>  	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> +	struct nfs4_ol_stateid *stp;
> +	struct nfs4_stid *stid;
>  
>  	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
>  		return status;
> @@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>  
>  	trace_nfsd_deleg_return(stateid);
>  	destroy_delegation(dp);
> +
> +	if (dp->dl_stateid.si_generation && dp->dl_stateid.si_opaque.so_id) {
> +		if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
> +				SC_TYPE_OPEN, 0, &stid, nn)) {
> +			stp = openlockstateid(stid);
> +			nfs4_downgrade_wronly_file_access(stp);
> +			nfs4_put_stid(stid);
> +		}
> +	}
> +
>  	smp_mb__after_atomic();
>  	wake_up_var(d_inode(cstate->current_fh.fh_dentry));
>  put_stateid:
> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> index 74d2d7b42676..3f2f1b92db66 100644
> --- a/fs/nfsd/state.h
> +++ b/fs/nfsd/state.h
> @@ -207,6 +207,8 @@ struct nfs4_delegation {
>  
>  	/* for CB_GETATTR */
>  	struct nfs4_cb_fattr    dl_cb_fattr;
> +
> +	stateid_t		dl_stateid;  /* open stateid */
>  };
>  
>  static inline bool deleg_is_read(u32 dl_type)

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Dai Ngo]

On 2/24/25 7:48 AM, Jeff Layton wrote:
> On Fri, 2025-02-21 at 15:42 -0800, Dai Ngo wrote:
>> Allow READ using write delegation stateid granted on OPENs with
>> OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
>> implementation may unavoidably do (e.g., due to buffer cache
>> constraints).
>>
>> When the server offers a write delegation for an OPEN with
>> OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
>> and nfs4_ol_stateid are upgraded as if the OPEN was sent with
>> OPEN4_SHARE_ACCESS_BOTH.
>>
>> When this delegation is returned or revoked, the corresponding open
>> stateid is looked up and if it's found then the file access mode,
>> the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
>> access.
>>
>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>> ---
>>   fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
>>   fs/nfsd/state.h     |  2 ++
>>   2 files changed, 64 insertions(+)
>>
>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>> index b533225e57cf..0c14f902c54c 100644
>> --- a/fs/nfsd/nfs4state.c
>> +++ b/fs/nfsd/nfs4state.c
>> @@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
>>   	return rc == 0;
>>   }
>>   
>> +/*
>> + * Upgrade file access mode to include FMODE_READ. This is called only when
>> + * a write delegation is granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE.
>> + */
>> +static void
>> +nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
>> +{
>> +	struct nfs4_file *fp = stp->st_stid.sc_file;
>> +	struct nfsd_file *nflp;
>> +	struct file *file;
>> +
>> +	spin_lock(&fp->fi_lock);
>> +	nflp = fp->fi_fds[O_WRONLY];
>> +	file = nflp->nf_file;
>> +	file->f_mode |= FMODE_READ;
> You can't just do this. Open upgrade/downgrade doesn't exist at the VFS
> layer currently. It might work with most local filesystems, but more
> complex filesystems have significant context attached to a file. Just
> because you've changed it here, doesn't mean that you will _actually_
> be able to do reads using it.

I think allowing read using a write delegation from a OPEN with
WRONLY is an optional feature and is not a requirement from the
spec. So if there are filesystems that do not allow this feature
to work then it is ok; we did not introduce any new problems with
this feature.

>
> This might even be actively unsafe, as you're bypassing permissions
> checks here. You never checked whether the file is readable. What if
> it's write only? Same clients will do an ACCESS check before allowing
> it, but a hostile actor might be able to exploit this.

Apparently the NFS server relies on the NFS client to do permission
check at time the file is opened. Once the permission check passes and
the OPEN is successful, then there is no permission check on READ/WRITE.

I wrote this pynfs test to verify:

def testReadOnWrOnlyFile(t, env):
     """Test read using open stateid with OPEN4_SHARE_ACCESS_WRITE
        on file with permission 0222

     FLAGS: writedelegations deleg
     CODE: DELEG28
     """

     # create a file with write-only mode (0222)
     sess = env.c1.new_client_session(b"%s_2" % env.testname(t))
     filename = env.testname(t)
     res = open_create_file(sess, filename, open_create=OPEN4_CREATE,
             attrs={FATTR4_MODE: 0o222}, access=OPEN4_SHARE_ACCESS_WRITE, want_deleg=False)
     check(res)

     # write file content
     fh = res.resarray[-1].object
     stateid = res.resarray[-2].stateid
     data = b"write test data"
     res = write_file(sess, fh, data, 0, stateid)
     check(res)

     # close the file
     res = close_file(sess, fh, stateid)
     check(res)

     # OPEN file with OPEN4_SHARE_ACCESS_WRITE
     access = OPEN4_SHARE_ACCESS_WRITE|OPEN4_SHARE_ACCESS_WANT_NO_DELEG
     res = open_file(sess, filename, access = access, want_deleg = True)
     check(res)

     # READ file using open stateid
     # Linux NFS server allows READ using open stateid from OPEN4_SHARE_ACCESS_WRITE.
     # However, the file permission mode 0222 then the READ should fail!
     stateid = res.resarray[-2].stateid
     res = sess.compound([op.putfh(fh), op.read(stateid, 0, 10)])
     check(res, NFS4ERR_ACCESS)
     res = close_file(sess, fh, stateid)
     check(res)

and the test failed with:
      "OP_READ should return NFS4ERR_ACCESS, instead got NFS4_OK"

Am i missing something?

-Dai

>
> I think you need to acquire a R/W open from the get-go instead when you
> intend to grant a delegation, and just fall back to doing a O_WRONLY
> open if that fails (and don't grant one).
>
>> +	swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
>> +	clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
>> +	set_access(NFS4_SHARE_ACCESS_BOTH, stp);
>> +	__nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);	/* incr fi_access[O_RDONLY] */
>> +	spin_unlock(&fp->fi_lock);
>> +}
>> +
>> +/*
>> + * Downgrade file access mode to remove FMODE_READ. This is called when
>> + * a write delegation, granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE,
>> + * is returned.
>> + */
>> +static void
>> +nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
>> +{
>> +	struct nfs4_file *fp = stp->st_stid.sc_file;
>> +	struct nfsd_file *nflp;
>> +	struct file *file;
>> +
>> +	spin_lock(&fp->fi_lock);
>> +	nflp = fp->fi_fds[O_RDWR];
>> +	file = nflp->nf_file;
>> +	file->f_mode &= ~FMODE_READ;
>> +	swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
>> +	clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
>> +	set_access(NFS4_SHARE_ACCESS_WRITE, stp);
>> +	spin_unlock(&fp->fi_lock);
>> +	nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);	/* decr. fi_access[O_RDONLY] */
>> +}
>> +
>>   /*
>>    * The Linux NFS server does not offer write delegations to NFSv4.0
>>    * clients in order to avoid conflicts between write delegations and
>> @@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>>   		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>>   		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
>>   		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>> +
>> +		if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_WRITE) {
>> +			dp->dl_stateid = stp->st_stid.sc_stateid;
>> +			nfs4_upgrade_rdwr_file_access(stp);
>> +		}
>>   	} else {
>>   		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
>>   						    OPEN_DELEGATE_READ;
>> @@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>   	struct nfs4_stid *s;
>>   	__be32 status;
>>   	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>> +	struct nfs4_ol_stateid *stp;
>> +	struct nfs4_stid *stid;
>>   
>>   	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
>>   		return status;
>> @@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>   
>>   	trace_nfsd_deleg_return(stateid);
>>   	destroy_delegation(dp);
>> +
>> +	if (dp->dl_stateid.si_generation && dp->dl_stateid.si_opaque.so_id) {
>> +		if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
>> +				SC_TYPE_OPEN, 0, &stid, nn)) {
>> +			stp = openlockstateid(stid);
>> +			nfs4_downgrade_wronly_file_access(stp);
>> +			nfs4_put_stid(stid);
>> +		}
>> +	}
>> +
>>   	smp_mb__after_atomic();
>>   	wake_up_var(d_inode(cstate->current_fh.fh_dentry));
>>   put_stateid:
>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>> index 74d2d7b42676..3f2f1b92db66 100644
>> --- a/fs/nfsd/state.h
>> +++ b/fs/nfsd/state.h
>> @@ -207,6 +207,8 @@ struct nfs4_delegation {
>>   
>>   	/* for CB_GETATTR */
>>   	struct nfs4_cb_fattr    dl_cb_fattr;
>> +
>> +	stateid_t		dl_stateid;  /* open stateid */
>>   };
>>   
>>   static inline bool deleg_is_read(u32 dl_type)

Re: [PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Jeff Layton]

On Mon, 2025-02-24 at 17:10 -0800, Dai Ngo wrote:
> On 2/24/25 7:48 AM, Jeff Layton wrote:
> > On Fri, 2025-02-21 at 15:42 -0800, Dai Ngo wrote:
> > > Allow READ using write delegation stateid granted on OPENs with
> > > OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
> > > implementation may unavoidably do (e.g., due to buffer cache
> > > constraints).
> > > 
> > > When the server offers a write delegation for an OPEN with
> > > OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
> > > and nfs4_ol_stateid are upgraded as if the OPEN was sent with
> > > OPEN4_SHARE_ACCESS_BOTH.
> > > 
> > > When this delegation is returned or revoked, the corresponding open
> > > stateid is looked up and if it's found then the file access mode,
> > > the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
> > > access.
> > > 
> > > Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
> > > ---
> > >   fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
> > >   fs/nfsd/state.h     |  2 ++
> > >   2 files changed, 64 insertions(+)
> > > 
> > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
> > > index b533225e57cf..0c14f902c54c 100644
> > > --- a/fs/nfsd/nfs4state.c
> > > +++ b/fs/nfsd/nfs4state.c
> > > @@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
> > >   	return rc == 0;
> > >   }
> > >   
> > > +/*
> > > + * Upgrade file access mode to include FMODE_READ. This is called only when
> > > + * a write delegation is granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE.
> > > + */
> > > +static void
> > > +nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
> > > +{
> > > +	struct nfs4_file *fp = stp->st_stid.sc_file;
> > > +	struct nfsd_file *nflp;
> > > +	struct file *file;
> > > +
> > > +	spin_lock(&fp->fi_lock);
> > > +	nflp = fp->fi_fds[O_WRONLY];
> > > +	file = nflp->nf_file;
> > > +	file->f_mode |= FMODE_READ;
> > You can't just do this. Open upgrade/downgrade doesn't exist at the VFS
> > layer currently. It might work with most local filesystems, but more
> > complex filesystems have significant context attached to a file. Just
> > because you've changed it here, doesn't mean that you will _actually_
> > be able to do reads using it.
> 
> I think allowing read using a write delegation from a OPEN with
> WRONLY is an optional feature and is not a requirement from the
> spec. So if there are filesystems that do not allow this feature
> to work then it is ok; we did not introduce any new problems with
> this feature.
>

This patch is swapping the O_RDWR fd in the nfsd4_file with an O_WRONLY
one that has had FMODE_READ added. That will break on filesystems that
don't actually allow you to do reads on a filp that was opened
O_WRONLY.

> > 
> > This might even be actively unsafe, as you're bypassing permissions
> > checks here. You never checked whether the file is readable. What if
> > it's write only? Same clients will do an ACCESS check before allowing
> > it, but a hostile actor might be able to exploit this.
> 
> Apparently the NFS server relies on the NFS client to do permission
> check at time the file is opened. Once the permission check passes and
> the OPEN is successful, then there is no permission check on READ/WRITE.
> 
> I wrote this pynfs test to verify:
> 
> def testReadOnWrOnlyFile(t, env):
>      """Test read using open stateid with OPEN4_SHARE_ACCESS_WRITE
>         on file with permission 0222
> 
>      FLAGS: writedelegations deleg
>      CODE: DELEG28
>      """
> 
>      # create a file with write-only mode (0222)
>      sess = env.c1.new_client_session(b"%s_2" % env.testname(t))
>      filename = env.testname(t)
>      res = open_create_file(sess, filename, open_create=OPEN4_CREATE,
>              attrs={FATTR4_MODE: 0o222}, access=OPEN4_SHARE_ACCESS_WRITE, want_deleg=False)
>      check(res)
> 
>      # write file content
>      fh = res.resarray[-1].object
>      stateid = res.resarray[-2].stateid
>      data = b"write test data"
>      res = write_file(sess, fh, data, 0, stateid)
>      check(res)
> 
>      # close the file
>      res = close_file(sess, fh, stateid)
>      check(res)
> 
>      # OPEN file with OPEN4_SHARE_ACCESS_WRITE
>      access = OPEN4_SHARE_ACCESS_WRITE|OPEN4_SHARE_ACCESS_WANT_NO_DELEG
>      res = open_file(sess, filename, access = access, want_deleg = True)

nit: shouldn't want_deleg be False there?

>      check(res)
> 
>      # READ file using open stateid
>      # Linux NFS server allows READ using open stateid from OPEN4_SHARE_ACCESS_WRITE.
>      # However, the file permission mode 0222 then the READ should fail!
>      stateid = res.resarray[-2].stateid
>      res = sess.compound([op.putfh(fh), op.read(stateid, 0, 10)])
>      check(res, NFS4ERR_ACCESS)
>      res = close_file(sess, fh, stateid)
>      check(res)
> 
> and the test failed with:
>       "OP_READ should return NFS4ERR_ACCESS, instead got NFS4_OK"
> 
> Am i missing something?
> 

nfsd actually does check permissions on every READ operation via:

  nfs4_preprocess_stateid_op
     nfs4_check_file
        nfsd_permission

You may be bypassing it via the NFSD_MAY_OWNER_OVERRIDE flag. Does the
above test also fail if the file is owned by a different user than the
one running the test?

> 
> > 
> > I think you need to acquire a R/W open from the get-go instead when you
> > intend to grant a delegation, and just fall back to doing a O_WRONLY
> > open if that fails (and don't grant one).
> > 
> > > +	swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
> > > +	clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
> > > +	set_access(NFS4_SHARE_ACCESS_BOTH, stp);
> > > +	__nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);	/* incr fi_access[O_RDONLY] */
> > > +	spin_unlock(&fp->fi_lock);
> > > +}
> > > +
> > > +/*
> > > + * Downgrade file access mode to remove FMODE_READ. This is called when
> > > + * a write delegation, granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE,
> > > + * is returned.
> > > + */
> > > +static void
> > > +nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
> > > +{
> > > +	struct nfs4_file *fp = stp->st_stid.sc_file;
> > > +	struct nfsd_file *nflp;
> > > +	struct file *file;
> > > +
> > > +	spin_lock(&fp->fi_lock);
> > > +	nflp = fp->fi_fds[O_RDWR];
> > > +	file = nflp->nf_file;
> > > +	file->f_mode &= ~FMODE_READ;
> > > +	swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
> > > +	clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
> > > +	set_access(NFS4_SHARE_ACCESS_WRITE, stp);
> > > +	spin_unlock(&fp->fi_lock);
> > > +	nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);	/* decr. fi_access[O_RDONLY] */
> > > +}
> > > +
> > >   /*
> > >    * The Linux NFS server does not offer write delegations to NFSv4.0
> > >    * clients in order to avoid conflicts between write delegations and
> > > @@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
> > >   		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
> > >   		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
> > >   		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
> > > +
> > > +		if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_WRITE) {
> > > +			dp->dl_stateid = stp->st_stid.sc_stateid;
> > > +			nfs4_upgrade_rdwr_file_access(stp);
> > > +		}
> > >   	} else {
> > >   		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
> > >   						    OPEN_DELEGATE_READ;
> > > @@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > >   	struct nfs4_stid *s;
> > >   	__be32 status;
> > >   	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
> > > +	struct nfs4_ol_stateid *stp;
> > > +	struct nfs4_stid *stid;
> > >   
> > >   	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
> > >   		return status;
> > > @@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> > >   
> > >   	trace_nfsd_deleg_return(stateid);
> > >   	destroy_delegation(dp);
> > > +
> > > +	if (dp->dl_stateid.si_generation && dp->dl_stateid.si_opaque.so_id) {
> > > +		if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
> > > +				SC_TYPE_OPEN, 0, &stid, nn)) {
> > > +			stp = openlockstateid(stid);
> > > +			nfs4_downgrade_wronly_file_access(stp);
> > > +			nfs4_put_stid(stid);
> > > +		}
> > > +	}
> > > +
> > >   	smp_mb__after_atomic();
> > >   	wake_up_var(d_inode(cstate->current_fh.fh_dentry));
> > >   put_stateid:
> > > diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
> > > index 74d2d7b42676..3f2f1b92db66 100644
> > > --- a/fs/nfsd/state.h
> > > +++ b/fs/nfsd/state.h
> > > @@ -207,6 +207,8 @@ struct nfs4_delegation {
> > >   
> > >   	/* for CB_GETATTR */
> > >   	struct nfs4_cb_fattr    dl_cb_fattr;
> > > +
> > > +	stateid_t		dl_stateid;  /* open stateid */
> > >   };
> > >   
> > >   static inline bool deleg_is_read(u32 dl_type)

-- 
Jeff Layton <jlayton@kernel.org>

Re: [PATCH V2 2/2] NFSD: allow client to use write delegation stateid for READ

[Dai Ngo]

On 2/25/25 4:31 AM, Jeff Layton wrote:
> On Mon, 2025-02-24 at 17:10 -0800, Dai Ngo wrote:
>> On 2/24/25 7:48 AM, Jeff Layton wrote:
>>> On Fri, 2025-02-21 at 15:42 -0800, Dai Ngo wrote:
>>>> Allow READ using write delegation stateid granted on OPENs with
>>>> OPEN4_SHARE_ACCESS_WRITE only, to accommodate clients whose WRITE
>>>> implementation may unavoidably do (e.g., due to buffer cache
>>>> constraints).
>>>>
>>>> When the server offers a write delegation for an OPEN with
>>>> OPEN4_SHARE_ACCESS_WRITE, the file access mode, the nfs4_file
>>>> and nfs4_ol_stateid are upgraded as if the OPEN was sent with
>>>> OPEN4_SHARE_ACCESS_BOTH.
>>>>
>>>> When this delegation is returned or revoked, the corresponding open
>>>> stateid is looked up and if it's found then the file access mode,
>>>> the nfs4_file and nfs4_ol_stateid are downgraded to remove the read
>>>> access.
>>>>
>>>> Signed-off-by: Dai Ngo <dai.ngo@oracle.com>
>>>> ---
>>>>    fs/nfsd/nfs4state.c | 62 +++++++++++++++++++++++++++++++++++++++++++++
>>>>    fs/nfsd/state.h     |  2 ++
>>>>    2 files changed, 64 insertions(+)
>>>>
>>>> diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
>>>> index b533225e57cf..0c14f902c54c 100644
>>>> --- a/fs/nfsd/nfs4state.c
>>>> +++ b/fs/nfsd/nfs4state.c
>>>> @@ -6126,6 +6126,51 @@ nfs4_delegation_stat(struct nfs4_delegation *dp, struct svc_fh *currentfh,
>>>>    	return rc == 0;
>>>>    }
>>>>    
>>>> +/*
>>>> + * Upgrade file access mode to include FMODE_READ. This is called only when
>>>> + * a write delegation is granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE.
>>>> + */
>>>> +static void
>>>> +nfs4_upgrade_rdwr_file_access(struct nfs4_ol_stateid *stp)
>>>> +{
>>>> +	struct nfs4_file *fp = stp->st_stid.sc_file;
>>>> +	struct nfsd_file *nflp;
>>>> +	struct file *file;
>>>> +
>>>> +	spin_lock(&fp->fi_lock);
>>>> +	nflp = fp->fi_fds[O_WRONLY];
>>>> +	file = nflp->nf_file;
>>>> +	file->f_mode |= FMODE_READ;
>>> You can't just do this. Open upgrade/downgrade doesn't exist at the VFS
>>> layer currently. It might work with most local filesystems, but more
>>> complex filesystems have significant context attached to a file. Just
>>> because you've changed it here, doesn't mean that you will _actually_
>>> be able to do reads using it.
>> I think allowing read using a write delegation from a OPEN with
>> WRONLY is an optional feature and is not a requirement from the
>> spec. So if there are filesystems that do not allow this feature
>> to work then it is ok; we did not introduce any new problems with
>> this feature.
>>
> This patch is swapping the O_RDWR fd in the nfsd4_file with an O_WRONLY
> one that has had FMODE_READ added. That will break on filesystems that
> don't actually allow you to do reads on a filp that was opened
> O_WRONLY.

So the current patch might not work for all filesystems, it is not
a perfect solution but it doesn't introduce any bugs or undesired
behavior. I wonder how many filesystems out there fall in to this
category. So far I've only tested with xfs and ext4.

>
>>> This might even be actively unsafe, as you're bypassing permissions
>>> checks here. You never checked whether the file is readable. What if
>>> it's write only? Same clients will do an ACCESS check before allowing
>>> it, but a hostile actor might be able to exploit this.
>> Apparently the NFS server relies on the NFS client to do permission
>> check at time the file is opened. Once the permission check passes and
>> the OPEN is successful, then there is no permission check on READ/WRITE.
>>
>> I wrote this pynfs test to verify:
>>
>> def testReadOnWrOnlyFile(t, env):
>>       """Test read using open stateid with OPEN4_SHARE_ACCESS_WRITE
>>          on file with permission 0222
>>
>>       FLAGS: writedelegations deleg
>>       CODE: DELEG28
>>       """
>>
>>       # create a file with write-only mode (0222)
>>       sess = env.c1.new_client_session(b"%s_2" % env.testname(t))
>>       filename = env.testname(t)
>>       res = open_create_file(sess, filename, open_create=OPEN4_CREATE,
>>               attrs={FATTR4_MODE: 0o222}, access=OPEN4_SHARE_ACCESS_WRITE, want_deleg=False)
>>       check(res)
>>
>>       # write file content
>>       fh = res.resarray[-1].object
>>       stateid = res.resarray[-2].stateid
>>       data = b"write test data"
>>       res = write_file(sess, fh, data, 0, stateid)
>>       check(res)
>>
>>       # close the file
>>       res = close_file(sess, fh, stateid)
>>       check(res)
>>
>>       # OPEN file with OPEN4_SHARE_ACCESS_WRITE
>>       access = OPEN4_SHARE_ACCESS_WRITE|OPEN4_SHARE_ACCESS_WANT_NO_DELEG
>>       res = open_file(sess, filename, access = access, want_deleg = True)
> nit: shouldn't want_deleg be False there?

Thanks! Fixed, result is the same.

>
>>       check(res)
>>
>>       # READ file using open stateid
>>       # Linux NFS server allows READ using open stateid from OPEN4_SHARE_ACCESS_WRITE.
>>       # However, the file permission mode 0222 then the READ should fail!
>>       stateid = res.resarray[-2].stateid
>>       res = sess.compound([op.putfh(fh), op.read(stateid, 0, 10)])
>>       check(res, NFS4ERR_ACCESS)
>>       res = close_file(sess, fh, stateid)
>>       check(res)
>>
>> and the test failed with:
>>        "OP_READ should return NFS4ERR_ACCESS, instead got NFS4_OK"
>>
>> Am i missing something?
>>
> nfsd actually does check permissions on every READ operation via:
>
>    nfs4_preprocess_stateid_op
>       nfs4_check_file
>          nfsd_permission

Ah I missed this check.

>
> You may be bypassing it via the NFSD_MAY_OWNER_OVERRIDE flag.

Yes, the NFSD_MAY_OWNER_OVERRIDE flag bypasses the permission check
if the file owner is the same as the user accessing the file.

>   Does the
> above test also fail if the file is owned by a different user than the
> one running the test?

I modified the test so that the file is created by different user from
the one that got the write delegation (on an open with O_WRONLY) and tries
to do the read using the deleg stateid. The read fails due to the check
in nfsd_permission which is the correct behavior.

>
>>> I think you need to acquire a R/W open from the get-go instead when you
>>> intend to grant a delegation, and just fall back to doing a O_WRONLY
>>> open if that fails (and don't grant one).

I think so far this patch works as expected, except that it might not
work for some filesystems due to the strict permission checking that
you mentioned above. However I'll explore your suggestion to acquire
a R/W open from the get-go to see how it goes.

Thanks,
-Dai

>>>
>>>> +	swap(fp->fi_fds[O_RDWR], fp->fi_fds[O_WRONLY]);
>>>> +	clear_access(NFS4_SHARE_ACCESS_WRITE, stp);
>>>> +	set_access(NFS4_SHARE_ACCESS_BOTH, stp);
>>>> +	__nfs4_file_get_access(fp, NFS4_SHARE_ACCESS_READ);	/* incr fi_access[O_RDONLY] */
>>>> +	spin_unlock(&fp->fi_lock);
>>>> +}
>>>> +
>>>> +/*
>>>> + * Downgrade file access mode to remove FMODE_READ. This is called when
>>>> + * a write delegation, granted for an OPEN with OPEN4_SHARE_ACCESS_WRITE,
>>>> + * is returned.
>>>> + */
>>>> +static void
>>>> +nfs4_downgrade_wronly_file_access(struct nfs4_ol_stateid *stp)
>>>> +{
>>>> +	struct nfs4_file *fp = stp->st_stid.sc_file;
>>>> +	struct nfsd_file *nflp;
>>>> +	struct file *file;
>>>> +
>>>> +	spin_lock(&fp->fi_lock);
>>>> +	nflp = fp->fi_fds[O_RDWR];
>>>> +	file = nflp->nf_file;
>>>> +	file->f_mode &= ~FMODE_READ;
>>>> +	swap(fp->fi_fds[O_WRONLY], fp->fi_fds[O_RDWR]);
>>>> +	clear_access(NFS4_SHARE_ACCESS_BOTH, stp);
>>>> +	set_access(NFS4_SHARE_ACCESS_WRITE, stp);
>>>> +	spin_unlock(&fp->fi_lock);
>>>> +	nfs4_file_put_access(fp, NFS4_SHARE_ACCESS_READ);	/* decr. fi_access[O_RDONLY] */
>>>> +}
>>>> +
>>>>    /*
>>>>     * The Linux NFS server does not offer write delegations to NFSv4.0
>>>>     * clients in order to avoid conflicts between write delegations and
>>>> @@ -6207,6 +6252,11 @@ nfs4_open_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
>>>>    		dp->dl_cb_fattr.ncf_cur_fsize = stat.size;
>>>>    		dp->dl_cb_fattr.ncf_initial_cinfo = nfsd4_change_attribute(&stat);
>>>>    		trace_nfsd_deleg_write(&dp->dl_stid.sc_stateid);
>>>> +
>>>> +		if ((open->op_share_access & NFS4_SHARE_ACCESS_BOTH) == NFS4_SHARE_ACCESS_WRITE) {
>>>> +			dp->dl_stateid = stp->st_stid.sc_stateid;
>>>> +			nfs4_upgrade_rdwr_file_access(stp);
>>>> +		}
>>>>    	} else {
>>>>    		open->op_delegate_type = deleg_ts ? OPEN_DELEGATE_READ_ATTRS_DELEG :
>>>>    						    OPEN_DELEGATE_READ;
>>>> @@ -7710,6 +7760,8 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>>    	struct nfs4_stid *s;
>>>>    	__be32 status;
>>>>    	struct nfsd_net *nn = net_generic(SVC_NET(rqstp), nfsd_net_id);
>>>> +	struct nfs4_ol_stateid *stp;
>>>> +	struct nfs4_stid *stid;
>>>>    
>>>>    	if ((status = fh_verify(rqstp, &cstate->current_fh, S_IFREG, 0)))
>>>>    		return status;
>>>> @@ -7724,6 +7776,16 @@ nfsd4_delegreturn(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>>>>    
>>>>    	trace_nfsd_deleg_return(stateid);
>>>>    	destroy_delegation(dp);
>>>> +
>>>> +	if (dp->dl_stateid.si_generation && dp->dl_stateid.si_opaque.so_id) {
>>>> +		if (!nfsd4_lookup_stateid(cstate, &dp->dl_stateid,
>>>> +				SC_TYPE_OPEN, 0, &stid, nn)) {
>>>> +			stp = openlockstateid(stid);
>>>> +			nfs4_downgrade_wronly_file_access(stp);
>>>> +			nfs4_put_stid(stid);
>>>> +		}
>>>> +	}
>>>> +
>>>>    	smp_mb__after_atomic();
>>>>    	wake_up_var(d_inode(cstate->current_fh.fh_dentry));
>>>>    put_stateid:
>>>> diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
>>>> index 74d2d7b42676..3f2f1b92db66 100644
>>>> --- a/fs/nfsd/state.h
>>>> +++ b/fs/nfsd/state.h
>>>> @@ -207,6 +207,8 @@ struct nfs4_delegation {
>>>>    
>>>>    	/* for CB_GETATTR */
>>>>    	struct nfs4_cb_fattr    dl_cb_fattr;
>>>> +
>>>> +	stateid_t		dl_stateid;  /* open stateid */
>>>>    };
>>>>    
>>>>    static inline bool deleg_is_read(u32 dl_type)