2021-09-01 23:39:35

by NeilBrown

Subject: [PATCH] SUNRPC: improve error response to over-size gss credential

When the NFS server receives a large gss (kerberos) credential and tries
to pass it up to rpc.svcgssd (which is deprecated), it triggers an
infinite loop in cache_read().

cache_request() always returns -EAGAIN, and this causes a "goto again".

This patch:
- changes the error to -E2BIG to avoid the infinite loop, and
- generates a WARN_ONCE when rsi_request first sees an over-sized
credential. The warning suggests switching to gssproxy.

Link: https://bugzilla.kernel.org/show_bug.cgi?id=196583
Signed-off-by: NeilBrown <[email protected]>
---
net/sunrpc/auth_gss/svcauth_gss.c | 2 ++
net/sunrpc/cache.c | 2 +-
2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index a81be45f40d9..e738c0182f09 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c
@@ -194,6 +194,8 @@ static void rsi_request(struct cache_detail *cd,
qword_addhex(bpp, blen, rsii->in_handle.data, rsii->in_handle.len);
qword_addhex(bpp, blen, rsii->in_token.data, rsii->in_token.len);
(*bpp)[-1] = '\n';
+ WARN_ONCE(*blen < 0,
+ "RPCSEC/GSS credential too large - please use gssproxy\n");
}

static int rsi_parse(struct cache_detail *cd,
diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
index 1a2c1c44bb00..59641803472c 100644
--- a/net/sunrpc/cache.c
+++ b/net/sunrpc/cache.c
@@ -803,7 +803,7 @@ static int cache_request(struct cache_detail *detail,

detail->cache_request(detail, crq->item, &bp, &len);
if (len < 0)
- return -EAGAIN;
+ return -E2BIG;
return PAGE_SIZE - len;
}

--
2.32.0

2021-09-03 17:54:44

by Chuck Lever III

Subject: Re: [PATCH] SUNRPC: improve error response to over-size gss credential

> On Sep 1, 2021, at 7:30 PM, NeilBrown <[email protected]> wrote:
>
>
> When the NFS server receives a large gss (kerberos) credential and tries
> to pass it up to rpc.svcgssd (which is deprecated), it triggers an
> infinite loop in cache_read().
>
> cache_request() always returns -EAGAIN, and this causes a "goto again".
>
> This patch:
> - changes the error to -E2BIG to avoid the infinite loop, and
> - generates a WARN_ONCE when rsi_request first sees an over-sized
> credential. The warning suggests switching to gssproxy.
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=196583
> Signed-off-by: NeilBrown <[email protected]>

Thanks, Neil, I've queued this up for nfsd-5.15-1.


> ---
> net/sunrpc/auth_gss/svcauth_gss.c | 2 ++
> net/sunrpc/cache.c | 2 +-
> 2 files changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
> index a81be45f40d9..e738c0182f09 100644
> --- a/net/sunrpc/auth_gss/svcauth_gss.c
> +++ b/net/sunrpc/auth_gss/svcauth_gss.c
> @@ -194,6 +194,8 @@ static void rsi_request(struct cache_detail *cd,
> qword_addhex(bpp, blen, rsii->in_handle.data, rsii->in_handle.len);
> qword_addhex(bpp, blen, rsii->in_token.data, rsii->in_token.len);
> (*bpp)[-1] = '\n';
> + WARN_ONCE(*blen < 0,
> + "RPCSEC/GSS credential too large - please use gssproxy\n");
> }
>
> static int rsi_parse(struct cache_detail *cd,
> diff --git a/net/sunrpc/cache.c b/net/sunrpc/cache.c
> index 1a2c1c44bb00..59641803472c 100644
> --- a/net/sunrpc/cache.c
> +++ b/net/sunrpc/cache.c
> @@ -803,7 +803,7 @@ static int cache_request(struct cache_detail *detail,
>
> detail->cache_request(detail, crq->item, &bp, &len);
> if (len < 0)
> - return -EAGAIN;
> + return -E2BIG;
> return PAGE_SIZE - len;
> }
>
> --
> 2.32.0
>

--
Chuck Lever