SUNRPC: auth_gss: fix memory leaks in XDR decoding error paths

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: 1d658336b05f ("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>

Chuck Lever 4 months ago 3e6397b0 404d7794

+64 -18

1 changed file

expand all

net

sunrpc

auth_gss

gss_rpc_xdr.c

+64 -18

net/sunrpc/auth_gss/gss_rpc_xdr.c

··· 320 320 321 321 /* status->minor_status */ 322 322 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 + } 325 327 p = xdr_decode_hyper(p, &status->minor_status); 326 328 327 329 /* status->major_status_string */ 328 330 err = gssx_dec_buffer(xdr, &status->major_status_string); 329 331 if (err) 330 - return err; 332 + goto out_free_mech; 331 333 332 334 /* status->minor_status_string */ 333 335 err = gssx_dec_buffer(xdr, &status->minor_status_string); 334 336 if (err) 335 - return err; 337 + goto out_free_major_status_string; 336 338 337 339 /* status->server_ctx */ 338 340 err = gssx_dec_buffer(xdr, &status->server_ctx); 339 341 if (err) 340 - return err; 342 + goto out_free_minor_status_string; 341 343 342 344 /* we assume we have no options for now, so simply consume them */ 343 345 /* status->options */ 344 346 err = dummy_dec_opt_array(xdr, &status->options); 347 + if (err) 348 + goto out_free_server_ctx; 345 349 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; 346 364 return err; 347 365 } 348 366 ··· 523 505 /* name->name_type */ 524 506 err = gssx_dec_buffer(xdr, &dummy_netobj); 525 507 if (err) 526 - return err; 508 + goto out_free_display_name; 527 509 528 510 /* name->exported_name */ 529 511 err = gssx_dec_buffer(xdr, &dummy_netobj); 530 512 if (err) 531 - return err; 513 + goto out_free_display_name; 532 514 533 515 /* name->exported_composite_name */ 534 516 err = gssx_dec_buffer(xdr, &dummy_netobj); 535 517 if (err) 536 - return err; 518 + goto out_free_display_name; 537 519 538 520 /* we assume we have no attributes for now, so simply consume them */ 539 521 /* name->name_attributes */ 540 522 err = dummy_dec_nameattr_array(xdr, &dummy_name_attr_array); 541 523 if (err) 542 - return err; 524 + goto out_free_display_name; 543 525 544 526 /* we assume we have no options for now, so simply consume them */ 545 527 /* name->extensions */ 546 528 err = dummy_dec_opt_array(xdr, &dummy_option_array); 529 + if (err) 530 + goto out_free_display_name; 547 531 532 + return 0; 533 + 534 + out_free_display_name: 535 + kfree(name->display_name.data); 536 + name->display_name.data = NULL; 548 537 return err; 549 538 } 550 539 ··· 674 649 /* ctx->state */ 675 650 err = gssx_dec_buffer(xdr, &ctx->state); 676 651 if (err) 677 - return err; 652 + goto out_free_exported_context_token; 678 653 679 654 /* ctx->need_release */ 680 655 err = gssx_dec_bool(xdr, &ctx->need_release); 681 656 if (err) 682 - return err; 657 + goto out_free_state; 683 658 684 659 /* ctx->mech */ 685 660 err = gssx_dec_buffer(xdr, &ctx->mech); 686 661 if (err) 687 - return err; 662 + goto out_free_state; 688 663 689 664 /* ctx->src_name */ 690 665 err = gssx_dec_name(xdr, &ctx->src_name); 691 666 if (err) 692 - return err; 667 + goto out_free_mech; 693 668 694 669 /* ctx->targ_name */ 695 670 err = gssx_dec_name(xdr, &ctx->targ_name); 696 671 if (err) 697 - return err; 672 + goto out_free_src_name; 698 673 699 674 /* ctx->lifetime */ 700 675 p = xdr_inline_decode(xdr, 8+8); 701 - if (unlikely(p == NULL)) 702 - return -ENOSPC; 676 + if (unlikely(p == NULL)) { 677 + err = -ENOSPC; 678 + goto out_free_targ_name; 679 + } 703 680 p = xdr_decode_hyper(p, &ctx->lifetime); 704 681 705 682 /* ctx->ctx_flags */ ··· 710 683 /* ctx->locally_initiated */ 711 684 err = gssx_dec_bool(xdr, &ctx->locally_initiated); 712 685 if (err) 713 - return err; 686 + goto out_free_targ_name; 714 687 715 688 /* ctx->open */ 716 689 err = gssx_dec_bool(xdr, &ctx->open); 717 690 if (err) 718 - return err; 691 + goto out_free_targ_name; 719 692 720 693 /* we assume we have no options for now, so simply consume them */ 721 694 /* ctx->options */ 722 695 err = dummy_dec_opt_array(xdr, &ctx->options); 696 + if (err) 697 + goto out_free_targ_name; 723 698 699 + return 0; 700 + 701 + out_free_targ_name: 702 + kfree(ctx->targ_name.display_name.data); 703 + ctx->targ_name.display_name.data = NULL; 704 + out_free_src_name: 705 + kfree(ctx->src_name.display_name.data); 706 + ctx->src_name.display_name.data = NULL; 707 + out_free_mech: 708 + kfree(ctx->mech.data); 709 + ctx->mech.data = NULL; 710 + out_free_state: 711 + kfree(ctx->state.data); 712 + ctx->state.data = NULL; 713 + out_free_exported_context_token: 714 + kfree(ctx->exported_context_token.data); 715 + ctx->exported_context_token.data = NULL; 724 716 return err; 725 717 } 726 718

Configure Feed

Configure Feed