| Message ID | 20231224082424.3539726-1-alexious@zju.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SUNRPC: fix some memleaks in gssx_dec_option_array | expand |
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 <alexious@zju.edu.cn> ... > 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. ...
> > 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 <alexious@zju.edu.cn> > > ... > > > 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.
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/202312260138.JJkoofSt-lkp@intel.com/config) compiler: nios2-linux-gcc (GCC) 13.2.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231226/202312260138.JJkoofSt-lkp@intel.com/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 <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202312260138.JJkoofSt-lkp@intel.com/ 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
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,
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 <alexious@zju.edu.cn> --- net/sunrpc/auth_gss/gss_rpc_xdr.c | 28 ++++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-)