2018-11-30 20:46:15

by Chuck Lever III

Subject: [PATCH] SUNRPC: Fix leak of krb5p encode pages

call_encode can be invoked more than once per RPC call. Ensure that
each call to gss_wrap_req_priv does not overwrite pointers to
previously allocated memory.

Signed-off-by: Chuck Lever <[email protected]>
Cc: [email protected]
---
net/sunrpc/auth_gss/auth_gss.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/net/sunrpc/auth_gss/auth_gss.c b/net/sunrpc/auth_gss/auth_gss.c
index 5d3f252..ba76547 100644
--- a/net/sunrpc/auth_gss/auth_gss.c
+++ b/net/sunrpc/auth_gss/auth_gss.c
@@ -1791,6 +1791,7 @@ static void gss_wrap_req_encode(kxdreproc_t encode, struct rpc_rqst *rqstp,
for (i=0; i < rqstp->rq_enc_pages_num; i++)
__free_page(rqstp->rq_enc_pages[i]);
kfree(rqstp->rq_enc_pages);
+ rqstp->rq_release_snd_buf = NULL;
}

static int
@@ -1799,6 +1800,9 @@ static void gss_wrap_req_encode(kxdreproc_t encode, struct rpc_rqst *rqstp,
struct xdr_buf *snd_buf = &rqstp->rq_snd_buf;
int first, last, i;

+ if (rqstp->rq_release_snd_buf)
+ rqstp->rq_release_snd_buf(rqstp);
+
if (snd_buf->page_len == 0) {
rqstp->rq_enc_pages_num = 0;
return 0;

2018-11-30 21:08:33

by Trond Myklebust

Subject: Re: [PATCH] SUNRPC: Fix leak of krb5p encode pages

On Fri, 2018-11-30 at 15:39 -0500, Chuck Lever wrote:
> call_encode can be invoked more than once per RPC call. Ensure that
> each call to gss_wrap_req_priv does not overwrite pointers to
> previously allocated memory.
>
> Signed-off-by: Chuck Lever <[email protected]>
> Cc: [email protected]
> ---
> net/sunrpc/auth_gss/auth_gss.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/net/sunrpc/auth_gss/auth_gss.c
> b/net/sunrpc/auth_gss/auth_gss.c
> index 5d3f252..ba76547 100644
> --- a/net/sunrpc/auth_gss/auth_gss.c
> +++ b/net/sunrpc/auth_gss/auth_gss.c
> @@ -1791,6 +1791,7 @@ static void gss_wrap_req_encode(kxdreproc_t
> encode, struct rpc_rqst *rqstp,
> for (i=0; i < rqstp->rq_enc_pages_num; i++)
> __free_page(rqstp->rq_enc_pages[i]);
> kfree(rqstp->rq_enc_pages);
> + rqstp->rq_release_snd_buf = NULL;
> }
>
> static int
> @@ -1799,6 +1800,9 @@ static void gss_wrap_req_encode(kxdreproc_t
> encode, struct rpc_rqst *rqstp,
> struct xdr_buf *snd_buf = &rqstp->rq_snd_buf;
> int first, last, i;
>
> + if (rqstp->rq_release_snd_buf)
> + rqstp->rq_release_snd_buf(rqstp);
> +
> if (snd_buf->page_len == 0) {
> rqstp->rq_enc_pages_num = 0;
> return 0;
>

Ouch... OK, I think I see where the bio_vec problem is happening.

If gss_wrap_req_priv() is replacing snd_buf->pages instead of reusing
the rq_enc_pages, then we need to call xdr_free_bvec() as part of
xs_stream_prepare_request() in order to ensure that we refresh the
contents of the bvec...

--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]