On Nov 28, 2022, at 8:11 AM, Jeff Layton [off-list ref] wrote:

On Sat, 2022-11-26 at 15:55 -0500, Chuck Lever wrote:

quoted

Fixes: 030d794bf498 ("SUNRPC: Use gssproxy upcall for server RPCGSS authentication.")
Signed-off-by: Chuck Lever <redacted>
Cc: <redacted>
---
net/sunrpc/auth_gss/svcauth_gss.c |    9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/sunrpc/auth_gss/svcauth_gss.c b/net/sunrpc/auth_gss/svcauth_gss.c
index bcd74dddbe2d..9a5db285d4ae 100644
--- a/net/sunrpc/auth_gss/svcauth_gss.c
+++ b/net/sunrpc/auth_gss/svcauth_gss.c

@@ -1162,18 +1162,23 @@ static int gss_read_proxy_verf(struct svc_rqst *rqstp,

		return res;

	inlen = svc_getnl(argv);
-	if (inlen > (argv->iov_len + rqstp->rq_arg.page_len))
+	if (inlen > (argv->iov_len + rqstp->rq_arg.page_len)) {
+		kfree(in_handle->data);

I wish this were more obvious in this code. It's not at all evident to
the casual reader that gss_read_common_verf calls dup_netobj here and
that you need to clean up after it. At a bare minimum, we ought to have
a comment to that effect over gss_read_common_verf. While you're in
there, that function is also pretty big to be marked static inline. Can
you change that too? Ditto for gss_read_verf.

Agreed: I've done that clean up in subsequent patches that are part
of the (yet to be posted) series to replace svc_get/putnl with
xdr_stream.

This seemed like a good fix to apply earlier rather than later. That
should enable it to be backported cleanly.

quoted

		return SVC_DENIED;
+	}

	pages = DIV_ROUND_UP(inlen, PAGE_SIZE);
	in_token->pages = kcalloc(pages, sizeof(struct page *), GFP_KERNEL);
-	if (!in_token->pages)
+	if (!in_token->pages) {
+		kfree(in_handle->data);
		return SVC_DENIED;
+	}
	in_token->page_base = 0;
	in_token->page_len = inlen;
	for (i = 0; i < pages; i++) {
		in_token->pages[i] = alloc_page(GFP_KERNEL);
		if (!in_token->pages[i]) {
+			kfree(in_handle->data);
			gss_free_in_token_pages(in_token);
			return SVC_DENIED;
		}

-- 
Jeff Layton [off-list ref]

--
Chuck Lever