Skip to content
/ linux Public

Commit 64303b9

Browse files
chuckleverSasha Levin
authored andcommitted
SUNRPC: auth_gss: fix memory leaks in XDR decoding error paths
commit 3e6397b upstream. The gssx_dec_ctx(), gssx_dec_status(), and gssx_dec_name() functions allocate memory via gssx_dec_buffer(), which calls kmemdup(). When a subsequent decode operation fails, these functions return immediately without freeing previously allocated buffers, causing memory leaks. The leak in gssx_dec_ctx() is particularly relevant because the caller (gssp_accept_sec_context_upcall) initializes several buffer length fields to non-zero values, resulting in memory allocation: struct gssx_ctx rctxh = { .exported_context_token.len = GSSX_max_output_handle_sz, .mech.len = GSS_OID_MAX_LEN, .src_name.display_name.len = GSSX_max_princ_sz, .targ_name.display_name.len = GSSX_max_princ_sz }; If, for example, gssx_dec_name() succeeds for src_name but fails for targ_name, the memory allocated for exported_context_token, mech, and src_name.display_name remains unreferenced and cannot be reclaimed. Add error handling with goto-based cleanup to free any previously allocated buffers before returning an error. Reported-by: Xingjing Deng <micro6947@gmail.com> Closes: https://lore.kernel.org/linux-nfs/CAK+ZN9qttsFDu6h1FoqGadXjMx1QXqPMoYQ=6O9RY4SxVTvKng@mail.gmail.com/ Fixes: 1d65833 ("SUNRPC: Add RPC based upcall mechanism for RPCGSS auth") Cc: stable@vger.kernel.org Reviewed-by: Jeff Layton <jlayton@kernel.org> Signed-off-by: Chuck Lever <chuck.lever@oracle.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent f343fd0 commit 64303b9

File tree

1 file changed

+64
-18
lines changed

1 file changed

+64
-18
lines changed

net/sunrpc/auth_gss/gss_rpc_xdr.c

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -320,29 +320,47 @@ static int gssx_dec_status(struct xdr_stream *xdr,
320320

321321
/* status->minor_status */
322322
p = xdr_inline_decode(xdr, 8);
323-
if (unlikely(p == NULL))
324-
return -ENOSPC;
323+
if (unlikely(p == NULL)) {
324+
err = -ENOSPC;
325+
goto out_free_mech;
326+
}
325327
p = xdr_decode_hyper(p, &status->minor_status);
326328

327329
/* status->major_status_string */
328330
err = gssx_dec_buffer(xdr, &status->major_status_string);
329331
if (err)
330-
return err;
332+
goto out_free_mech;
331333

332334
/* status->minor_status_string */
333335
err = gssx_dec_buffer(xdr, &status->minor_status_string);
334336
if (err)
335-
return err;
337+
goto out_free_major_status_string;
336338

337339
/* status->server_ctx */
338340
err = gssx_dec_buffer(xdr, &status->server_ctx);
339341
if (err)
340-
return err;
342+
goto out_free_minor_status_string;
341343

342344
/* we assume we have no options for now, so simply consume them */
343345
/* status->options */
344346
err = dummy_dec_opt_array(xdr, &status->options);
347+
if (err)
348+
goto out_free_server_ctx;
345349

350+
return 0;
351+
352+
out_free_server_ctx:
353+
kfree(status->server_ctx.data);
354+
status->server_ctx.data = NULL;
355+
out_free_minor_status_string:
356+
kfree(status->minor_status_string.data);
357+
status->minor_status_string.data = NULL;
358+
out_free_major_status_string:
359+
kfree(status->major_status_string.data);
360+
status->major_status_string.data = NULL;
361+
out_free_mech:
362+
kfree(status->mech.data);
363+
status->mech.data = NULL;
346364
return err;
347365
}
348366

@@ -505,28 +523,35 @@ static int gssx_dec_name(struct xdr_stream *xdr,
505523
/* name->name_type */
506524
err = gssx_dec_buffer(xdr, &dummy_netobj);
507525
if (err)
508-
return err;
526+
goto out_free_display_name;
509527

510528
/* name->exported_name */
511529
err = gssx_dec_buffer(xdr, &dummy_netobj);
512530
if (err)
513-
return err;
531+
goto out_free_display_name;
514532

515533
/* name->exported_composite_name */
516534
err = gssx_dec_buffer(xdr, &dummy_netobj);
517535
if (err)
518-
return err;
536+
goto out_free_display_name;
519537

520538
/* we assume we have no attributes for now, so simply consume them */
521539
/* name->name_attributes */
522540
err = dummy_dec_nameattr_array(xdr, &dummy_name_attr_array);
523541
if (err)
524-
return err;
542+
goto out_free_display_name;
525543

526544
/* we assume we have no options for now, so simply consume them */
527545
/* name->extensions */
528546
err = dummy_dec_opt_array(xdr, &dummy_option_array);
547+
if (err)
548+
goto out_free_display_name;
529549

550+
return 0;
551+
552+
out_free_display_name:
553+
kfree(name->display_name.data);
554+
name->display_name.data = NULL;
530555
return err;
531556
}
532557

@@ -649,32 +674,34 @@ static int gssx_dec_ctx(struct xdr_stream *xdr,
649674
/* ctx->state */
650675
err = gssx_dec_buffer(xdr, &ctx->state);
651676
if (err)
652-
return err;
677+
goto out_free_exported_context_token;
653678

654679
/* ctx->need_release */
655680
err = gssx_dec_bool(xdr, &ctx->need_release);
656681
if (err)
657-
return err;
682+
goto out_free_state;
658683

659684
/* ctx->mech */
660685
err = gssx_dec_buffer(xdr, &ctx->mech);
661686
if (err)
662-
return err;
687+
goto out_free_state;
663688

664689
/* ctx->src_name */
665690
err = gssx_dec_name(xdr, &ctx->src_name);
666691
if (err)
667-
return err;
692+
goto out_free_mech;
668693

669694
/* ctx->targ_name */
670695
err = gssx_dec_name(xdr, &ctx->targ_name);
671696
if (err)
672-
return err;
697+
goto out_free_src_name;
673698

674699
/* ctx->lifetime */
675700
p = xdr_inline_decode(xdr, 8+8);
676-
if (unlikely(p == NULL))
677-
return -ENOSPC;
701+
if (unlikely(p == NULL)) {
702+
err = -ENOSPC;
703+
goto out_free_targ_name;
704+
}
678705
p = xdr_decode_hyper(p, &ctx->lifetime);
679706

680707
/* ctx->ctx_flags */
@@ -683,17 +710,36 @@ static int gssx_dec_ctx(struct xdr_stream *xdr,
683710
/* ctx->locally_initiated */
684711
err = gssx_dec_bool(xdr, &ctx->locally_initiated);
685712
if (err)
686-
return err;
713+
goto out_free_targ_name;
687714

688715
/* ctx->open */
689716
err = gssx_dec_bool(xdr, &ctx->open);
690717
if (err)
691-
return err;
718+
goto out_free_targ_name;
692719

693720
/* we assume we have no options for now, so simply consume them */
694721
/* ctx->options */
695722
err = dummy_dec_opt_array(xdr, &ctx->options);
723+
if (err)
724+
goto out_free_targ_name;
725+
726+
return 0;
696727

728+
out_free_targ_name:
729+
kfree(ctx->targ_name.display_name.data);
730+
ctx->targ_name.display_name.data = NULL;
731+
out_free_src_name:
732+
kfree(ctx->src_name.display_name.data);
733+
ctx->src_name.display_name.data = NULL;
734+
out_free_mech:
735+
kfree(ctx->mech.data);
736+
ctx->mech.data = NULL;
737+
out_free_state:
738+
kfree(ctx->state.data);
739+
ctx->state.data = NULL;
740+
out_free_exported_context_token:
741+
kfree(ctx->exported_context_token.data);
742+
ctx->exported_context_token.data = NULL;
697743
return err;
698744
}
699745

0 commit comments

Comments
 (0)