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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] apparmor_secctx_to_lsmprop and apparmor_secctx_to_secid leak aa_label reference counts.
--

commit 571cc7b76439569c095b0f5a60fbb3a11fc2aa6b
Author: Casey Schaufler <casey@schaufler-ca.com>

Apparmor: hooks for secctx_to_lsmprop and update_lsmprop
    
Implement the secctx_to_lsmprop and update_lsmprop hooks for AppArmor.

> 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 isn't a bug introduced by this patch, but apparmor_secctx_to_secid()
appears to leak the ref-counted aa_label object. When aa_label_strn_parse()
is called earlier in this function, it returns a label with an incremented
reference count. The secid is extracted, but aa_put_label() is never called
before returning, leaving the reference permanently leaked.

>  
> +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 leak the newly acquired label reference?

When security_secctx_to_lsmprop() is called (for example, by
nft_secmark_compute_secid() to create a new nftables SECMARK rule),
aa_label_strn_parse() parses the string and returns an aa_label pointer
with an incremented reference count.

The reference is placed into prop->apparmor.label here, but since the
generic LSM framework treats struct lsm_prop as a transient unmanaged
container and lacks a release_lsmprop hook, this reference is never
dropped. 

If the lsm_prop matches an existing entry in secxa_xa, secxa_from_lsmprop()
simply discards the duplicate prop, permanently leaking the newly acquired
reference. Since this is reachable by unprivileged userspaces configuring
SECMARK rules, could this be abused to cause memory exhaustion?

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