2023-12-24 08:25:27

by Zhipeng Lu

[permalink] [raw]
Subject: [PATCH] SUNRPC: fix some memleaks in gssx_dec_option_array

The creds and oa->data need to be freed in the error-handling paths after
there allocation. So this patch add these deallocations in the
corresponding paths.

Fixes: 1d658336b05f ("SUNRPC: Add RPC based upcall mechanism for RPCGSS auth")
Signed-off-by: Zhipeng Lu <[email protected]>
---
net/sunrpc/auth_gss/gss_rpc_xdr.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
index d79f12c2550a..de533b20231b 100644
--- a/net/sunrpc/auth_gss/gss_rpc_xdr.c
+++ b/net/sunrpc/auth_gss/gss_rpc_xdr.c
@@ -250,8 +250,8 @@ static int gssx_dec_option_array(struct xdr_stream *xdr,

creds = kzalloc(sizeof(struct svc_cred), GFP_KERNEL);
if (!creds) {
- kfree(oa->data);
- return -ENOMEM;
+ err = -ENOMEM;
+ goto free_oa;
}

oa->data[0].option.data = CREDS_VALUE;
@@ -265,29 +265,41 @@ static int gssx_dec_option_array(struct xdr_stream *xdr,

/* option buffer */
p = xdr_inline_decode(xdr, 4);
- if (unlikely(p == NULL))
- return -ENOSPC;
+ if (unlikely(p == NULL)) {
+ err = -ENOSPC
+ goto free_creds;
+ }

length = be32_to_cpup(p);
p = xdr_inline_decode(xdr, length);
- if (unlikely(p == NULL))
- return -ENOSPC;
+ if (unlikely(p == NULL)) {
+ err = -ENOSPC
+ goto free_creds;
+ }

if (length == sizeof(CREDS_VALUE) &&
memcmp(p, CREDS_VALUE, sizeof(CREDS_VALUE)) == 0) {
/* We have creds here. parse them */
err = gssx_dec_linux_creds(xdr, creds);
if (err)
- return err;
+ goto free_creds;
oa->data[0].value.len = 1; /* presence */
} else {
/* consume uninteresting buffer */
err = gssx_dec_buffer(xdr, &dummy);
if (err)
- return err;
+ goto free_creds;
}
}
return 0;
+
+free_creds:
+ kfree(creds);
+free_oa:
+ kfree(oa->data);
+ oa->data = NULL;
+err:
+ return err;
}

static int gssx_dec_status(struct xdr_stream *xdr,
--
2.34.1



2023-12-24 21:28:07

by Simon Horman

[permalink] [raw]
Subject: Re: [PATCH] SUNRPC: fix some memleaks in gssx_dec_option_array

On Sun, Dec 24, 2023 at 04:24:22PM +0800, Zhipeng Lu wrote:
> The creds and oa->data need to be freed in the error-handling paths after
> there allocation. So this patch add these deallocations in the
> corresponding paths.
>
> Fixes: 1d658336b05f ("SUNRPC: Add RPC based upcall mechanism for RPCGSS auth")
> Signed-off-by: Zhipeng Lu <[email protected]>

...

> diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c

...

> @@ -265,29 +265,41 @@ static int gssx_dec_option_array(struct xdr_stream *xdr,
>
> /* option buffer */
> p = xdr_inline_decode(xdr, 4);
> - if (unlikely(p == NULL))
> - return -ENOSPC;
> + if (unlikely(p == NULL)) {
> + err = -ENOSPC

Hi Zhipeng Lu,

unfortunately the line above causes a build failure.

...

2023-12-25 15:50:55

by Zhipeng Lu

[permalink] [raw]
Subject: Re: Re: [PATCH] SUNRPC: fix some memleaks in gssx_dec_option_array



>
> On Sun, Dec 24, 2023 at 04:24:22PM +0800, Zhipeng Lu wrote:
> > The creds and oa->data need to be freed in the error-handling paths after
> > there allocation. So this patch add these deallocations in the
> > corresponding paths.
> >
> > Fixes: 1d658336b05f ("SUNRPC: Add RPC based upcall mechanism for RPCGSS auth")
> > Signed-off-by: Zhipeng Lu <[email protected]>
>
> ...
>
> > diff --git a/net/sunrpc/auth_gss/gss_rpc_xdr.c b/net/sunrpc/auth_gss/gss_rpc_xdr.c
>
> ...
>
> > @@ -265,29 +265,41 @@ static int gssx_dec_option_array(struct xdr_stream *xdr,
> >
> > /* option buffer */
> > p = xdr_inline_decode(xdr, 4);
> > - if (unlikely(p == NULL))
> > - return -ENOSPC;
> > + if (unlikely(p == NULL)) {
> > + err = -ENOSPC
>
> Hi Zhipeng Lu,
>
> unfortunately the line above causes a build failure.
>
> ...

Sorry for my mistake, I'll send a version 2 of this patch soon.

2023-12-25 17:56:54

by kernel test robot

[permalink] [raw]
Subject: Re: [PATCH] SUNRPC: fix some memleaks in gssx_dec_option_array

Hi Zhipeng,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.7-rc7 next-20231222]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url: https://github.com/intel-lab-lkp/linux/commits/Zhipeng-Lu/SUNRPC-fix-some-memleaks-in-gssx_dec_option_array/20231225-152918
base: linus/master
patch link: https://lore.kernel.org/r/20231224082424.3539726-1-alexious%40zju.edu.cn
patch subject: [PATCH] SUNRPC: fix some memleaks in gssx_dec_option_array
config: nios2-randconfig-r081-20231225 (https://download.01.org/0day-ci/archive/20231226/[email protected]/config)
compiler: nios2-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231226/[email protected]/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/

All error/warnings (new ones prefixed by >>):

net/sunrpc/auth_gss/gss_rpc_xdr.c: In function 'gssx_dec_option_array':
>> net/sunrpc/auth_gss/gss_rpc_xdr.c:270:25: error: expected ';' before 'goto'
270 | goto free_creds;
| ^~~~
net/sunrpc/auth_gss/gss_rpc_xdr.c:277:25: error: expected ';' before 'goto'
277 | goto free_creds;
| ^~~~
>> net/sunrpc/auth_gss/gss_rpc_xdr.c:301:1: warning: label 'err' defined but not used [-Wunused-label]
301 | err:
| ^~~


vim +270 net/sunrpc/auth_gss/gss_rpc_xdr.c

228
229 static int gssx_dec_option_array(struct xdr_stream *xdr,
230 struct gssx_option_array *oa)
231 {
232 struct svc_cred *creds;
233 u32 count, i;
234 __be32 *p;
235 int err;
236
237 p = xdr_inline_decode(xdr, 4);
238 if (unlikely(p == NULL))
239 return -ENOSPC;
240 count = be32_to_cpup(p++);
241 if (!count)
242 return 0;
243
244 /* we recognize only 1 currently: CREDS_VALUE */
245 oa->count = 1;
246
247 oa->data = kmalloc(sizeof(struct gssx_option), GFP_KERNEL);
248 if (!oa->data)
249 return -ENOMEM;
250
251 creds = kzalloc(sizeof(struct svc_cred), GFP_KERNEL);
252 if (!creds) {
253 err = -ENOMEM;
254 goto free_oa;
255 }
256
257 oa->data[0].option.data = CREDS_VALUE;
258 oa->data[0].option.len = sizeof(CREDS_VALUE);
259 oa->data[0].value.data = (void *)creds;
260 oa->data[0].value.len = 0;
261
262 for (i = 0; i < count; i++) {
263 gssx_buffer dummy = { 0, NULL };
264 u32 length;
265
266 /* option buffer */
267 p = xdr_inline_decode(xdr, 4);
268 if (unlikely(p == NULL)) {
269 err = -ENOSPC
> 270 goto free_creds;
271 }
272
273 length = be32_to_cpup(p);
274 p = xdr_inline_decode(xdr, length);
275 if (unlikely(p == NULL)) {
276 err = -ENOSPC
277 goto free_creds;
278 }
279
280 if (length == sizeof(CREDS_VALUE) &&
281 memcmp(p, CREDS_VALUE, sizeof(CREDS_VALUE)) == 0) {
282 /* We have creds here. parse them */
283 err = gssx_dec_linux_creds(xdr, creds);
284 if (err)
285 goto free_creds;
286 oa->data[0].value.len = 1; /* presence */
287 } else {
288 /* consume uninteresting buffer */
289 err = gssx_dec_buffer(xdr, &dummy);
290 if (err)
291 goto free_creds;
292 }
293 }
294 return 0;
295
296 free_creds:
297 kfree(creds);
298 free_oa:
299 kfree(oa->data);
300 oa->data = NULL;
> 301 err:
302 return err;
303 }
304

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki