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.
