[PATCH] security: use default hook return value in call_int_hook()

[Casey Schaufler]

On 1/30/2024 4:56 AM, Ondrej Mosnacek wrote:
> Change the definition of call_int_hook() to treat LSM_RET_DEFAULT(...)
> as the "continue" value instead of 0. To further simplify this macro,
> also drop the IRC argument and replace it with LSM_RET_DEFAULT(...).
>
> After this the macro can be used in a couple more hooks, where similar
> logic is currently open-coded. At the same time, some other existing
> call_int_hook() users now need to be open-coded, but overall it's still
> a net simplification.
>
> There should be no functional change resulting from this patch.
>
> Signed-off-by: Ondrej Mosnacek <omosnace at redhat.com>

Minor comment below, otherwise

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

> ---
>  security/security.c | 525 +++++++++++++++++++-------------------------
>  1 file changed, 221 insertions(+), 304 deletions(-)
>
> diff --git a/security/security.c b/security/security.c
> index cedd6c150bdd..11012dcfd68e 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -855,14 +855,14 @@ out:
>  			P->hook.FUNC(__VA_ARGS__);		\
>  	} while (0)
>  
> -#define call_int_hook(FUNC, IRC, ...) ({			\
> -	int RC = IRC;						\
> +#define call_int_hook(FUNC, ...) ({				\
> +	int RC = LSM_RET_DEFAULT(FUNC);				\
>  	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)				\
> +			if (RC != LSM_RET_DEFAULT(FUNC))	\
>  				break;				\
>  		}						\
>  	} while (0);						\
> @@ -881,7 +881,7 @@ out:
>   */
>  ...
> @@ -2629,21 +2603,15 @@ EXPORT_SYMBOL(security_inode_copy_up);
>   */
>  int security_inode_copy_up_xattr(const char *name)
>  {
> -	struct security_hook_list *hp;
> -	int rc;
> -
>  	/*
>  	 * The implementation can return 0 (accept the xattr), 1 (discard the
>  	 * xattr), -EOPNOTSUPP if it does not know anything about the xattr or
>  	 * any other error code in case of an error.
>  	 */
> -	hlist_for_each_entry(hp,
> -			     &security_hook_heads.inode_copy_up_xattr, list) {
> -		rc = hp->hook.inode_copy_up_xattr(name);
> -		if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> -			return rc;
> -	}
> +	int rc = call_int_hook(inode_copy_up_xattr, name);

I think this is the only place call_int_hook() is used in a
declaration. Maybe break it up into:

	int rc;

	rc = call_int_hook(inode_copy_up_xattr, name);

Not a big deal, I won't fuss over it, but it might make the next mass
overhaul a touch simpler. Even more trivial: use ret instead of rc.

>  
> +	if (rc != LSM_RET_DEFAULT(inode_copy_up_xattr))
> +		return rc;
>  	return evm_inode_copy_up_xattr(name);
>  }
>  EXPORT_SYMBOL(security_inode_copy_up_xattr);
> @@ -2661,7 +2629,7 @@ EXPORT_SYMBOL(security_inode_copy_up_xattr);
>