[PATCH] net: socket: Always initialize family field at move_addr_to_kernel().

Tetsuo Handa penguin-kernel at i-love.sakura.ne.jp
Fri Apr 12 00:24:10 UTC 2019


Paul Moore wrote:
> > +       /*
> > +        * Nothing more to do if valid length is too short to check
> > +        * address->sa_family.
> > +        */
> > +       if (addrlen < offsetofend(struct sockaddr, sa_family))
> > +               goto out;
> 
> SELinux already checks the address length further down in
> selinux_socket_bind() for the address families it care about, although
> it does read sa_family before the address length check so no
> unfortunately it's of no help.

Exactly.

Since we will anyway have to check valid length after sa_family is checked,
reading a stale sa_family is harmless except KMSAN complains uninit-value.
Therefore,

--- a/net/socket.c
+++ b/net/socket.c
@@ -181,6 +181,7 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
 
 int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr)
 {
+	kaddr->ss_family = 0;
 	if (ulen < 0 || ulen > sizeof(struct sockaddr_storage))
 		return -EINVAL;
 	if (ulen == 0)

or

--- a/net/socket.c
+++ b/net/socket.c
@@ -181,6 +181,7 @@ static ssize_t sock_splice_read(struct file *file, loff_t *ppos,
 
 int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr)
 {
+	memset(kaddr, 0, sizeof(struct sockaddr_storage));
 	if (ulen < 0 || ulen > sizeof(struct sockaddr_storage))
 		return -EINVAL;
 	if (ulen == 0)

will avoid adding this "addrlen < offsetofend(struct sockaddr, sa_family)" check to every
LSM module. It is a bit pity that while it is guaranteed that sizeof(struct sockaddr_storage)
bytes are accessible, it is not guaranteed that reading "struct sockaddr_storage"->family is
safe. (Thus, I wanted to hear comments from LSM people.)

> 
> I imagine you could move this new length check inside the
> PF_INET/PF_INET6 if-then code block to minimize the impact to other
> address families, but I suppose there is some value in keeping it
> where it is too.

I guess that since PF_INET/PF_INET6/PF_UNIX will be most frequently used protocols,
the impact to non-PF_INET/PF_INET6 protocols is negligible.



More information about the Linux-security-module-archive mailing list