Re: [PATCH bpf-next v2 04/10] bpf: lsm: Add mutable hooks list for the BPF LSM

[Stephen Smalley]

On 1/15/20 12:13 PM, KP Singh wrote:
> From: KP Singh <kpsingh@xxxxxxxxxx>
>
> - The list of hooks registered by an LSM is currently immutable as they
>    are declared with __lsm_ro_after_init and they are attached to a
>    security_hook_heads struct.
> - For the BPF LSM we need to de/register the hooks at runtime. Making
>    the existing security_hook_heads mutable broadens an
>    attack vector, so a separate security_hook_heads is added for only
>    those that ~must~ be mutable.
> - These mutable hooks are run only after all the static hooks have
>    successfully executed.
>
> This is based on the ideas discussed in:
>
>    https://lore.kernel.org/lkml/20180408065916.GA2832@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx
>
> Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx>
> ---
[...]
> diff --git a/security/security.c b/security/security.c
> index cd2d18d2d279..4a2eb4c089b2 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -652,20 +653,21 @@ static void __init lsm_early_task(struct task_struct *task)
>   								\
>   		hlist_for_each_entry(P, &security_hook_heads.FUNC, list) \
>   			P->hook.FUNC(__VA_ARGS__);		\
> +		CALL_BPF_LSM_VOID_HOOKS(FUNC, __VA_ARGS__);	\
>   	} while (0)
>   
> -#define call_int_hook(FUNC, IRC, ...) ({			\
> -	int RC = IRC;						\
> -	do {							\
> -		struct security_hook_list *P;			\
> -								\
> +#define call_int_hook(FUNC, IRC, ...) ({				\
> +	int RC = IRC;							\
> +	do {								\
> +		struct security_hook_list *P;				\
>   		hlist_for_each_entry(P, &security_hook_heads.FUNC, list) { \
> -			RC = P->hook.FUNC(__VA_ARGS__);		\
> -			if (RC != 0)				\
> -				break;				\
> -		}						\
> -	} while (0);						\
> -	RC;							\
> +			RC = P->hook.FUNC(__VA_ARGS__);			\
> +			if (RC != 0)					\
> +				break;					\
> +		}							\
> +		RC = CALL_BPF_LSM_INT_HOOKS(RC, FUNC, __VA_ARGS__);	\

Let's not clobber the return code from the other LSMs with the bpf one.

> +	} while (0);							\
> +	RC;								\
>   })
>   
>   /* Security operations */