[PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Mikhail Ivanov]

selinux_socket_bind() is called without holding the socket lock.

Use READ_ONCE() to safely read sk->sk_family for IPv6 socket in case
of lockless transformation to IPv4 socket via IPV6_ADDRFORM [1].

[1] https://lore.kernel.org/all/20240202095404.183274-1-edumazet@google.com/

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
---
 security/selinux/hooks.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 5e5f3398f39d..b7adff2cf5f6 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4715,8 +4715,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
 	if (err)
 		goto out;
 
+	/* IPV6_ADDRFORM can change sk->sk_family under us. */
+	family = READ_ONCE(sk->sk_family);
+
 	/* If PF_INET or PF_INET6, check name_bind permission for the port. */
-	family = sk->sk_family;
 	if (family == PF_INET || family == PF_INET6) {
 		char *addrp;
 		struct common_audit_data ad;

base-commit: 034294fbfdf0ded4f931f9503d2ca5bbf8b9aebd
-- 
2.34.1

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Mickaël Salaün]

This looks good be there are other places using sk->sk_family that
should also be fixed.

On Thu, Dec 12, 2024 at 06:20:00PM +0800, Mikhail Ivanov wrote:
> selinux_socket_bind() is called without holding the socket lock.
> 
> Use READ_ONCE() to safely read sk->sk_family for IPv6 socket in case
> of lockless transformation to IPv4 socket via IPV6_ADDRFORM [1].
> 
> [1] https://lore.kernel.org/all/20240202095404.183274-1-edumazet@google.com/
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> ---
>  security/selinux/hooks.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 5e5f3398f39d..b7adff2cf5f6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4715,8 +4715,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>  	if (err)
>  		goto out;
>  
> +	/* IPV6_ADDRFORM can change sk->sk_family under us. */
> +	family = READ_ONCE(sk->sk_family);
> +
>  	/* If PF_INET or PF_INET6, check name_bind permission for the port. */
> -	family = sk->sk_family;
>  	if (family == PF_INET || family == PF_INET6) {
>  		char *addrp;
>  		struct common_audit_data ad;
> 
> base-commit: 034294fbfdf0ded4f931f9503d2ca5bbf8b9aebd
> -- 
> 2.34.1
> 
> 

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Mikhail Ivanov]

On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
> This looks good be there are other places using sk->sk_family that
> should also be fixed.

Thanks for checking this!

For selinux this should be enough, I haven't found any other places
where sk->sk_family could be read from an IPv6 socket without locking.

I also would like to prepare such fix for other LSMs (apparmor, smack,
tomoyo) (in separate patches).

> 
> On Thu, Dec 12, 2024 at 06:20:00PM +0800, Mikhail Ivanov wrote:
>> selinux_socket_bind() is called without holding the socket lock.
>>
>> Use READ_ONCE() to safely read sk->sk_family for IPv6 socket in case
>> of lockless transformation to IPv4 socket via IPV6_ADDRFORM [1].
>>
>> [1] https://lore.kernel.org/all/20240202095404.183274-1-edumazet@google.com/
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
>> ---
>>   security/selinux/hooks.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>> index 5e5f3398f39d..b7adff2cf5f6 100644
>> --- a/security/selinux/hooks.c
>> +++ b/security/selinux/hooks.c
>> @@ -4715,8 +4715,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>   	if (err)
>>   		goto out;
>>   
>> +	/* IPV6_ADDRFORM can change sk->sk_family under us. */
>> +	family = READ_ONCE(sk->sk_family);
>> +
>>   	/* If PF_INET or PF_INET6, check name_bind permission for the port. */
>> -	family = sk->sk_family;
>>   	if (family == PF_INET || family == PF_INET6) {
>>   		char *addrp;
>>   		struct common_audit_data ad;
>>
>> base-commit: 034294fbfdf0ded4f931f9503d2ca5bbf8b9aebd
>> -- 
>> 2.34.1
>>
>>

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Stephen Smalley]

On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov
<ivanov.mikhail1@huawei-partners.com> wrote:
>
> On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
> > This looks good be there are other places using sk->sk_family that
> > should also be fixed.
>
> Thanks for checking this!
>
> For selinux this should be enough, I haven't found any other places
> where sk->sk_family could be read from an IPv6 socket without locking.
>
> I also would like to prepare such fix for other LSMs (apparmor, smack,
> tomoyo) (in separate patches).

I'm wondering about the implications for SELinux beyond just
sk->sk_family access, e.g. SELinux maps the (family, type, protocol)
triple to a security class at socket creation time via
socket_type_to_security_class() and caches the security class in the
inode_security_struct and sk_security_struct for later use.

>
> >
> > On Thu, Dec 12, 2024 at 06:20:00PM +0800, Mikhail Ivanov wrote:
> >> selinux_socket_bind() is called without holding the socket lock.
> >>
> >> Use READ_ONCE() to safely read sk->sk_family for IPv6 socket in case
> >> of lockless transformation to IPv4 socket via IPV6_ADDRFORM [1].
> >>
> >> [1] https://lore.kernel.org/all/20240202095404.183274-1-edumazet@google.com/
> >>
> >> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> >> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
> >> ---
> >>   security/selinux/hooks.c | 4 +++-
> >>   1 file changed, 3 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> >> index 5e5f3398f39d..b7adff2cf5f6 100644
> >> --- a/security/selinux/hooks.c
> >> +++ b/security/selinux/hooks.c
> >> @@ -4715,8 +4715,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
> >>      if (err)
> >>              goto out;
> >>
> >> +    /* IPV6_ADDRFORM can change sk->sk_family under us. */
> >> +    family = READ_ONCE(sk->sk_family);
> >> +
> >>      /* If PF_INET or PF_INET6, check name_bind permission for the port. */
> >> -    family = sk->sk_family;
> >>      if (family == PF_INET || family == PF_INET6) {
> >>              char *addrp;
> >>              struct common_audit_data ad;
> >>
> >> base-commit: 034294fbfdf0ded4f931f9503d2ca5bbf8b9aebd
> >> --
> >> 2.34.1
> >>
> >>

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Mikhail Ivanov]

On 12/13/2024 6:46 PM, Stephen Smalley wrote:
> On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov
> <ivanov.mikhail1@huawei-partners.com> wrote:
>>
>> On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
>>> This looks good be there are other places using sk->sk_family that
>>> should also be fixed.
>>
>> Thanks for checking this!
>>
>> For selinux this should be enough, I haven't found any other places
>> where sk->sk_family could be read from an IPv6 socket without locking.
>>
>> I also would like to prepare such fix for other LSMs (apparmor, smack,
>> tomoyo) (in separate patches).
> 
> I'm wondering about the implications for SELinux beyond just
> sk->sk_family access, e.g. SELinux maps the (family, type, protocol)
> triple to a security class at socket creation time via
> socket_type_to_security_class() and caches the security class in the
> inode_security_struct and sk_security_struct for later use.

IPv6 and IPv4 TCP sockets are mapped to the same SECCLASS_TCP_SOCKET
security class. AFAICS there is no other places that can be affected by
the IPV6_ADDFORM transformation.

> 
>>
>>>
>>> On Thu, Dec 12, 2024 at 06:20:00PM +0800, Mikhail Ivanov wrote:
>>>> selinux_socket_bind() is called without holding the socket lock.
>>>>
>>>> Use READ_ONCE() to safely read sk->sk_family for IPv6 socket in case
>>>> of lockless transformation to IPv4 socket via IPV6_ADDRFORM [1].
>>>>
>>>> [1] https://lore.kernel.org/all/20240202095404.183274-1-edumazet@google.com/
>>>>
>>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>>> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>
>>>> ---
>>>>    security/selinux/hooks.c | 4 +++-
>>>>    1 file changed, 3 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
>>>> index 5e5f3398f39d..b7adff2cf5f6 100644
>>>> --- a/security/selinux/hooks.c
>>>> +++ b/security/selinux/hooks.c
>>>> @@ -4715,8 +4715,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>>>>       if (err)
>>>>               goto out;
>>>>
>>>> +    /* IPV6_ADDRFORM can change sk->sk_family under us. */
>>>> +    family = READ_ONCE(sk->sk_family);
>>>> +
>>>>       /* If PF_INET or PF_INET6, check name_bind permission for the port. */
>>>> -    family = sk->sk_family;
>>>>       if (family == PF_INET || family == PF_INET6) {
>>>>               char *addrp;
>>>>               struct common_audit_data ad;
>>>>
>>>> base-commit: 034294fbfdf0ded4f931f9503d2ca5bbf8b9aebd
>>>> --
>>>> 2.34.1
>>>>
>>>>

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Stephen Smalley]

On Fri, Dec 13, 2024 at 11:40 AM Mikhail Ivanov
<ivanov.mikhail1@huawei-partners.com> wrote:
>
> On 12/13/2024 6:46 PM, Stephen Smalley wrote:
> > On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov
> > <ivanov.mikhail1@huawei-partners.com> wrote:
> >>
> >> On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
> >>> This looks good be there are other places using sk->sk_family that
> >>> should also be fixed.
> >>
> >> Thanks for checking this!
> >>
> >> For selinux this should be enough, I haven't found any other places
> >> where sk->sk_family could be read from an IPv6 socket without locking.
> >>
> >> I also would like to prepare such fix for other LSMs (apparmor, smack,
> >> tomoyo) (in separate patches).
> >
> > I'm wondering about the implications for SELinux beyond just
> > sk->sk_family access, e.g. SELinux maps the (family, type, protocol)
> > triple to a security class at socket creation time via
> > socket_type_to_security_class() and caches the security class in the
> > inode_security_struct and sk_security_struct for later use.
>
> IPv6 and IPv4 TCP sockets are mapped to the same SECCLASS_TCP_SOCKET
> security class. AFAICS there is no other places that can be affected by
> the IPV6_ADDFORM transformation.

Great, thank you for checking!

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Paul Moore]

On Fri, Dec 13, 2024 at 11:40 AM Mikhail Ivanov
<ivanov.mikhail1@huawei-partners.com> wrote:
> On 12/13/2024 6:46 PM, Stephen Smalley wrote:
> > On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov
> > <ivanov.mikhail1@huawei-partners.com> wrote:
> >>
> >> On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
> >>> This looks good be there are other places using sk->sk_family that
> >>> should also be fixed.
> >>
> >> Thanks for checking this!
> >>
> >> For selinux this should be enough, I haven't found any other places
> >> where sk->sk_family could be read from an IPv6 socket without locking.
> >>
> >> I also would like to prepare such fix for other LSMs (apparmor, smack,
> >> tomoyo) (in separate patches).
> >
> > I'm wondering about the implications for SELinux beyond just
> > sk->sk_family access, e.g. SELinux maps the (family, type, protocol)
> > triple to a security class at socket creation time via
> > socket_type_to_security_class() and caches the security class in the
> > inode_security_struct and sk_security_struct for later use.
>
> IPv6 and IPv4 TCP sockets are mapped to the same SECCLASS_TCP_SOCKET
> security class. AFAICS there is no other places that can be affected by
> the IPV6_ADDFORM transformation.

Yes, thankfully we don't really encode the IP address family in any of
the SELinux object classes so that shouldn't be an issue.  I also
don't think we have to worry about the per-packet labeling protocols
as it's too late in the communication to change the socket's
associated packet labeling, it's either working or it isn't; we should
handle the mapped IPv4 address already.

I am a little concerned about bind being the only place where we have
to worry about accessing sk_family while the socket isn't locked.  As
an example, I'm a little concerned about the netfilter code paths; I
haven't chased them down, but my guess is that the associated
socket/sock isn't locked in those cases (in the relevant output and
postroute cases, forward should be a non-issue).

How bad is the performance impact of READ_ONCE()?  In other words, how
stupid would it be to simply do all of our sock->sk_family lookups
using READ_ONCE()?

-- 
paul-moore.com

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Stephen Smalley]

On Thu, Dec 12, 2024 at 5:20 AM Mikhail Ivanov
<ivanov.mikhail1@huawei-partners.com> wrote:
>
> selinux_socket_bind() is called without holding the socket lock.
>
> Use READ_ONCE() to safely read sk->sk_family for IPv6 socket in case
> of lockless transformation to IPv4 socket via IPV6_ADDRFORM [1].
>
> [1] https://lore.kernel.org/all/20240202095404.183274-1-edumazet@google.com/
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Mikhail Ivanov <ivanov.mikhail1@huawei-partners.com>

Acked-by: Stephen Smalley <stephen.smalley.work@gmail.com>

> ---
>  security/selinux/hooks.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 5e5f3398f39d..b7adff2cf5f6 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -4715,8 +4715,10 @@ static int selinux_socket_bind(struct socket *sock, struct sockaddr *address, in
>         if (err)
>                 goto out;
>
> +       /* IPV6_ADDRFORM can change sk->sk_family under us. */
> +       family = READ_ONCE(sk->sk_family);
> +
>         /* If PF_INET or PF_INET6, check name_bind permission for the port. */
> -       family = sk->sk_family;
>         if (family == PF_INET || family == PF_INET6) {
>                 char *addrp;
>                 struct common_audit_data ad;
>
> base-commit: 034294fbfdf0ded4f931f9503d2ca5bbf8b9aebd
> --
> 2.34.1
>

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Stephen Smalley]

On Fri, Dec 13, 2024 at 3:09 PM Paul Moore <paul@paul-moore.com> wrote:
>
> On Fri, Dec 13, 2024 at 11:40 AM Mikhail Ivanov
> <ivanov.mikhail1@huawei-partners.com> wrote:
> > On 12/13/2024 6:46 PM, Stephen Smalley wrote:
> > > On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov
> > > <ivanov.mikhail1@huawei-partners.com> wrote:
> > >>
> > >> On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
> > >>> This looks good be there are other places using sk->sk_family that
> > >>> should also be fixed.
> > >>
> > >> Thanks for checking this!
> > >>
> > >> For selinux this should be enough, I haven't found any other places
> > >> where sk->sk_family could be read from an IPv6 socket without locking.
> > >>
> > >> I also would like to prepare such fix for other LSMs (apparmor, smack,
> > >> tomoyo) (in separate patches).
> > >
> > > I'm wondering about the implications for SELinux beyond just
> > > sk->sk_family access, e.g. SELinux maps the (family, type, protocol)
> > > triple to a security class at socket creation time via
> > > socket_type_to_security_class() and caches the security class in the
> > > inode_security_struct and sk_security_struct for later use.
> >
> > IPv6 and IPv4 TCP sockets are mapped to the same SECCLASS_TCP_SOCKET
> > security class. AFAICS there is no other places that can be affected by
> > the IPV6_ADDFORM transformation.
>
> Yes, thankfully we don't really encode the IP address family in any of
> the SELinux object classes so that shouldn't be an issue.  I also
> don't think we have to worry about the per-packet labeling protocols
> as it's too late in the communication to change the socket's
> associated packet labeling, it's either working or it isn't; we should
> handle the mapped IPv4 address already.
>
> I am a little concerned about bind being the only place where we have
> to worry about accessing sk_family while the socket isn't locked.  As
> an example, I'm a little concerned about the netfilter code paths; I
> haven't chased them down, but my guess is that the associated
> socket/sock isn't locked in those cases (in the relevant output and
> postroute cases, forward should be a non-issue).
>
> How bad is the performance impact of READ_ONCE()?  In other words, how
> stupid would it be to simply do all of our sock->sk_family lookups
> using READ_ONCE()?

I could be wrong, but I don't think there is any overhead except on Dec Alpha.

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Paul Moore]

On Tue, Jan 7, 2025 at 3:16 PM Stephen Smalley
<stephen.smalley.work@gmail.com> wrote:
> On Fri, Dec 13, 2024 at 3:09 PM Paul Moore <paul@paul-moore.com> wrote:
> > On Fri, Dec 13, 2024 at 11:40 AM Mikhail Ivanov
> > <ivanov.mikhail1@huawei-partners.com> wrote:
> > > On 12/13/2024 6:46 PM, Stephen Smalley wrote:
> > > > On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov
> > > > <ivanov.mikhail1@huawei-partners.com> wrote:
> > > >>
> > > >> On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
> > > >>> This looks good be there are other places using sk->sk_family that
> > > >>> should also be fixed.
> > > >>
> > > >> Thanks for checking this!
> > > >>
> > > >> For selinux this should be enough, I haven't found any other places
> > > >> where sk->sk_family could be read from an IPv6 socket without locking.
> > > >>
> > > >> I also would like to prepare such fix for other LSMs (apparmor, smack,
> > > >> tomoyo) (in separate patches).
> > > >
> > > > I'm wondering about the implications for SELinux beyond just
> > > > sk->sk_family access, e.g. SELinux maps the (family, type, protocol)
> > > > triple to a security class at socket creation time via
> > > > socket_type_to_security_class() and caches the security class in the
> > > > inode_security_struct and sk_security_struct for later use.
> > >
> > > IPv6 and IPv4 TCP sockets are mapped to the same SECCLASS_TCP_SOCKET
> > > security class. AFAICS there is no other places that can be affected by
> > > the IPV6_ADDFORM transformation.
> >
> > Yes, thankfully we don't really encode the IP address family in any of
> > the SELinux object classes so that shouldn't be an issue.  I also
> > don't think we have to worry about the per-packet labeling protocols
> > as it's too late in the communication to change the socket's
> > associated packet labeling, it's either working or it isn't; we should
> > handle the mapped IPv4 address already.
> >
> > I am a little concerned about bind being the only place where we have
> > to worry about accessing sk_family while the socket isn't locked.  As
> > an example, I'm a little concerned about the netfilter code paths; I
> > haven't chased them down, but my guess is that the associated
> > socket/sock isn't locked in those cases (in the relevant output and
> > postroute cases, forward should be a non-issue).

We still need an answer on this.

> > How bad is the performance impact of READ_ONCE()?  In other words, how
> > stupid would it be to simply do all of our sock->sk_family lookups
> > using READ_ONCE()?
>
> I could be wrong, but I don't think there is any overhead except on Dec Alpha.

Then perhaps the right answer is to use it everywhere.

-- 
paul-moore.com

Re: [PATCH] selinux: Read sk->sk_family once in selinux_socket_bind()

[Mikhail Ivanov]

On 1/8/2025 12:00 AM, Paul Moore wrote:
> On Tue, Jan 7, 2025 at 3:16 PM Stephen Smalley
> <stephen.smalley.work@gmail.com> wrote:
>> On Fri, Dec 13, 2024 at 3:09 PM Paul Moore <paul@paul-moore.com> wrote:
>>> On Fri, Dec 13, 2024 at 11:40 AM Mikhail Ivanov
>>> <ivanov.mikhail1@huawei-partners.com> wrote:
>>>> On 12/13/2024 6:46 PM, Stephen Smalley wrote:
>>>>> On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov
>>>>> <ivanov.mikhail1@huawei-partners.com> wrote:
>>>>>>
>>>>>> On 12/12/2024 8:50 PM, Mickaël Salaün wrote:
>>>>>>> This looks good be there are other places using sk->sk_family that
>>>>>>> should also be fixed.
>>>>>>
>>>>>> Thanks for checking this!
>>>>>>
>>>>>> For selinux this should be enough, I haven't found any other places
>>>>>> where sk->sk_family could be read from an IPv6 socket without locking.
>>>>>>
>>>>>> I also would like to prepare such fix for other LSMs (apparmor, smack,
>>>>>> tomoyo) (in separate patches).
>>>>>
>>>>> I'm wondering about the implications for SELinux beyond just
>>>>> sk->sk_family access, e.g. SELinux maps the (family, type, protocol)
>>>>> triple to a security class at socket creation time via
>>>>> socket_type_to_security_class() and caches the security class in the
>>>>> inode_security_struct and sk_security_struct for later use.
>>>>
>>>> IPv6 and IPv4 TCP sockets are mapped to the same SECCLASS_TCP_SOCKET
>>>> security class. AFAICS there is no other places that can be affected by
>>>> the IPV6_ADDFORM transformation.
>>>
>>> Yes, thankfully we don't really encode the IP address family in any of
>>> the SELinux object classes so that shouldn't be an issue.  I also
>>> don't think we have to worry about the per-packet labeling protocols
>>> as it's too late in the communication to change the socket's
>>> associated packet labeling, it's either working or it isn't; we should
>>> handle the mapped IPv4 address already.
>>>
>>> I am a little concerned about bind being the only place where we have
>>> to worry about accessing sk_family while the socket isn't locked.  As
>>> an example, I'm a little concerned about the netfilter code paths; I
>>> haven't chased them down, but my guess is that the associated
>>> socket/sock isn't locked in those cases (in the relevant output and
>>> postroute cases, forward should be a non-issue).
> 
> We still need an answer on this.

Sorry for the late reply,
I found out that security_sock_rcv_skb() can also be called without
locking the IPv6 socket (this can be easily verified by manual testing).

Netfilter hooks seems to be ok, family value is taken from the
nf_hook_state structure, so there is no access to sk->sk_family.

SCTP and MPTCP hooks should not be considered, because IPV6_ADDRFORM is
only available for TCP, UDP and UDPLITE protocols.

There are 2 more functions that access sk_family:
* security_sock_graft() - socket is locked by inet_accept(),
* security_inet_conn_established() - socket is locked by connect(2) or
   in BH context (Cf. tcp_v6_rcv).

> 
>>> How bad is the performance impact of READ_ONCE()?  In other words, how
>>> stupid would it be to simply do all of our sock->sk_family lookups
>>> using READ_ONCE()?
>>
>> I could be wrong, but I don't think there is any overhead except on Dec Alpha.
> 
> Then perhaps the right answer is to use it everywhere.
> 

Indeed, using READ_ONCE() in the considered hooks should not lead to
any overhead. I wonder if it would be better not to touch the SCTP
and MPTCP hooks anyway. Adding READ_ONCE() in selinux_sock_graft() is
fine if you think it's better this way.