[PATCH] security: fix the logic in security_inode_getsecctx()

[Casey Schaufler]

On 1/26/2024 2:44 AM, Ondrej Mosnacek wrote:
> The inode_getsecctx LSM hook has previously been corrected to have
> -EOPNOTSUPP instead of 0 as the default return value to fix BPF LSM
> behavior. However, the call_int_hook()-generated loop in
> security_inode_getsecctx() was left treating 0 as the neutral value, so
> after an LSM returns 0, the loop continues to try other LSMs, and if one
> of them returns a non-zero value, the function immediately returns with
> said value. So in a situation where SELinux and the BPF LSMs registered
> this hook, -EOPNOTSUPP would be incorrectly returned whenever SELinux
> returned 0.
>
> Fix this by open-coding the call_int_hook() loop and making it use the
> correct LSM_RET_DEFAULT() value as the neutral one, similar to what
> other hooks do.
>
> Reported-by: Stephen Smalley <stephen.smalley.work at gmail.com>
> Link: https://lore.kernel.org/selinux/CAEjxPJ4ev-pasUwGx48fDhnmjBnq_Wh90jYPwRQRAqXxmOKD4Q@mail.gmail.com/
> Fixes: b36995b8609a ("lsm: fix default return value for inode_getsecctx")
> Signed-off-by: Ondrej Mosnacek <omosnace at redhat.com>

Reviewed-by: Casey Schaufler <casey at schaufler-ca.com>

> ---
>
> I ran 'tools/nfs.sh' on the patch and even though it fixes the most
> serious issue that Stephen reported, some of the tests are still
> failing under NFS (but I will presume that these are pre-existing issues
> not caused by the patch).
>
> I can also see an opportunity to clean up the hook implementations in
> security/security.c - I plan to have a go at it and send it as a
> separate patch later.
>
>  security/security.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/security/security.c b/security/security.c
> index 0144a98d3712..6196ccaba433 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4255,7 +4255,19 @@ EXPORT_SYMBOL(security_inode_setsecctx);
>   */
>  int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
>  {
> -	return call_int_hook(inode_getsecctx, -EOPNOTSUPP, inode, ctx, ctxlen);
> +	struct security_hook_list *hp;
> +	int rc;
> +
> +	/*
> +	 * Only one module will provide a security context.
> +	 */
> +	hlist_for_each_entry(hp, &security_hook_heads.inode_getsecctx, list) {
> +		rc = hp->hook.inode_getsecctx(inode, ctx, ctxlen);
> +		if (rc != LSM_RET_DEFAULT(inode_getsecctx))
> +			return rc;
> +	}
> +
> +	return LSM_RET_DEFAULT(inode_getsecctx);
>  }
>  EXPORT_SYMBOL(security_inode_getsecctx);
>