[PATCH 0/4] sunrpc: close length validation gaps in krb5p unwrap

[PATCH 0/4] sunrpc: close length validation gaps in krb5p unwrap

[Chuck Lever]

From: Chuck Lever <chuck.lever@oracle.com>

Neither svcauth_gss_unwrap_priv() nor gss_unwrap_resp_priv()
enforces a minimum on the wire-supplied opaque length before
forwarding it to the krb5 unwrap core.  An authenticated krb5p
peer -- NFS client or server -- can send a token shorter than the
16-byte RFC 4121 header and reach gss_krb5_unwrap_v2() with a
length that drives pre-decrypt out-of-bounds reads, a
divide-by-zero in _rotate_left(), and a post-decrypt unsigned
underflow in the movelen computation that feeds a multi-gigabyte
memmove.

The fixes are layered so that neither the caller boundary nor
the unwrap core depends on the other for safety.  The caller-side
floor checks (patches 1, 2) are sufficient to close the attack,
but gss_krb5_unwrap_v2() must also be self-defending because it
is reachable through gss_unwrap() from any future caller that
might not enforce the same floor.  The xdr_buf_trim() clamp
(patch 3) is independent of krb5 -- it corrects a generic
invariant violation that gss_krb5_unwrap_v2() happens to be the
only current trigger for, but that any caller could hit if
buf->len and the component iov_lens get out of sync.

The client side (patch 2) has an additional u32 integer-overflow
bypass that the server side does not: the original
`offset + opaque_len > rcv_buf->len` wraps when opaque_len is
near UINT_MAX, accepting a huge token length that re-enters the
same underflow chain.  The replacement splits the check into
separate offset and length guards that are safe in u32 arithmetic.

Chris Mason (4):
  SUNRPC: svcauth_gss: enforce krb5 token minimum length
  SUNRPC: harden gss_unwrap_resp_priv length checks
  SUNRPC: xdr_buf_trim: clamp buf->len to avoid underflow
  SUNRPC: harden gss_krb5_unwrap_v2 against short tokens

 net/sunrpc/auth_gss/auth_gss.c      |  6 +++++-
 net/sunrpc/auth_gss/gss_krb5_wrap.c | 11 +++++++++--
 net/sunrpc/auth_gss/svcauth_gss.c   |  2 ++
 net/sunrpc/xdr.c                    |  2 +-
 4 files changed, 17 insertions(+), 4 deletions(-)

-- 
2.54.0

[PATCH 1/4] SUNRPC: svcauth_gss: enforce krb5 token minimum length

[Chuck Lever]

From: Chris Mason <clm@meta.com>

svcauth_gss_unwrap_priv() validates only an upper bound on the
wire-supplied opaque length before handing the buffer to
gss_unwrap():

    if (len > xdr_stream_remaining(xdr))
            goto unwrap_failed;
    offset = xdr_stream_pos(xdr);
    ...
    maj_stat = gss_unwrap(ctx, offset, offset + len, buf);

The wire value `len` flows unchanged as the upper bound into the
krb5 unwrap path, so a len in [0, 16] passes this check and is
handed to gss_unwrap(). For a krb5 v2 context that lands in
gss_krb5_unwrap_v2(), which reads the 16-byte RFC 4121 token
header fields at ptr+4 and ptr+6 and then calls rotate_left()
before any integrity check. With a sub-header length the header
reads run past the token, and _rotate_left()'s `shift %= buf->len`
path can divide by zero when buf->len has been driven to zero by
the truncated token. A header-only token (len == 16) is equally
invalid: with a non-zero RRC field and the opaque blob ending at
the XDR buffer boundary, rotate_left() builds a zero-length
subbuffer, reaching the same division.

Reject the token at the server entry point before it reaches the
krb5 unwrap core. A valid sealed RFC 4121 token must contain
the 16-byte header plus at least some encrypted payload.

Fix by adding a minimum-length check immediately after the
existing upper-bound check:

    if (len <= GSS_KRB5_TOK_HDR_LEN)
            goto unwrap_failed;

Fixes: 7c9fdcfb1b64 ("[PATCH] knfsd: svcrpc: gss: server-side implementation of rpcsec_gss privacy")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/svcauth_gss.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index d14209031e18..8e8aceb31270 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -949,6 +949,8 @@ svcauth_gss_unwrap_priv(struct svc_rqst *rqstp, u32 seq, struct gss_ctx *ctx)
 	}
 	if (len > xdr_stream_remaining(xdr))
 		goto unwrap_failed;
+	if (len <= GSS_KRB5_TOK_HDR_LEN)
+		goto unwrap_failed;
 	offset = xdr_stream_pos(xdr);
 
 	saved_len = buf->len;
-- 
2.54.0

[PATCH 2/4] SUNRPC: harden gss_unwrap_resp_priv length checks

[Chuck Lever]

From: Chris Mason <clm@meta.com>

gss_unwrap_resp_priv() validates the RPCSEC_GSS opaque length with

    offset = (u8 *)(p) - (u8 *)head->iov_base;
    if (offset + opaque_len > rcv_buf->len)
            goto unwrap_failed;
    maj_stat = gss_unwrap(ctx->gc_gss_ctx, offset,
                          offset + opaque_len, rcv_buf);

Both operands are u32 and the sum is computed in u32. A reply with
opaque_len near 0xffffffff makes offset + opaque_len wrap to a small
value that is below rcv_buf->len, so the bound check passes and
gss_unwrap() is called with end < begin. The check also lacks a
lower bound, so any opaque_len in [0, GSS_KRB5_TOK_HDR_LEN) is
accepted and forwarded to gss_krb5_unwrap_v2(), whose pre-decrypt
header reads at ptr+4 and ptr+6 then run past the token.

A krb5p NFS server returning a crafted RPCSEC_GSS reply can drive
the client into out-of-bounds reads in gss_krb5_unwrap_v2() and the
rotate_left() loop that follows.

Fix by replacing the single combined check with three guards that
are safe in u32 arithmetic and that enforce the RFC 4121 minimum
outer token length:

    if (offset > rcv_buf->len)
            goto unwrap_failed;
    if (opaque_len > rcv_buf->len - offset)
            goto unwrap_failed;
    if (opaque_len < GSS_KRB5_TOK_HDR_LEN)
            goto unwrap_failed;

The first guard makes the subtraction in the second guard
unconditionally safe; offset is derived from a successful
xdr_inline_decode() in the head kvec, so in practice it already
satisfies the bound. The floor mirrors the server-side check added
in commit 7507012eeb98 ("SUNRPC: svcauth_gss: enforce krb5 token
minimum length").

Fixes: 2d2da60c63b6 ("RPCSEC_GSS: client-side privacy support")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/auth_gss.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 9d3fb6848f40..8ddc65e894da 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -2072,7 +2072,11 @@ gss_unwrap_resp_priv(struct rpc_task *task, struct rpc_cred *cred,
 		goto unwrap_failed;
 	opaque_len = be32_to_cpup(p++);
 	offset = (u8 *)(p) - (u8 *)head->iov_base;
-	if (offset + opaque_len > rcv_buf->len)
+	if (offset > rcv_buf->len)
+		goto unwrap_failed;
+	if (opaque_len > rcv_buf->len - offset)
+		goto unwrap_failed;
+	if (opaque_len <= GSS_KRB5_TOK_HDR_LEN)
 		goto unwrap_failed;
 
 	maj_stat = gss_unwrap(ctx->gc_gss_ctx, offset,
-- 
2.54.0

[PATCH 3/4] SUNRPC: xdr_buf_trim: clamp buf->len to avoid underflow

[Chuck Lever]

From: Chris Mason <clm@meta.com>

xdr_buf_trim() trims `len` bytes from the tail of an xdr_buf by
walking the tail, pages, and head iovecs.  Each per-section step
uses min_t() so it never removes more bytes than that section
holds, but the final accounting at the fix_len label subtracts the
total bytes actually consumed from buf->len without any clamp:

    fix_len:
            buf->len -= (len - trim);

When the caller has set buf->len to a value smaller than the sum
of the iov_lens, (len - trim) can exceed buf->len and the unsigned
subtraction wraps to near UINT_MAX.  gss_krb5_unwrap_v2() reaches
xdr_buf_trim() in exactly that state:

    buf->head[0].iov_len -= GSS_KRB5_TOK_HDR_LEN + headskip;
    buf->len = len - (GSS_KRB5_TOK_HDR_LEN + headskip);
    xdr_buf_trim(buf, ec + GSS_KRB5_TOK_HDR_LEN + tailskip);

buf->len is a small wire-derived value while the iov_lens are at
page scale, so the per-section loops legitimately consume far more
bytes than buf->len records.  The wrapped buf->len then propagates
as the authoritative stream bound into every downstream XDR
decoder.

Fix by clamping the decrement so buf->len bottoms out at zero:

    buf->len -= min_t(unsigned int, buf->len, len - trim);

On the normal path where the iov_lens sum to buf->len, (len - trim)
is always <= buf->len and the result is identical to before.  No
callers change behavior outside the underflow case.

Fixes: 0a8e7b7d0846 ("SUNRPC: Revert 241b1f419f0e (\"SUNRPC: Remove xdr_buf_trim()\")")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/xdr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sunrpc/xdr.c b/net/sunrpc/xdr.c
index fa6a30b5f046..cb2ef428651f 100644
--- a/net/sunrpc/xdr.c
+++ b/net/sunrpc/xdr.c
@@ -2049,7 +2049,7 @@ void xdr_buf_trim(struct xdr_buf *buf, unsigned int len)
 		trim -= cur;
 	}
 fix_len:
-	buf->len -= (len - trim);
+	buf->len -= min_t(unsigned int, buf->len, len - trim);
 }
 EXPORT_SYMBOL_GPL(xdr_buf_trim);
 
-- 
2.54.0

[PATCH 4/4] SUNRPC: harden gss_krb5_unwrap_v2 against short tokens

[Chuck Lever]

From: Chris Mason <clm@meta.com>

gss_krb5_unwrap_v2() reads the EC and RRC header fields at ptr+4 and
ptr+6 before validating that the token is at least GSS_KRB5_TOK_HDR_LEN
(16) bytes long, and its rotate_left() helper passes buf->len - base
to xdr_buf_subsegment() without verifying that base <= buf->len. When
a caller hands in a sub-16-byte token, or a token whose declared len
leaves base past the end of the buffer, three distinct failures follow:

    gss_krb5_unwrap_v2(offset, len, buf)
      ptr = buf->head[0].iov_base + offset
      ec  = *(ptr + 4)              /* OOB read on short head */
      rrc = *(ptr + 6)              /* OOB read on short head */
      rotate_left(offset + 16, buf, rrc)
        xdr_buf_subsegment(buf, &subbuf,
                           base, buf->len - base)   /* u32 wrap when base > len */
        _rotate_left(&subbuf, shift)
          shift %= buf->len         /* divide-by-zero when base == len */

After decryption, the cleanup arithmetic has the same shape:

    movelen = min_t(unsigned int, buf->head[0].iov_len, len);
    movelen -= offset + GSS_KRB5_TOK_HDR_LEN + headskip;
    BUG_ON(offset + GSS_KRB5_TOK_HDR_LEN + headskip + movelen >
                                            buf->head[0].iov_len);

The BUG_ON re-adds the value just subtracted, so it reduces to
min(A, B) > A and is permanently false; it cannot catch the unsigned
underflow of movelen, which then drives a ~UINT_MAX-byte memmove().

Add four defense-in-depth guards inside the unwrap core so it is safe
regardless of what its callers validate:

  - reject tokens with len - offset < GSS_KRB5_TOK_HDR_LEN before
    touching ptr+4/ptr+6;
  - bail from rotate_left() when buf->len <= base, covering both the
    underflow and zero-length cases;
  - return early from _rotate_left() when buf->len is zero, so the
    shift %= buf->len modulo cannot fault;
  - replace the dead BUG_ON with a live check that returns
    GSS_S_DEFECTIVE_TOKEN before the movelen subtraction.

Fixes: de9c17eb4a91 ("gss_krb5: add support for new token formats in rfc4121")
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 net/sunrpc/auth_gss/gss_krb5_wrap.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_krb5_wrap.c b/net/sunrpc/auth_gss/gss_krb5_wrap.c
index ac4b32df42b9..d84c35f779f5 100644
--- a/net/sunrpc/auth_gss/gss_krb5_wrap.c
+++ b/net/sunrpc/auth_gss/gss_krb5_wrap.c
@@ -73,6 +73,8 @@ static void _rotate_left(struct xdr_buf *buf, unsigned int shift)
 	int shifted = 0;
 	int this_shift;
 
+	if (!buf->len)
+		return;
 	shift %= buf->len;
 	while (shifted < shift) {
 		this_shift = min(shift - shifted, LOCAL_BUF_LEN);
@@ -85,6 +87,8 @@ static void rotate_left(u32 base, struct xdr_buf *buf, unsigned int shift)
 {
 	struct xdr_buf subbuf;
 
+	if (buf->len <= base)
+		return;
 	xdr_buf_subsegment(buf, &subbuf, base, buf->len - base);
 	_rotate_left(&subbuf, shift);
 }
@@ -154,6 +158,9 @@ gss_krb5_unwrap_v2(struct krb5_ctx *kctx, int offset, int len,
 
 	dprintk("RPC:       %s\n", __func__);
 
+	if (len - offset <= GSS_KRB5_TOK_HDR_LEN)
+		return GSS_S_DEFECTIVE_TOKEN;
+
 	ptr = buf->head[0].iov_base + offset;
 
 	if (be16_to_cpu(*((__be16 *)ptr)) != KG2_TOK_WRAP)
@@ -220,9 +227,9 @@ gss_krb5_unwrap_v2(struct krb5_ctx *kctx, int offset, int len,
 	 * head buffer space rather than that actually occupied.
 	 */
 	movelen = min_t(unsigned int, buf->head[0].iov_len, len);
+	if (movelen < offset + GSS_KRB5_TOK_HDR_LEN + headskip)
+		return GSS_S_DEFECTIVE_TOKEN;
 	movelen -= offset + GSS_KRB5_TOK_HDR_LEN + headskip;
-	BUG_ON(offset + GSS_KRB5_TOK_HDR_LEN + headskip + movelen >
-							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);
-- 
2.54.0

Re: [PATCH 0/4] sunrpc: close length validation gaps in krb5p unwrap

[Jeff Layton]

On Sat, 2026-05-23 at 21:02 -0400, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> Neither svcauth_gss_unwrap_priv() nor gss_unwrap_resp_priv()
> enforces a minimum on the wire-supplied opaque length before
> forwarding it to the krb5 unwrap core.  An authenticated krb5p
> peer -- NFS client or server -- can send a token shorter than the
> 16-byte RFC 4121 header and reach gss_krb5_unwrap_v2() with a
> length that drives pre-decrypt out-of-bounds reads, a
> divide-by-zero in _rotate_left(), and a post-decrypt unsigned
> underflow in the movelen computation that feeds a multi-gigabyte
> memmove.
> 
> The fixes are layered so that neither the caller boundary nor
> the unwrap core depends on the other for safety.  The caller-side
> floor checks (patches 1, 2) are sufficient to close the attack,
> but gss_krb5_unwrap_v2() must also be self-defending because it
> is reachable through gss_unwrap() from any future caller that
> might not enforce the same floor.  The xdr_buf_trim() clamp
> (patch 3) is independent of krb5 -- it corrects a generic
> invariant violation that gss_krb5_unwrap_v2() happens to be the
> only current trigger for, but that any caller could hit if
> buf->len and the component iov_lens get out of sync.
> 
> The client side (patch 2) has an additional u32 integer-overflow
> bypass that the server side does not: the original
> `offset + opaque_len > rcv_buf->len` wraps when opaque_len is
> near UINT_MAX, accepting a huge token length that re-enters the
> same underflow chain.  The replacement splits the check into
> separate offset and length guards that are safe in u32 arithmetic.
> 
> Chris Mason (4):
>   SUNRPC: svcauth_gss: enforce krb5 token minimum length
>   SUNRPC: harden gss_unwrap_resp_priv length checks
>   SUNRPC: xdr_buf_trim: clamp buf->len to avoid underflow
>   SUNRPC: harden gss_krb5_unwrap_v2 against short tokens
> 
>  net/sunrpc/auth_gss/auth_gss.c      |  6 +++++-
>  net/sunrpc/auth_gss/gss_krb5_wrap.c | 11 +++++++++--
>  net/sunrpc/auth_gss/svcauth_gss.c   |  2 ++
>  net/sunrpc/xdr.c                    |  2 +-
>  4 files changed, 17 insertions(+), 4 deletions(-)

Looks good. The unsigned offset math in this parsing code is just so
nasty. I wonder if we'd be better off using signed math here for some
of this. Oh well...

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