| Message ID | 20240126184531.1167999-1-omosnace@redhat.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Delegated to: | Paul Moore |
| Headers | show |
| Series | lsm: fix default return value of the socket_getpeersec_* hooks | expand |
On Fri, Jan 26, 2024 at 1:45 PM Ondrej Mosnacek <omosnace@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@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?
On 1/29/2024 3:02 PM, Paul Moore wrote: > On Fri, Jan 26, 2024 at 1:45 PM Ondrej Mosnacek <omosnace@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@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? call_int_hook() was intended to address the "normal" case, where all hooks registered would be called and the first error, if any, would result in an immediate failure return. Hooks that behaved in any other manner were expected to be open coded. The point of using the macros was to reduce so much code duplication. I really don't want to see call_int_hook() evolve into something hard to work with, or that has non-obvious side effects. I think we could probably integrate LSM_RET_DEFAULT() safely, but I'm wary of hiding these abnormal cases in the macro.
On Mon, Jan 29, 2024 at 6:25 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > On 1/29/2024 3:02 PM, Paul Moore wrote: > > On Fri, Jan 26, 2024 at 1:45 PM Ondrej Mosnacek <omosnace@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@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? > > call_int_hook() was intended to address the "normal" case, where all > hooks registered would be called and the first error, if any, would > result in an immediate failure return. Hooks that behaved in any other > manner were expected to be open coded. The point of using the macros > was to reduce so much code duplication. I really don't want to see > call_int_hook() evolve into something hard to work with, or that has > non-obvious side effects. I think we could probably integrate > LSM_RET_DEFAULT() safely, but I'm wary of hiding these abnormal cases > in the macro. Yes, I'm not talking about modifying call_int_hook() to handle something like security_vm_enough_memory_mm(), I'm just talking about updating it use LSM_RET_DEFAULT() instead of zero. While we are at it, we should probably get rid of the second parameter too, @IRC, and just use the assigned LSM_RET_DEFAULT(). That always struck me as a bug waiting to happen if/when those two fell out of sync. -- paul-moore.com
On Tue, Jan 30, 2024 at 4:01 AM Paul Moore <paul@paul-moore.com> wrote: > > On Mon, Jan 29, 2024 at 6:25 PM Casey Schaufler <casey@schaufler-ca.com> wrote: > > > > On 1/29/2024 3:02 PM, Paul Moore wrote: > > > On Fri, Jan 26, 2024 at 1:45 PM Ondrej Mosnacek <omosnace@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@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? > > > > call_int_hook() was intended to address the "normal" case, where all > > hooks registered would be called and the first error, if any, would > > result in an immediate failure return. Hooks that behaved in any other > > manner were expected to be open coded. The point of using the macros > > was to reduce so much code duplication. I really don't want to see > > call_int_hook() evolve into something hard to work with, or that has > > non-obvious side effects. I think we could probably integrate > > LSM_RET_DEFAULT() safely, but I'm wary of hiding these abnormal cases > > in the macro. > > Yes, I'm not talking about modifying call_int_hook() to handle > something like security_vm_enough_memory_mm(), I'm just talking about > updating it use LSM_RET_DEFAULT() instead of zero. > > While we are at it, we should probably get rid of the second parameter > too, @IRC, and just use the assigned LSM_RET_DEFAULT(). That always > struck me as a bug waiting to happen if/when those two fell out of > sync. You're reading my mind :) I already started writing a patch that does exactly that after I posted the security_inode_getsecctx() patch. While working on it I gradually found two more pre-existing issues, so I wanted to post fixes for them before the refactor for better backportability. I should be able to post the patch today. BTW, the IRC param removal means that a few of the existing call_int_hook() calls have to be open-coded, but even then the patch removes more LoC than it adds, so I think it's worth it.
On 1/30/2024 12:29 AM, Ondrej Mosnacek wrote: > On Tue, Jan 30, 2024 at 4:01 AM Paul Moore <paul@paul-moore.com> wrote: >> On Mon, Jan 29, 2024 at 6:25 PM Casey Schaufler <casey@schaufler-ca.com> wrote: >>> On 1/29/2024 3:02 PM, Paul Moore wrote: >>>> On Fri, Jan 26, 2024 at 1:45 PM Ondrej Mosnacek <omosnace@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@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? >>> call_int_hook() was intended to address the "normal" case, where all >>> hooks registered would be called and the first error, if any, would >>> result in an immediate failure return. Hooks that behaved in any other >>> manner were expected to be open coded. The point of using the macros >>> was to reduce so much code duplication. I really don't want to see >>> call_int_hook() evolve into something hard to work with, or that has >>> non-obvious side effects. I think we could probably integrate >>> LSM_RET_DEFAULT() safely, but I'm wary of hiding these abnormal cases >>> in the macro. >> Yes, I'm not talking about modifying call_int_hook() to handle >> something like security_vm_enough_memory_mm(), I'm just talking about >> updating it use LSM_RET_DEFAULT() instead of zero. >> >> While we are at it, we should probably get rid of the second parameter >> too, @IRC, and just use the assigned LSM_RET_DEFAULT(). That always >> struck me as a bug waiting to happen if/when those two fell out of >> sync. > You're reading my mind :) I already started writing a patch that does > exactly that after I posted the security_inode_getsecctx() patch. > While working on it I gradually found two more pre-existing issues, so > I wanted to post fixes for them before the refactor for better > backportability. I should be able to post the patch today. > > BTW, the IRC param removal means that a few of the existing > call_int_hook() calls have to be open-coded, but even then the patch > removes more LoC than it adds, so I think it's worth it. OK, I'm good with it. Getting rid of @IRC is the clincher.
On Jan 26, 2024 Ondrej Mosnacek <omosnace@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@redhat.com> > --- > include/linux/lsm_hook_defs.h | 4 ++-- > security/security.c | 31 +++++++++++++++++++++++++++---- > 2 files changed, 29 insertions(+), 6 deletions(-) I was originally going to merge this via lsm/dev, but thinking about this some more today, and considering the other inode_getsecctx() fix, I think this patch should be marked for stable too. I'm going to merge this into lsm/stable-6.8 and assuming all the tests come back clean (which they should), I'll send this up to Linus tomorrow with the inode_getsecctx() fix. Thanks all! -- paul-moore.com
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); } /** @@ -4645,8 +4657,19 @@ int security_socket_getpeersec_stream(struct socket *sock, sockptr_t optval, int security_socket_getpeersec_dgram(struct socket *sock, struct sk_buff *skb, u32 *secid) { - return call_int_hook(socket_getpeersec_dgram, -ENOPROTOOPT, sock, - skb, secid); + 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_dgram, + list) { + rc = hp->hook.socket_getpeersec_dgram(sock, skb, secid); + if (rc != LSM_RET_DEFAULT(socket_getpeersec_dgram)) + return rc; + } + return LSM_RET_DEFAULT(socket_getpeersec_dgram); } EXPORT_SYMBOL(security_socket_getpeersec_dgram);
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@redhat.com> --- include/linux/lsm_hook_defs.h | 4 ++-- security/security.c | 31 +++++++++++++++++++++++++++---- 2 files changed, 29 insertions(+), 6 deletions(-)