[PATCH] lsm: fix default return value of the socket_getpeersec_* hooks

[Paul Moore]

On Fri, Jan 26, 2024 at 1:45 PM Ondrej Mosnacek <omosnace at redhat.com> wrote:
>
> For these hooks the true "neutral" value is -EOPNOTSUPP, which is
> currently what is returned when no LSM provides this hook and what LSMs
> return when there is no security context set on the socket. Correct the
> value in <linux/lsm_hooks.h> and adjust the dispatch functions in
> security/security.c to avoid issues when the BPF LSM is enabled.
>
> Fixes: 98e828a0650f ("security: Refactor declaration of LSM hooks")
> Signed-off-by: Ondrej Mosnacek <omosnace at redhat.com>
> ---
>  include/linux/lsm_hook_defs.h |  4 ++--
>  security/security.c           | 31 +++++++++++++++++++++++++++----
>  2 files changed, 29 insertions(+), 6 deletions(-)
>
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 185924c56378..76458b6d53da 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -315,9 +315,9 @@ LSM_HOOK(int, 0, socket_getsockopt, struct socket *sock, int level, int optname)
>  LSM_HOOK(int, 0, socket_setsockopt, struct socket *sock, int level, int optname)
>  LSM_HOOK(int, 0, socket_shutdown, struct socket *sock, int how)
>  LSM_HOOK(int, 0, socket_sock_rcv_skb, struct sock *sk, struct sk_buff *skb)
> -LSM_HOOK(int, 0, socket_getpeersec_stream, struct socket *sock,
> +LSM_HOOK(int, -ENOPROTOOPT, socket_getpeersec_stream, struct socket *sock,
>          sockptr_t optval, sockptr_t optlen, unsigned int len)
> -LSM_HOOK(int, 0, socket_getpeersec_dgram, struct socket *sock,
> +LSM_HOOK(int, -ENOPROTOOPT, socket_getpeersec_dgram, struct socket *sock,
>          struct sk_buff *skb, u32 *secid)
>  LSM_HOOK(int, 0, sk_alloc_security, struct sock *sk, int family, gfp_t priority)
>  LSM_HOOK(void, LSM_RET_VOID, sk_free_security, struct sock *sk)
> diff --git a/security/security.c b/security/security.c
> index 6196ccaba433..3aaad75c9ce8 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -4624,8 +4624,20 @@ EXPORT_SYMBOL(security_sock_rcv_skb);
>  int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval,
>                                       sockptr_t optlen, unsigned int len)
>  {
> -       return call_int_hook(socket_getpeersec_stream, -ENOPROTOOPT, sock,
> -                            optval, optlen, len);
> +       struct security_hook_list *hp;
> +       int rc;
> +
> +       /*
> +        * Only one module will provide a security context.
> +        */
> +       hlist_for_each_entry(hp, &security_hook_heads.socket_getpeersec_stream,
> +                            list) {
> +               rc = hp->hook.socket_getpeersec_stream(sock, optval, optlen,
> +                                                      len);
> +               if (rc != LSM_RET_DEFAULT(socket_getpeersec_stream))
> +                       return rc;
> +       }
> +       return LSM_RET_DEFAULT(socket_getpeersec_stream);
>  }

I'm beginning to wonder if we shouldn't update call_int_hook() so that
it works for LSM_RET_DEFAULT() instead of assuming a zero/0 return
value.  Thoughts?

-- 
paul-moore.com