mirror of
https://github.com/torvalds/linux.git
synced 2026-09-22 12:44:03 +02:00
SUNRPC: fix gssx_dec_option_array error path bugs
Four coupled defects in the gssx XDR option-array decoder make the
error paths unsafe: a NULL deref in the caller, a refcount leak on
the decoded group_info, and a latent use-after-free that the leak
fix would otherwise expose.
gssx_dec_option_array() sets oa->count = 1 before allocating
oa->data. If that allocation fails, -ENOMEM is returned with
oa->count == 1 and oa->data == NULL. All other error paths jump
to free_oa: which frees oa->data and NULLs it but also leaves
oa->count == 1. The caller trusts the count:
gssp_accept_sec_context_upcall()
gssx_dec_accept_sec_context()
gssx_dec_option_array() /* fails, count=1 data=NULL */
data = res.options.data[0].value /* NULL deref */
Independently, free_creds: releases the partially decoded svc_cred
with a bare kfree(creds). gssx_dec_linux_creds() installs a
groups_alloc() result into creds->cr_group_info; that object is
kvmalloc-backed and refcounted, and only put_group_info() reaches
kvfree(). A plain kfree(creds) drops the wrapper and leaks the
group_info allocation.
The natural fix for the leak is to call free_svc_cred(creds) before
kfree(creds), but free_svc_cred() invokes put_group_info() on
creds->cr_group_info unconditionally when non-NULL. The existing
out_free_groups: path in gssx_dec_linux_creds() already called
groups_free() on that pointer without clearing it, so once
free_svc_cred() is wired in, the subsequent put_group_info() would
touch freed memory.
Fix all four together:
- Move the oa->count = 1 assignment below the oa->data allocation
so it is never set when oa->data is NULL.
- Reset oa->count to 0 at free_oa: so count and data stay
coherent and the caller sees an empty option array.
- Call free_svc_cred(creds) before kfree(creds) at free_creds:
so the refcounted cr_group_info is released. free_svc_cred()
either NULL-guards each field explicitly (cr_group_info has
an if() check) or delegates to a helper that is NULL-safe
itself (kfree for the string fields, gss_mech_put() which
guards with if(gm) at gss_mech_switch.c:342), so it is safe
to call on a partially decoded svc_cred where only
cr_uid/cr_gid/cr_group_info have been written and everything
else is zero from kzalloc.
- In gssx_dec_linux_creds()'s out_free_groups: path, release
cr_group_info with put_group_info() rather than groups_free()
so the teardown matches free_svc_cred()'s refcount-aware path,
and clear the pointer so a later free_svc_cred() on the same
creds does not release it a second time.
Fixes: 3cfcfc102a ("SUNRPC: fix some memleaks in gssx_dec_option_array")
Cc: stable@vger.kernel.org
Assisted-by: kres (claude-opus-4-7)
Signed-off-by: Chris Mason <clm@meta.com>
Reviewed-by: Jeff Layton <jlayton@kernel.org>
Link: https://patch.msgid.link/20260528-tier2-v1-2-d026a1415e0b@oracle.com
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
This commit is contained in:
parent
ad484748ee
commit
5e9a94539b
|
|
@ -222,7 +222,8 @@ static int gssx_dec_linux_creds(struct xdr_stream *xdr,
|
|||
|
||||
return 0;
|
||||
out_free_groups:
|
||||
groups_free(creds->cr_group_info);
|
||||
put_group_info(creds->cr_group_info);
|
||||
creds->cr_group_info = NULL;
|
||||
return err;
|
||||
}
|
||||
|
||||
|
|
@ -242,12 +243,12 @@ static int gssx_dec_option_array(struct xdr_stream *xdr,
|
|||
return 0;
|
||||
|
||||
/* we recognize only 1 currently: CREDS_VALUE */
|
||||
oa->count = 1;
|
||||
|
||||
oa->data = kmalloc_obj(struct gssx_option);
|
||||
if (!oa->data)
|
||||
return -ENOMEM;
|
||||
|
||||
oa->count = 1;
|
||||
|
||||
creds = kzalloc_obj(struct svc_cred);
|
||||
if (!creds) {
|
||||
err = -ENOMEM;
|
||||
|
|
@ -294,8 +295,10 @@ static int gssx_dec_option_array(struct xdr_stream *xdr,
|
|||
return 0;
|
||||
|
||||
free_creds:
|
||||
free_svc_cred(creds);
|
||||
kfree(creds);
|
||||
free_oa:
|
||||
oa->count = 0;
|
||||
kfree(oa->data);
|
||||
oa->data = NULL;
|
||||
return err;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user