[PATCH RFC 0/2] Fix problems with NFSv4 on krb5p

[PATCH RFC 0/2] Fix problems with NFSv4 on krb5p

[Chuck Lever]

This tiny series would address a couple of bugs in the recent commits
that fixed xdr_buf_trim(), namely:

  31c9590ae468 ("SUNRPC: Add "@len" parameter to gss_unwrap()")
  a7e429a6fa6d ("SUNRPC: Fix GSS privacy computation of auth->au_ralign")
  0a8e7b7d0846 ("SUNRPC: Revert 241b1f419f0e ("SUNRPC: Remove xdr_buf_trim()")")

Turns out 31c9590ae468 had a couple of problems that were introduced
by refactoring late, and therefore were not caught during testing. The
client-side problems are documented here:

  https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1886277

The first patch addresses the NFSv4/krb5p failures reported in that
bug. It is a straightforward and obvious fix.

When this fix is applied on the server, NFSv3/krb5p stops working.
Thus the second patch is also needed, but this patch is somewhat
more controversial. It's not clear to me how much of the "pad
adjustment" logic is still needed in unwrap_priv_data(), so I'm
asking for some quick but careful review of this proposed change.

---

Chuck Lever (2):
      SUNRPC: Set rcv_buf->len correctly in gss_unwrap_kerberos_v2()
      SUNRPC: Fix buf->len calculation in unwrap_priv_data()


 net/sunrpc/auth_gss/gss_krb5_wrap.c | 2 +-
 net/sunrpc/auth_gss/svcauth_gss.c   | 2 --
 2 files changed, 1 insertion(+), 3 deletions(-)

--
Chuck Lever

[PATCH RFC 1/2] SUNRPC: Set rcv_buf->len correctly in gss_unwrap_kerberos_v2()

[Chuck Lever]

Braino when converting "buf->len -=" to "buf->len = len -".

The result is under-estimation of the ralign and rslack values. On
krb5p mounts, this has caused READDIR to fail with EIO, and KASAN
splats when decoding READLINK replies.

Reported-by: Marian Rainer-Harbach
Reported-by: Pierre Sauter <pierre.sauter@stwm.de>
BugLink: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1886277
Fixes: 31c9590ae468 ("SUNRPC: Add "@len" parameter to gss_unwrap()")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/gss_krb5_wrap.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
index cf0fd170ac18..90b8329fef82 100644
--- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
+++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
@@ -584,7 +584,7 @@ gss_unwrap_kerberos_v2(struct krb5_ctx *kctx, int offset, int len,
 							buf->head[0].iov_len);
 	memmove(ptr, ptr + GSS_KRB5_TOK_HDR_LEN + headskip, movelen);
 	buf->head[0].iov_len -= GSS_KRB5_TOK_HDR_LEN + headskip;
-	buf->len = len - GSS_KRB5_TOK_HDR_LEN + headskip;
+	buf->len = len - (GSS_KRB5_TOK_HDR_LEN + headskip);
 
 	/* Trim off the trailing "extra count" and checksum blob */
 	xdr_buf_trim(buf, ec + GSS_KRB5_TOK_HDR_LEN + tailskip);

[PATCH RFC 2/2] SUNRPC: Fix buf->len calculation in unwrap_priv_data()

[Chuck Lever]

The pad adjustment logic in unwrap_priv_data() is unnecessary.
gss_unwrap() should already updates buf->len correctly; the
additional adjustment can drive buf->len negative. This causes the
nfsd_request_too_large check to fail during some NFSv3 operations.

Fixes: 31c9590ae468 ("SUNRPC: Add "@len" parameter to gss_unwrap()")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c |    2 --
 1 file changed, 2 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index 7d83f54aaaa6..9ac23d83f9a2 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -989,8 +989,6 @@ unwrap_priv_data(struct svc_rqst *rqstp, struct xdr_buf *buf, u32 seq, struct gs
 	fix_priv_head(buf, pad);
 
 	maj_stat = gss_unwrap(ctx, 0, priv_len, buf);
-	pad = priv_len - buf->len;
-	buf->len -= pad;
 	/* The upper layers assume the buffer is aligned on 4-byte boundaries.
 	 * In the krb5p case, at least, the data ends up offset, so we need to
 	 * move it around. */