net: clear msg_get_inq in __sys_recvfrom() and __copy_msghdr_from_user()
authorEric Dumazet <edumazet@google.com>
Wed, 22 Jun 2022 15:02:20 +0000 (15:02 +0000)
committerJakub Kicinski <kuba@kernel.org>
Fri, 24 Jun 2022 03:56:23 +0000 (20:56 -0700)
syzbot reported uninit-value in tcp_recvmsg() [1]

Issue here is that msg->msg_get_inq should have been cleared,
otherwise tcp_recvmsg() might read garbage and perform
more work than needed, or have undefined behavior.

Given CONFIG_INIT_STACK_ALL_ZERO=y is probably going to be
the default soon, I chose to change __sys_recvfrom() to clear
all fields but msghdr.addr which might be not NULL.

For __copy_msghdr_from_user(), I added an explicit clear
of kmsg->msg_get_inq.

[1]
BUG: KMSAN: uninit-value in tcp_recvmsg+0x6cf/0xb60 net/ipv4/tcp.c:2557
tcp_recvmsg+0x6cf/0xb60 net/ipv4/tcp.c:2557
inet_recvmsg+0x13a/0x5a0 net/ipv4/af_inet.c:850
sock_recvmsg_nosec net/socket.c:995 [inline]
sock_recvmsg net/socket.c:1013 [inline]
__sys_recvfrom+0x696/0x900 net/socket.c:2176
__do_sys_recvfrom net/socket.c:2194 [inline]
__se_sys_recvfrom net/socket.c:2190 [inline]
__x64_sys_recvfrom+0x122/0x1c0 net/socket.c:2190
do_syscall_x64 arch/x86/entry/common.c:50 [inline]
do_syscall_64+0x3d/0xb0 arch/x86/entry/common.c:80
entry_SYSCALL_64_after_hwframe+0x46/0xb0

Local variable msg created at:
__sys_recvfrom+0x81/0x900 net/socket.c:2154
__do_sys_recvfrom net/socket.c:2194 [inline]
__se_sys_recvfrom net/socket.c:2190 [inline]
__x64_sys_recvfrom+0x122/0x1c0 net/socket.c:2190

CPU: 0 PID: 3493 Comm: syz-executor170 Not tainted 5.19.0-rc3-syzkaller-30868-g4b28366af7d9 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011

Fixes: f94fd25cb0aa ("tcp: pass back data left in socket after receive")
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Jens Axboe <axboe@kernel.dk>
Tested-by: Alexander Potapenko<glider@google.com>
Link: https://lore.kernel.org/r/20220622150220.1091182-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net/socket.c

index 2bc8773d9dc5e60595b14c2289e48d4ff0241da0..96300cdc06251fd654bce6277c54677f686079f8 100644 (file)
@@ -2149,10 +2149,13 @@ SYSCALL_DEFINE4(send, int, fd, void __user *, buff, size_t, len,
 int __sys_recvfrom(int fd, void __user *ubuf, size_t size, unsigned int flags,
                   struct sockaddr __user *addr, int __user *addr_len)
 {
+       struct sockaddr_storage address;
+       struct msghdr msg = {
+               /* Save some cycles and don't copy the address if not needed */
+               .msg_name = addr ? (struct sockaddr *)&address : NULL,
+       };
        struct socket *sock;
        struct iovec iov;
-       struct msghdr msg;
-       struct sockaddr_storage address;
        int err, err2;
        int fput_needed;
 
@@ -2163,14 +2166,6 @@ int __sys_recvfrom(int fd, void __user *ubuf, size_t size, unsigned int flags,
        if (!sock)
                goto out;
 
-       msg.msg_control = NULL;
-       msg.msg_controllen = 0;
-       /* Save some cycles and don't copy the address if not needed */
-       msg.msg_name = addr ? (struct sockaddr *)&address : NULL;
-       /* We assume all kernel code knows the size of sockaddr_storage */
-       msg.msg_namelen = 0;
-       msg.msg_iocb = NULL;
-       msg.msg_flags = 0;
        if (sock->file->f_flags & O_NONBLOCK)
                flags |= MSG_DONTWAIT;
        err = sock_recvmsg(sock, &msg, flags);
@@ -2375,6 +2370,7 @@ int __copy_msghdr_from_user(struct msghdr *kmsg,
                return -EFAULT;
 
        kmsg->msg_control_is_user = true;
+       kmsg->msg_get_inq = 0;
        kmsg->msg_control_user = msg.msg_control;
        kmsg->msg_controllen = msg.msg_controllen;
        kmsg->msg_flags = msg.msg_flags;