On Tue, Jan 7, 2025 at 3:16 PM Stephen Smalley <stephen.smalley.work@xxxxxxxxx> wrote: > On Fri, Dec 13, 2024 at 3:09 PM Paul Moore <paul@xxxxxxxxxxxxxx> wrote: > > On Fri, Dec 13, 2024 at 11:40 AM Mikhail Ivanov > > <ivanov.mikhail1@xxxxxxxxxxxxxxxxxxx> wrote: > > > On 12/13/2024 6:46 PM, Stephen Smalley wrote: > > > > On Fri, Dec 13, 2024 at 5:57 AM Mikhail Ivanov > > > > <ivanov.mikhail1@xxxxxxxxxxxxxxxxxxx> 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
