[PATCH v2] NFS: return DENIED in decode_lock_denied if we cannot decode owner

[PATCH v2] NFS: return DENIED in decode_lock_denied if we cannot decode owner

[Roberto Bergantinos Corpas]

After commit 43502f6e8d1e ("NFS: fix open_owner_id_maxsz and related
fields.") we dramatically changed the size of the LOCK/LOCKT reply buffer
from 164 to 52 bytes. This made visible a situation on a buggy NFS
server that sent oversized denied responses which led to us returning
EIO instead of DENIED.

Now, apart from buggy server issue, this poses an interesting question:
what if other implementations have legitimate but bigger than we expect
lock owner response, especially now that we have reduced the size we
allocate for it (20 bytes for the lock owner part).

This change proposes to tackle that issue returning DENIED instead of
EIO regardless of whether we manage to decode the owner or not:

- At this point of decode_lock_denied we know there is an owner.
- Server legitimately denied the lock request, with an owner that fits on
  NFS4_OPAQUE_LIMIT but we returned EIO instead to userspace.
- The owner data is decoded but actually never used, decode_lock path
  simply retries, and decode_lockt turns it into 0 on file_lock and
  returns it to userspace.
- LOCK/LOCKT are the last operations on the compound so it's not relevant
  if we didn't advance the pointer.
- Also removes an inverted likely(!p) hint.

Fixes: 43502f6e8d1e ("NFS: fix open_owner_id_maxsz and related fields.")
Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
Reviewed-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
---
 v2:
  - fixed formatting issues

 fs/nfs/nfs4xdr.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index fc049ce4ba8a..9d0f7ad7525a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -5169,8 +5169,7 @@ static int decode_lock_denied(struct xdr_stream *xdr, struct file_lock *fl)
 	p = xdr_decode_hyper(p, &clientid); /* read 8 bytes */
 	namelen = be32_to_cpup(p); /* read 4 bytes */  /* have read all 32 bytes now */
 	p = xdr_inline_decode(xdr, namelen); /* variable size field */
-	if (likely(!p))
-		return -EIO;
+	/* We have an owner here, return DENIED */
 	return -NFS4ERR_DENIED;
 }
 
-- 
2.45.0

Re: [PATCH v2] NFS: return DENIED in decode_lock_denied if we cannot decode owner

[Jeff Layton]

On Thu, 2026-09-17 at 12:49 +0200, Roberto Bergantinos Corpas wrote:
> After commit 43502f6e8d1e ("NFS: fix open_owner_id_maxsz and related
> fields.") we dramatically changed the size of the LOCK/LOCKT reply buffer
> from 164 to 52 bytes. This made visible a situation on a buggy NFS
> server that sent oversized denied responses which led to us returning
> EIO instead of DENIED.
> 
> Now, apart from buggy server issue, this poses an interesting question:
> what if other implementations have legitimate but bigger than we expect
> lock owner response, especially now that we have reduced the size we
> allocate for it (20 bytes for the lock owner part).
> 
> This change proposes to tackle that issue returning DENIED instead of
> EIO regardless of whether we manage to decode the owner or not:
> 
> - At this point of decode_lock_denied we know there is an owner.
> - Server legitimately denied the lock request, with an owner that fits on
>   NFS4_OPAQUE_LIMIT but we returned EIO instead to userspace.
> - The owner data is decoded but actually never used, decode_lock path
>   simply retries, and decode_lockt turns it into 0 on file_lock and
>   returns it to userspace.
> - LOCK/LOCKT are the last operations on the compound so it's not relevant
>   if we didn't advance the pointer.
> - Also removes an inverted likely(!p) hint.
> 
> Fixes: 43502f6e8d1e ("NFS: fix open_owner_id_maxsz and related fields.")
> Signed-off-by: Roberto Bergantinos Corpas <rbergant@redhat.com>
> Reviewed-by: Prabhakar Pujeri <prabhakar.pujeri@dell.com>
> ---
>  v2:
>   - fixed formatting issues
> 
>  fs/nfs/nfs4xdr.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index fc049ce4ba8a..9d0f7ad7525a 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -5169,8 +5169,7 @@ static int decode_lock_denied(struct xdr_stream *xdr, struct file_lock *fl)
>  	p = xdr_decode_hyper(p, &clientid); /* read 8 bytes */
>  	namelen = be32_to_cpup(p); /* read 4 bytes */  /* have read all 32 bytes now */
>  	p = xdr_inline_decode(xdr, namelen); /* variable size field */
> -	if (likely(!p))
> -		return -EIO;
> +	/* We have an owner here, return DENIED */
>  	return -NFS4ERR_DENIED;
>  }
>  

Typically, we don't work around server bugs in the client (and vice
versa), but I think this is a reasonable thing to do in this case since
we don't use remote lockowner information anywhere in the kernel.

Reviewed-by: Jeff Layton <jlayton@kernel.org>

Re: [PATCH v2] NFS: return DENIED in decode_lock_denied if we cannot decode owner

[Tomas Dabasinskas]

On Wed, 2026-09-17, Roberto Bergantinos Corpas wrote:
> a buggy NFS server that sent oversized denied responses which led to
> us returning EIO instead of DENIED

We hit this in production last week and spent a while tracking it down
before finding your patch, so here is an independent data point, plus a
request to consider it for stable.

The server is Google Cloud Filestore (NFSv4.1, zonal tier), which is
nfs-ganesha based. When a blocking lock conflicts, it reports the
conflicting owner as the 21 byte placeholder "ganesha_unknown_owner".
With decode_lock_denied_maxsz now budgeting 52 bytes, which allows a 20
byte owner, decode_lock_denied() cannot decode it, and userspace gets
EIO from a plain F_SETLKW instead of waiting. I confirmed the same
behaviour against upstream nfs-ganesha 6.5 on a plain VM, so it is not
specific to Google's build.

Reproducer, two processes on one client, one file on an NFSv4.1 mount:

  import fcntl, time
  for i in range(20):
      f = open('/mnt/share/lockfile', 'a+b')
      fcntl.lockf(f, fcntl.LOCK_EX)   # EIO here instead of blocking
      time.sleep(0.05)
      fcntl.lockf(f, fcntl.LOCK_UN)
      f.close()

Run two of them at once. A single process never fails, since nothing
has to be reported as denied.

Results, same server and same mount options throughout
(vers=4.1,retrans=2,timeo=30), 8 processes x 40 iterations:

  6.1.0-53       320 locks OK, 0 EIO
  6.8.0-1069     320 locks OK, 0 EIO
  6.11.0-1013    320 locks OK, 0 EIO
  6.12.107       1 lock OK, 319 EIO
  7.0.0-1011     same failure

Tracepoint on the failing kernels:

  nfs4_set_lock: error=-5 (EIO) cmd=SETLKW:WRLCK

The capture shows the server replying NFS4ERR_DENIED with a 21 byte
owner, and no CB_NOTIFY_LOCK is ever requested, because the client
never reaches the wait path. The same client against a Linux knfsd
export shows no failures at all.

The practical impact is wider than one filer: every GKE node image on
6.12 or later hits this against Filestore, and anything taking file
locks on such a share stops working. For us it was OpenTofu's shared
provider cache, which takes one blocking lock per provider and treats
EIO as fatal. That is also why a stable backport would help, since
cloud node images follow the stable trees rather than mainline.

I reported the placeholder owner to nfs-ganesha as well, since
returning the real owner, or at least a shorter placeholder, would help
clients that are already deployed:

  https://github.com/nfs-ganesha/nfs-ganesha/issues/1420

Thanks for the fix.

Tomas

RE: [PATCH v2] NFS: return DENIED in decode_lock_denied if we cannot decode owner

[Frank Filz]

The thing is, it may not be a server bug. The protocol specifies a limit of 1024 bytes for the owner opaque. The owner may come from something other than another Linux client.

In fact, as has been pointed out by someone else, Ganesha returns a 21 byte "ganesha_unknown_owner" when the lock owner is not another NFS owner. Obviously we could (and probably will) shorten this, but the owner could just as well come from another client, or be some representation of a local owner. Nothing in the protocol even gives a hint that the owner should be limited to something as small as 20 bytes.

Personally I have wondered of the conflicting lock owner is really useful to anyone anyway. Obviously the Linux client does nothing with it (which really is reasonable since the POSIX fcntl call only allows for returning a pid). I guess there might be some use in a tcpdump trace (but even there, one might argue it would be nice to see the IP address of the client holding the conflicting lock along with its [probably 20 byte] lock owner).

Frank

-----Original Message-----
From: Jeff Layton [mailto:jlayton@kernel.org] 
Sent: Thursday, September 17, 2026 5:24 AM
To: Roberto Bergantinos Corpas <rbergant@redhat.com>; trondmy@kernel.org; anna@kernel.org
Cc: neil@brown.name; prabhakar.pujeri@dell.com; linux-nfs@vger.kernel.org
Subject: Re: [PATCH v2] NFS: return DENIED in decode_lock_denied if we cannot decode owner

...  

Typically, we don't work around server bugs in the client (and vice versa), but I think this is a reasonable thing to do in this case since we don't use remote lockowner information anywhere in the kernel.

Reviewed-by: Jeff Layton <jlayton@kernel.org>