Linux-NFS Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH RFC 0/2] Fix problems with NFSv4 on krb5p
@ 2020-07-23 23:14 Chuck Lever
  2020-07-23 23:14 ` [PATCH RFC 1/2] SUNRPC: Set rcv_buf->len correctly in gss_unwrap_kerberos_v2() Chuck Lever
  2020-07-23 23:14 ` [PATCH RFC 2/2] SUNRPC: Fix buf->len calculation in unwrap_priv_data() Chuck Lever
  0 siblings, 2 replies; 3+ messages in thread
From: Chuck Lever @ 2020-07-23 23:14 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH RFC 1/2] SUNRPC: Set rcv_buf->len correctly in gss_unwrap_kerberos_v2()
  2020-07-23 23:14 [PATCH RFC 0/2] Fix problems with NFSv4 on krb5p Chuck Lever
@ 2020-07-23 23:14 ` Chuck Lever
  2020-07-23 23:14 ` [PATCH RFC 2/2] SUNRPC: Fix buf->len calculation in unwrap_priv_data() Chuck Lever
  1 sibling, 0 replies; 3+ messages in thread
From: Chuck Lever @ 2020-07-23 23:14 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

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);



^ permalink raw reply	[flat|nested] 3+ messages in thread

* [PATCH RFC 2/2] SUNRPC: Fix buf->len calculation in unwrap_priv_data()
  2020-07-23 23:14 [PATCH RFC 0/2] Fix problems with NFSv4 on krb5p Chuck Lever
  2020-07-23 23:14 ` [PATCH RFC 1/2] SUNRPC: Set rcv_buf->len correctly in gss_unwrap_kerberos_v2() Chuck Lever
@ 2020-07-23 23:14 ` Chuck Lever
  1 sibling, 0 replies; 3+ messages in thread
From: Chuck Lever @ 2020-07-23 23:14 UTC (permalink / raw)
  To: bfields; +Cc: linux-nfs

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. */



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, back to index

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23 23:14 [PATCH RFC 0/2] Fix problems with NFSv4 on krb5p Chuck Lever
2020-07-23 23:14 ` [PATCH RFC 1/2] SUNRPC: Set rcv_buf->len correctly in gss_unwrap_kerberos_v2() Chuck Lever
2020-07-23 23:14 ` [PATCH RFC 2/2] SUNRPC: Fix buf->len calculation in unwrap_priv_data() Chuck Lever

Linux-NFS Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-nfs/0 linux-nfs/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-nfs linux-nfs/ https://lore.kernel.org/linux-nfs \
		linux-nfs@vger.kernel.org
	public-inbox-index linux-nfs

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-nfs


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git