netdev - [PATCH] net: socket: Always initialize family field at move_addr_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Date:   Mon,  1 Apr 2019 23:19:22 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     "David S. Miller" <davem@...emloft.net>
Cc:     netdev@...r.kernel.org,
        Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>,
        syzbot <syzbot+0049bebbf3042dbd2e8f@...kaller.appspotmail.com>,
        syzbot <syzbot+915c9f99f3dbc4bd6cd1@...kaller.appspotmail.com>
Subject: [PATCH] net: socket: Always initialize family field at move_addr_to_kernel().

syzbot is reporting uninitialized value at rds_connect [1] and
rds_bind [2]. This is because syzbot is passing ulen == 0 whereas
these functions expects that it is safe to access sockaddr->family field
in order to determine minimal ulen size for validation. I noticed that
the same problem also exists in tomoyo_check_inet_address() function.

Although the right fix might be to scatter around

  if (ulen < sizeof(__kernel_sa_family_t))
    return 0;

if the function wants to become no-op when the address is too short or

  if (ulen < sizeof(__kernel_sa_family_t))
    return -EINVAL;

if the function wants to reject when the address is too short, we can
avoid duplication (at e.g. LSM layer and protocol layer) if we make sure
that sockaddr->family field is always accessible.

[1] https://syzkaller.appspot.com/bug?id=f4e61c010416c1e6f0fa3ffe247561b60a50ad71
[2] https://syzkaller.appspot.com/bug?id=a4bf9e41b7e055c3823fdcd83e8c58ca7270e38f

Reported-by: syzbot <syzbot+0049bebbf3042dbd2e8f@...kaller.appspotmail.com>
Reported-by: syzbot <syzbot+915c9f99f3dbc4bd6cd1@...kaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
---
 net/socket.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/socket.c b/net/socket.c
index 8255f5b..10a780b 100644
--- 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)
-- 
1.8.3.1