[PATCH] net: initialize msg.msg_flags in recvfrom
Subsystems:
networking [general] , networking [sockets] , the rest
3440d
5 messages,
3 authors,
2017-03-07 · open the first message on its own page
KMSAN (KernelMemorySanitizer, a new error detection tool) reports use
of uninitialized memory in put_cmsg()):
==================================================================
BUG: KMSAN: use of unitialized memory
inter: 0
CPU: 3 PID: 1086 Comm: syz-executor0 Not tainted 4.8.0-rc6+ #1920
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
0000000000000000 ffff88008374fa48 ffffffff82576c18 ffff88008374f988
ffffffff818c06d1 ffffffff85bab870 0000000000000092 ffffffff85bab550
0000000000000000 0000000000000092 00000000f9600a55 0000000000000002
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff82576c18>] dump_stack+0x238/0x290 lib/dump_stack.c:51
[<ffffffff818beef6>] kmsan_report+0x276/0x2e0 mm/kmsan/kmsan.c:1006
[<ffffffff818c027b>] __msan_warning+0x6b/0xc0 mm/kmsan/kmsan_instr.c:425
[<ffffffff842f01e7>] put_cmsg+0x6f7/0x850 net/core/scm.c:223
[< inline >] netlink_cmsg_recv_pktinfo net/netlink/af_netlink.c:1700
[<ffffffff844b7054>] netlink_recvmsg+0x8e4/0x1980 net/netlink/af_netlink.c:1856
[< inline >] sock_recvmsg_nosec net/socket.c:711
[< inline >] sock_recvmsg net/socket.c:718
[<ffffffff8426c175>] SYSC_recvfrom+0x6e5/0xa00 net/socket.c:1695
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
chained origin: 00000000f8c00a55
[<ffffffff810bd4d7>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
[< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322
[< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337
[<ffffffff818be2cb>] kmsan_internal_chain_origin+0x12b/0x1f0 mm/kmsan/kmsan.c:530
[<ffffffff818bfc0f>] __kmsan_store_origin+0x9f/0x120 mm/kmsan/kmsan_instr.c:190
[<ffffffff844b6bdd>] netlink_recvmsg+0x46d/0x1980 net/netlink/af_netlink.c:1839
[< inline >] sock_recvmsg_nosec net/socket.c:711
[< inline >] sock_recvmsg net/socket.c:718
[<ffffffff8426c175>] SYSC_recvfrom+0x6e5/0xa00 net/socket.c:1695
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
chained origin: 00000000b9a00a53
[<ffffffff810bd4d7>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
[< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322
[< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337
[<ffffffff818be2cb>] kmsan_internal_chain_origin+0x12b/0x1f0 mm/kmsan/kmsan.c:530
[<ffffffff818c01a3>] __msan_set_alloca_origin4+0xc3/0x130 mm/kmsan/kmsan_instr.c:380
[<ffffffff8426bc13>] SYSC_recvfrom+0x183/0xa00 net/socket.c:1671
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
origin description: ----msg@SYSC_recvfrom (origin=00000000b9400a53)
==================================================================
The fields of |msg| in SYSC_recvfrom() are initialized except
for |msg.msg_flags|, which is later updated in netlink_recvmsg():
1839 msg->msg_flags |= MSG_TRUNC;
and then used in a condition in put_cmsg():
223 if (MSG_CMSG_COMPAT & msg->msg_flags)
The following program has been used to reproduce the error under KMSAN:
===================================================
#include <linux/netlink.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/wait.h>
void child()
{
int sock = socket(PF_NETLINK, SOCK_RAW, 16);
uint32_t val = 0x1f;
setsockopt(sock, 0x10e, 0x3, &val, 4);
char obuf[31] = "\x1f\x00\x00\x00\x10\x00\x07\xff\xff\x08\x00\x00"
"\x00\x00\x00\x00\x03\x00\x01\x0a\x56\x67\x69\x93"
"\x3e\x00\x00\x01\x92\x9a\x02";
char ibuf[31];
write(sock, obuf, sizeof(obuf));
struct sockaddr_nl sockaddr = {AF_NETLINK, 0, 0, 0};
socklen_t addrlen = sizeof(sockaddr);
recvfrom(sock, ibuf, 0, 0, &sockaddr, &addrlen);
}
int main()
{
int pid = fork();
if (!pid) {
child();
return 0;
}
int status = 0;
while (waitpid(pid, &status, __WALL) != pid) {}
return 0;
}
===================================================
Signed-off-by: Alexander Potapenko <glider@google.com>
---
net/socket.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/socket.c b/net/socket.c
index 2c1e8677ff2d..e0757e648c0c 100644
--- a/net/socket.c
+++ b/net/socket.c @@ -1731,6 +1731,7 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
/* 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 ); --
2.12.0.rc1.440.g5b76565f74-goog
On Tue, 2017-03-07 at 14:58 +0100, Alexander Potapenko wrote: KMSAN (KernelMemorySanitizer, a new error detection tool) reports use
of uninitialized memory in put_cmsg()):
I would prefer that you do not put the stack trace in the changelog,
same for the reproducer since this has little value in understanding the
impact.
It looks like a false positive, but you do not say.
recvmsg() does not care about msg.msg_flags, only KMSAN.
(The important part is that msg.msg_control and msg.msg_controllen are
0)
Fine to avoid the false positive, but better be explicit in the
changelog and says there is no visible effect for this bug.
If there is a visible effect, please state so instead of technical
details.
We try to reduce S/N in the changelogs ;)
Thanks a lot !
On Tue, Mar 7, 2017 at 3:26 PM, Eric Dumazet [off-list ref] wrote: On Tue, 2017-03-07 at 14:58 +0100, Alexander Potapenko wrote: quoted KMSAN (KernelMemorySanitizer, a new error detection tool) reports use
of uninitialized memory in put_cmsg()):
I would prefer that you do not put the stack trace in the changelog,
same for the reproducer since this has little value in understanding the
impact. Understood.
Should be ok to put the report/reproducer below the triple dash, right?
It looks like a false positive, but you do not say. Ah, now I see.
Irrespective of the value of (MSG_CMSG_COMPAT & msg->msg_flags) the
code will return 0 either directly from put_cmsg(), or from
put_cmsg_compat().
I wouldn't call this a false positive, as KMSAN can't possibly know
that both branches taken depending on the uninitialized condition are
safe.
But I can imagine this to be less of an problem to the code owners who
do know that :)
recvmsg() does not care about msg.msg_flags, only KMSAN.
(The important part is that msg.msg_control and msg.msg_controllen are
0)
Fine to avoid the false positive, but better be explicit in the
changelog and says there is no visible effect for this bug. Ok, I'll change the description. If there is a visible effect, please state so instead of technical
details.
We try to reduce S/N in the changelogs ;)
Thanks a lot !
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
On Tue, Mar 7, 2017 at 2:58 PM, Alexander Potapenko [off-list ref] wrote: quoted hunk KMSAN (KernelMemorySanitizer, a new error detection tool) reports use
of uninitialized memory in put_cmsg()):
==================================================================
BUG: KMSAN: use of unitialized memory
inter: 0
CPU: 3 PID: 1086 Comm: syz-executor0 Not tainted 4.8.0-rc6+ #1920
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
0000000000000000 ffff88008374fa48 ffffffff82576c18 ffff88008374f988
ffffffff818c06d1 ffffffff85bab870 0000000000000092 ffffffff85bab550
0000000000000000 0000000000000092 00000000f9600a55 0000000000000002
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff82576c18>] dump_stack+0x238/0x290 lib/dump_stack.c:51
[<ffffffff818beef6>] kmsan_report+0x276/0x2e0 mm/kmsan/kmsan.c:1006
[<ffffffff818c027b>] __msan_warning+0x6b/0xc0 mm/kmsan/kmsan_instr.c:425
[<ffffffff842f01e7>] put_cmsg+0x6f7/0x850 net/core/scm.c:223
[< inline >] netlink_cmsg_recv_pktinfo net/netlink/af_netlink.c:1700
[<ffffffff844b7054>] netlink_recvmsg+0x8e4/0x1980 net/netlink/af_netlink.c:1856
[< inline >] sock_recvmsg_nosec net/socket.c:711
[< inline >] sock_recvmsg net/socket.c:718
[<ffffffff8426c175>] SYSC_recvfrom+0x6e5/0xa00 net/socket.c:1695
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
chained origin: 00000000f8c00a55
[<ffffffff810bd4d7>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
[< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322
[< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337
[<ffffffff818be2cb>] kmsan_internal_chain_origin+0x12b/0x1f0 mm/kmsan/kmsan.c:530
[<ffffffff818bfc0f>] __kmsan_store_origin+0x9f/0x120 mm/kmsan/kmsan_instr.c:190
[<ffffffff844b6bdd>] netlink_recvmsg+0x46d/0x1980 net/netlink/af_netlink.c:1839
[< inline >] sock_recvmsg_nosec net/socket.c:711
[< inline >] sock_recvmsg net/socket.c:718
[<ffffffff8426c175>] SYSC_recvfrom+0x6e5/0xa00 net/socket.c:1695
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
chained origin: 00000000b9a00a53
[<ffffffff810bd4d7>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
[< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322
[< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337
[<ffffffff818be2cb>] kmsan_internal_chain_origin+0x12b/0x1f0 mm/kmsan/kmsan.c:530
[<ffffffff818c01a3>] __msan_set_alloca_origin4+0xc3/0x130 mm/kmsan/kmsan_instr.c:380
[<ffffffff8426bc13>] SYSC_recvfrom+0x183/0xa00 net/socket.c:1671
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
origin description: ----msg@SYSC_recvfrom (origin=00000000b9400a53)
==================================================================
The fields of |msg| in SYSC_recvfrom() are initialized except
for |msg.msg_flags|, which is later updated in netlink_recvmsg():
1839 msg->msg_flags |= MSG_TRUNC;
and then used in a condition in put_cmsg():
223 if (MSG_CMSG_COMPAT & msg->msg_flags)
The following program has been used to reproduce the error under KMSAN:
===================================================
#include <linux/netlink.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/wait.h>
void child()
{
int sock = socket(PF_NETLINK, SOCK_RAW, 16);
uint32_t val = 0x1f;
setsockopt(sock, 0x10e, 0x3, &val, 4);
char obuf[31] = "\x1f\x00\x00\x00\x10\x00\x07\xff\xff\x08\x00\x00"
"\x00\x00\x00\x00\x03\x00\x01\x0a\x56\x67\x69\x93"
"\x3e\x00\x00\x01\x92\x9a\x02";
char ibuf[31];
write(sock, obuf, sizeof(obuf));
struct sockaddr_nl sockaddr = {AF_NETLINK, 0, 0, 0};
socklen_t addrlen = sizeof(sockaddr);
recvfrom(sock, ibuf, 0, 0, &sockaddr, &addrlen);
}
int main()
{
int pid = fork();
if (!pid) {
child();
return 0;
}
int status = 0;
while (waitpid(pid, &status, __WALL) != pid) {}
return 0;
}
===================================================
Signed-off-by: Alexander Potapenko <glider@google.com>
---
net/socket.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/socket.c b/net/socket.c
index 2c1e8677ff2d..e0757e648c0c 100644
--- a/net/socket.c
+++ b/net/socket.c @@ -1731,6 +1731,7 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
/* 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 );
- - 2.12.0.rc1.440.g5b76565f74-goog
I am not sure this is correct. The compat syscall does:
COMPAT_SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, buf,
compat_size_t, len,
unsigned int, flags, struct sockaddr __user *, addr,
int __user *, addrlen)
{
return sys_recvfrom(fd, buf, len, flags | MSG_CMSG_COMPAT, addr, addrlen);
}
netlink_cmsg_recv_pktinfo expects that MSG_CMSG_COMPAT in msg.flags,
but we put it into flags.
On Tue, Mar 7, 2017 at 3:23 PM, Dmitry Vyukov [off-list ref] wrote: On Tue, Mar 7, 2017 at 2:58 PM, Alexander Potapenko [off-list ref] wrote: quoted KMSAN (KernelMemorySanitizer, a new error detection tool) reports use
of uninitialized memory in put_cmsg()):
==================================================================
BUG: KMSAN: use of unitialized memory
inter: 0
CPU: 3 PID: 1086 Comm: syz-executor0 Not tainted 4.8.0-rc6+ #1920
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
0000000000000000 ffff88008374fa48 ffffffff82576c18 ffff88008374f988
ffffffff818c06d1 ffffffff85bab870 0000000000000092 ffffffff85bab550
0000000000000000 0000000000000092 00000000f9600a55 0000000000000002
Call Trace:
[< inline >] __dump_stack lib/dump_stack.c:15
[<ffffffff82576c18>] dump_stack+0x238/0x290 lib/dump_stack.c:51
[<ffffffff818beef6>] kmsan_report+0x276/0x2e0 mm/kmsan/kmsan.c:1006
[<ffffffff818c027b>] __msan_warning+0x6b/0xc0 mm/kmsan/kmsan_instr.c:425
[<ffffffff842f01e7>] put_cmsg+0x6f7/0x850 net/core/scm.c:223
[< inline >] netlink_cmsg_recv_pktinfo net/netlink/af_netlink.c:1700
[<ffffffff844b7054>] netlink_recvmsg+0x8e4/0x1980 net/netlink/af_netlink.c:1856
[< inline >] sock_recvmsg_nosec net/socket.c:711
[< inline >] sock_recvmsg net/socket.c:718
[<ffffffff8426c175>] SYSC_recvfrom+0x6e5/0xa00 net/socket.c:1695
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
chained origin: 00000000f8c00a55
[<ffffffff810bd4d7>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
[< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322
[< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337
[<ffffffff818be2cb>] kmsan_internal_chain_origin+0x12b/0x1f0 mm/kmsan/kmsan.c:530
[<ffffffff818bfc0f>] __kmsan_store_origin+0x9f/0x120 mm/kmsan/kmsan_instr.c:190
[<ffffffff844b6bdd>] netlink_recvmsg+0x46d/0x1980 net/netlink/af_netlink.c:1839
[< inline >] sock_recvmsg_nosec net/socket.c:711
[< inline >] sock_recvmsg net/socket.c:718
[<ffffffff8426c175>] SYSC_recvfrom+0x6e5/0xa00 net/socket.c:1695
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
chained origin: 00000000b9a00a53
[<ffffffff810bd4d7>] save_stack_trace+0x27/0x50 arch/x86/kernel/stacktrace.c:67
[< inline >] kmsan_save_stack_with_flags mm/kmsan/kmsan.c:322
[< inline >] kmsan_save_stack mm/kmsan/kmsan.c:337
[<ffffffff818be2cb>] kmsan_internal_chain_origin+0x12b/0x1f0 mm/kmsan/kmsan.c:530
[<ffffffff818c01a3>] __msan_set_alloca_origin4+0xc3/0x130 mm/kmsan/kmsan_instr.c:380
[<ffffffff8426bc13>] SYSC_recvfrom+0x183/0xa00 net/socket.c:1671
[<ffffffff8426ba67>] SyS_recvfrom+0xb7/0xe0 net/socket.c:1668
[<ffffffff851840db>] entry_SYSCALL_64_fastpath+0x13/0x8f arch/x86/entry/entry_64.o:?
origin description: ----msg@SYSC_recvfrom (origin=00000000b9400a53)
==================================================================
The fields of |msg| in SYSC_recvfrom() are initialized except
for |msg.msg_flags|, which is later updated in netlink_recvmsg():
1839 msg->msg_flags |= MSG_TRUNC;
and then used in a condition in put_cmsg():
223 if (MSG_CMSG_COMPAT & msg->msg_flags)
The following program has been used to reproduce the error under KMSAN:
===================================================
#include <linux/netlink.h>
#include <stdint.h>
#include <stdio.h>
#include <sys/socket.h>
#include <sys/wait.h>
void child()
{
int sock = socket(PF_NETLINK, SOCK_RAW, 16);
uint32_t val = 0x1f;
setsockopt(sock, 0x10e, 0x3, &val, 4);
char obuf[31] = "\x1f\x00\x00\x00\x10\x00\x07\xff\xff\x08\x00\x00"
"\x00\x00\x00\x00\x03\x00\x01\x0a\x56\x67\x69\x93"
"\x3e\x00\x00\x01\x92\x9a\x02";
char ibuf[31];
write(sock, obuf, sizeof(obuf));
struct sockaddr_nl sockaddr = {AF_NETLINK, 0, 0, 0};
socklen_t addrlen = sizeof(sockaddr);
recvfrom(sock, ibuf, 0, 0, &sockaddr, &addrlen);
}
int main()
{
int pid = fork();
if (!pid) {
child();
return 0;
}
int status = 0;
while (waitpid(pid, &status, __WALL) != pid) {}
return 0;
}
===================================================
Signed-off-by: Alexander Potapenko <glider@google.com>
---
net/socket.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/net/socket.c b/net/socket.c
index 2c1e8677ff2d..e0757e648c0c 100644
--- a/net/socket.c
+++ b/net/socket.c @@ -1731,6 +1731,7 @@ SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, ubuf, size_t, size,
/* 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 );
- - 2.12.0.rc1.440.g5b76565f74-goog
I am not sure this is correct. The compat syscall does: I've changed the initial value of msg.msg_flags to 0xdeadbeef and
added the following line to put_cmsg():
pr_err("msg->msg_flags: %p\n", msg->msg_flags);
When I run the above repro, I see the following lines in the log:
msg->msg_flags: 00000000deadbeef
Therefore I believe the bug is real.
COMPAT_SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, buf, How is the compat syscall related to that? compat_size_t, len,
unsigned int, flags, struct sockaddr __user *, addr,
int __user *, addrlen)
{
return sys_recvfrom(fd, buf, len, flags | MSG_CMSG_COMPAT, addr, addrlen);
}
netlink_cmsg_recv_pktinfo expects that MSG_CMSG_COMPAT in msg.flags,
but we put it into flags.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg