Re: [PATCH v2 6/7] Apparmor: hooks for secctx_to_lsmprop and update_lsmprop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] Memory leak of `aa_label` reference in `apparmor_secctx_to_lsmprop()`.

Pre-existing issues:
- [High] Memory leak of `aa_label` reference in `apparmor_secctx_to_secid()`.
--

commit 1e8503b950ea8fae325db00bc26a72d7c257c11e
Author: Casey Schaufler <casey@schaufler-ca.com>

Apparmor: hooks for secctx_to_lsmprop and update_lsmprop

Implement these hooks.

> diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
> index 28caf66b90335..d35fdbf074e05 100644
> --- a/security/apparmor/secid.c
> +++ b/security/apparmor/secid.c
> @@ -106,6 +106,27 @@ int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
>  	return 0;
>  }

[Severity: High]
This is a pre-existing issue, but does this function leak the reference
to the aa_label?

Looking at the body of the function:

int apparmor_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
{
	struct aa_label *label;

	label = aa_label_strn_parse(&root_ns->unconfined->label, secdata,
				    seclen, GFP_KERNEL, false, false);
	if (IS_ERR(label))
		return PTR_ERR(label);
	*secid = label->secid;

	return 0;
}

It looks like aa_label_strn_parse() returns an aa_label with an
incremented reference count, and apparmor_secctx_to_secid() extracts
label->secid and returns without dropping the reference.

This can be triggered by unprivileged users in a user and network namespace
by repeatedly adding secmark rules (via xt_SECMARK) with an existing
profile name. This allows an unprivileged user to leak references until
refcount_t saturation, triggering kernel warnings and potential crashes
under panic_on_warn.

>  
> +int apparmor_secctx_to_lsmprop(const char *secdata, u32 seclen,
> +			       struct lsm_prop *prop)
> +{
> +	struct aa_label *label;
> +
> +	label = aa_label_strn_parse(&root_ns->unconfined->label, secdata,
> +				    seclen, GFP_KERNEL, false, false);
> +	if (IS_ERR(label))
> +		return PTR_ERR(label);
> +	prop->apparmor.label = label;
> +
> +	return 0;
> +}

[Severity: High]
Does this code leak the aa_label reference?

Similar to apparmor_secctx_to_secid(), aa_label_strn_parse() returns an
aa_label with an incremented reference count. The label pointer is assigned
to prop->apparmor.label but returns without dropping the reference. Because
the lsm_prop struct acts as a simple value container and lacks any release
lifecycle hook across the LSM framework, this reference is permanently leaked.

An unprivileged user can trigger this whenever security_secctx_to_lsmprop()
is called, such as by nft_secmark_compute_secid() when adding a netfilter
secmark rule. By gaining CAP_NET_ADMIN in a user and network namespace, they
can repeatedly add netfilter rules and leak references to an existing AppArmor
profile until the refcount_t saturates.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902220150.18586-1-casey@schaufler-ca.com?part=6