Re: [PATCH 3/7] LSM: Two hooks for manipulating struct lsm_prop

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] `security_secctx_to_lsmprop` silently returns success (0) when the requested LSM is not registered or no hook executes, leaving the `prop` out-parameter uninitialized.
- [Medium] `security_update_lsmprop` breaks the established dispatcher pattern for `lsmid`-targeted hooks by using `call_int_hook`, forcing the `lsmid` parameter into the hook signature and breaking the `LSM_ID_UNDEF` iteration case.
- [Low] Syntax error in the static inline stub for `security_secctx_to_lsmprop`, causing a build failure when `CONFIG_SECURITY` is disabled.
--

--- Patch [3]: [PATCH 3/7] LSM: Two hooks for manipulating struct lsm_prop ---
commit c0d20c4ec8715fc3470893661ae09affc4943cbc
Author: Casey Schaufler <casey@schaufler-ca.com>

LSM: Two hooks for manipulating struct lsm_prop

This commit introduces two new LSM hooks, security_update_lsmprop() and
security_secctx_to_lsmprop(). These hooks allow updating a destination
lsm_prop structure from a source, and converting a security context
string into an lsm_prop structure respectively.

> diff --git a/include/linux/security.h b/include/linux/security.h
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
[ ... ]
> @@ -1580,6 +1585,12 @@ static inline int security_lsmprop_to_secctx(struct lsm_prop *prop,
[ ... ]
> +static inline int security_secctx_to_lsmprop(const char *secdata, u32 seclen,
> +					     struct lsm_prop *prop, int lsmid);
> +{
> +	return -EOPNOTSUPP;
> +}

[Severity: Low]
Does the trailing semicolon on the function signature here cause a build
failure when CONFIG_SECURITY is disabled?

> diff --git a/security/security.c b/security/security.c
> --- a/security/security.c
> +++ b/security/security.c
[ ... ]
> @@ -3965,6 +3965,13 @@ int security_lsmprop_to_secctx(struct lsm_prop *prop, struct lsm_context *cp,
>  }
>  EXPORT_SYMBOL(security_lsmprop_to_secctx);
>  
> +int security_update_lsmprop(struct lsm_prop *dest, struct lsm_prop *src,
> +			    int lsmid)
> +{
> +	return call_int_hook(update_lsmprop, dest, src, lsmid);
> +}

[Severity: Medium]
Is call_int_hook() the correct macro to use here instead of
lsm_for_each_hook()?

Because call_int_hook() aborts on the first return value that does not match
the default, if LSM_ID_UNDEF is passed to update all active LSMs, will this
loop abort immediately after the first successful LSM hook returns 0? This
might fail to update any subsequent LSMs.

Additionally, using call_int_hook() forces lsmid to be passed into the hook
signature, requiring every LSM to write boilerplate ID checks, unlike
security_secctx_to_lsmprop() which filters centrally.

[ ... ]
> @@ -3982,6 +3989,31 @@ int security_secctx_to_secid(const char *secdata, u32 seclen, u32 *secid)
[ ... ]
> +int security_secctx_to_lsmprop(const char *secdata, u32 seclen,
> +			       struct lsm_prop *prop, int lsmid)
> +{
> +	struct lsm_static_call *scall;
> +
> +	lsm_for_each_hook(scall, secctx_to_lsmprop) {
> +		if (lsmid != LSM_ID_UNDEF && lsmid != scall->hl->lsmid->id)
> +			continue;
> +		return scall->hl->hook.secctx_to_lsmprop(secdata, seclen, prop);
> +	}
> +	return LSM_RET_DEFAULT(secctx_to_lsmprop);
> +}

[Severity: High]
If the requested LSM is not registered or no hook executes, this returns
LSM_RET_DEFAULT(secctx_to_lsmprop). In include/linux/lsm_hook_defs.h, the
default return value for this hook is set to 0.

Does this mean the function will silently return success without initializing
the prop out-parameter? If callers use this API for access control, could
they end up treating uninitialized stack memory as valid security identifiers?

Should this return an error code instead, or initialize prop before returning?

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