KMSAN (https://github.com/google/kmsan) reported accessing uninitialized
skb->data[0] in the case the skb is empty (i.e. skb->len is 0):
================================================
BUG: KMSAN: use of uninitialized memory in tun_get_user+0x19ba/0x3770
CPU: 0 PID: 3051 Comm: probe Not tainted 4.13.0+ #3140
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
...
__msan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:477
tun_get_user+0x19ba/0x3770 drivers/net/tun.c:1301
tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
call_write_iter ./include/linux/fs.h:1743
new_sync_write fs/read_write.c:457
__vfs_write+0x6c3/0x7f0 fs/read_write.c:470
vfs_write+0x3e4/0x770 fs/read_write.c:518
SYSC_write+0x12f/0x2b0 fs/read_write.c:565
SyS_write+0x55/0x80 fs/read_write.c:557
do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:245
...
origin:
...
kmsan_poison_shadow+0x6e/0xc0 mm/kmsan/kmsan.c:211
slab_alloc_node mm/slub.c:2732
__kmalloc_node_track_caller+0x351/0x370 mm/slub.c:4351
__kmalloc_reserve net/core/skbuff.c:138
__alloc_skb+0x26a/0x810 net/core/skbuff.c:231
alloc_skb ./include/linux/skbuff.h:903
alloc_skb_with_frags+0x1d7/0xc80 net/core/skbuff.c:4756
sock_alloc_send_pskb+0xabf/0xfe0 net/core/sock.c:2037
tun_alloc_skb drivers/net/tun.c:1144
tun_get_user+0x9a8/0x3770 drivers/net/tun.c:1274
tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
call_write_iter ./include/linux/fs.h:1743
new_sync_write fs/read_write.c:457
__vfs_write+0x6c3/0x7f0 fs/read_write.c:470
vfs_write+0x3e4/0x770 fs/read_write.c:518
SYSC_write+0x12f/0x2b0 fs/read_write.c:565
SyS_write+0x55/0x80 fs/read_write.c:557
do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:245
================================================
Make sure tun_get_user() doesn't touch skb->data[0] unless there is
actual data.
C reproducer below:
==========================
// autogenerated by syzkaller (http://github.com/google/syzkaller)
#define _GNU_SOURCE
#include <fcntl.h>
#include <linux/if_tun.h>
#include <netinet/ip.h>
#include <net/if.h>
#include <string.h>
#include <sys/ioctl.h>
int main()
{
int sock = socket(PF_INET, SOCK_STREAM, IPPROTO_IP);
int tun_fd = open("/dev/net/tun", O_RDWR);
struct ifreq req;
memset(&req, 0, sizeof(struct ifreq));
strcpy((char*)&req.ifr_name, "gre0");
req.ifr_flags = IFF_UP | IFF_MULTICAST;
ioctl(tun_fd, TUNSETIFF, &req);
ioctl(sock, SIOCSIFFLAGS, "gre0");
write(tun_fd, "hi", 0);
return 0;
}
==========================
Signed-off-by: Alexander Potapenko <[email protected]>
---
drivers/net/tun.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 3c9985f29950..25d96f54a729 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1496,6 +1496,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
switch (tun->flags & TUN_TYPE_MASK) {
case IFF_TUN:
if (tun->flags & IFF_NO_PI) {
+ if (!skb->len)
+ return -EINVAL;
switch (skb->data[0] & 0xf0) {
case 0x40:
pi.proto = htons(ETH_P_IP);
--
2.14.1.821.g8fa685d3b7-goog
On Tue, Sep 26, 2017 at 7:53 AM, Alexander Potapenko <[email protected]> wrote:
> KMSAN (https://github.com/google/kmsan) reported accessing uninitialized
> skb->data[0] in the case the skb is empty (i.e. skb->len is 0):
>
> ================================================
> BUG: KMSAN: use of uninitialized memory in tun_get_user+0x19ba/0x3770
> CPU: 0 PID: 3051 Comm: probe Not tainted 4.13.0+ #3140
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
> Call Trace:
> ...
> __msan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:477
> tun_get_user+0x19ba/0x3770 drivers/net/tun.c:1301
> tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
> call_write_iter ./include/linux/fs.h:1743
> new_sync_write fs/read_write.c:457
> __vfs_write+0x6c3/0x7f0 fs/read_write.c:470
> vfs_write+0x3e4/0x770 fs/read_write.c:518
> SYSC_write+0x12f/0x2b0 fs/read_write.c:565
> SyS_write+0x55/0x80 fs/read_write.c:557
> do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
> entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:245
> ...
> origin:
> ...
> kmsan_poison_shadow+0x6e/0xc0 mm/kmsan/kmsan.c:211
> slab_alloc_node mm/slub.c:2732
> __kmalloc_node_track_caller+0x351/0x370 mm/slub.c:4351
> __kmalloc_reserve net/core/skbuff.c:138
> __alloc_skb+0x26a/0x810 net/core/skbuff.c:231
> alloc_skb ./include/linux/skbuff.h:903
> alloc_skb_with_frags+0x1d7/0xc80 net/core/skbuff.c:4756
> sock_alloc_send_pskb+0xabf/0xfe0 net/core/sock.c:2037
> tun_alloc_skb drivers/net/tun.c:1144
> tun_get_user+0x9a8/0x3770 drivers/net/tun.c:1274
> tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
> call_write_iter ./include/linux/fs.h:1743
> new_sync_write fs/read_write.c:457
> __vfs_write+0x6c3/0x7f0 fs/read_write.c:470
> vfs_write+0x3e4/0x770 fs/read_write.c:518
> SYSC_write+0x12f/0x2b0 fs/read_write.c:565
> SyS_write+0x55/0x80 fs/read_write.c:557
> do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
> return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:245
> ================================================
>
> Make sure tun_get_user() doesn't touch skb->data[0] unless there is
> actual data.
>
> C reproducer below:
> ==========================
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>
> #define _GNU_SOURCE
>
> #include <fcntl.h>
> #include <linux/if_tun.h>
> #include <netinet/ip.h>
> #include <net/if.h>
> #include <string.h>
> #include <sys/ioctl.h>
>
> int main()
> {
> int sock = socket(PF_INET, SOCK_STREAM, IPPROTO_IP);
> int tun_fd = open("/dev/net/tun", O_RDWR);
> struct ifreq req;
> memset(&req, 0, sizeof(struct ifreq));
> strcpy((char*)&req.ifr_name, "gre0");
> req.ifr_flags = IFF_UP | IFF_MULTICAST;
> ioctl(tun_fd, TUNSETIFF, &req);
> ioctl(sock, SIOCSIFFLAGS, "gre0");
> write(tun_fd, "hi", 0);
> return 0;
> }
> ==========================
>
> Signed-off-by: Alexander Potapenko <[email protected]>
> ---
> drivers/net/tun.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
> index 3c9985f29950..25d96f54a729 100644
> --- a/drivers/net/tun.c
> +++ b/drivers/net/tun.c
> @@ -1496,6 +1496,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
> switch (tun->flags & TUN_TYPE_MASK) {
> case IFF_TUN:
> if (tun->flags & IFF_NO_PI) {
> + if (!skb->len)
> + return -EINVAL;
> switch (skb->data[0] & 0xf0) {
> case 0x40:
> pi.proto = htons(ETH_P_IP);
> --
> 2.14.1.821.g8fa685d3b7-goog
>
Good catch, but...
You are replacing an harmless read by a memory leak, which is far more
problematic :/
On Tue, Sep 26, 2017 at 5:02 PM, 'Eric Dumazet' via syzkaller
<[email protected]> wrote:
> On Tue, Sep 26, 2017 at 7:53 AM, Alexander Potapenko <[email protected]> wrote:
>> KMSAN (https://github.com/google/kmsan) reported accessing uninitialized
>> skb->data[0] in the case the skb is empty (i.e. skb->len is 0):
>>
>> ================================================
>> BUG: KMSAN: use of uninitialized memory in tun_get_user+0x19ba/0x3770
>> CPU: 0 PID: 3051 Comm: probe Not tainted 4.13.0+ #3140
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>> Call Trace:
>> ...
>> __msan_warning_32+0x66/0xb0 mm/kmsan/kmsan_instr.c:477
>> tun_get_user+0x19ba/0x3770 drivers/net/tun.c:1301
>> tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
>> call_write_iter ./include/linux/fs.h:1743
>> new_sync_write fs/read_write.c:457
>> __vfs_write+0x6c3/0x7f0 fs/read_write.c:470
>> vfs_write+0x3e4/0x770 fs/read_write.c:518
>> SYSC_write+0x12f/0x2b0 fs/read_write.c:565
>> SyS_write+0x55/0x80 fs/read_write.c:557
>> do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
>> entry_SYSCALL64_slow_path+0x25/0x25 arch/x86/entry/entry_64.S:245
>> ...
>> origin:
>> ...
>> kmsan_poison_shadow+0x6e/0xc0 mm/kmsan/kmsan.c:211
>> slab_alloc_node mm/slub.c:2732
>> __kmalloc_node_track_caller+0x351/0x370 mm/slub.c:4351
>> __kmalloc_reserve net/core/skbuff.c:138
>> __alloc_skb+0x26a/0x810 net/core/skbuff.c:231
>> alloc_skb ./include/linux/skbuff.h:903
>> alloc_skb_with_frags+0x1d7/0xc80 net/core/skbuff.c:4756
>> sock_alloc_send_pskb+0xabf/0xfe0 net/core/sock.c:2037
>> tun_alloc_skb drivers/net/tun.c:1144
>> tun_get_user+0x9a8/0x3770 drivers/net/tun.c:1274
>> tun_chr_write_iter+0x19f/0x300 drivers/net/tun.c:1365
>> call_write_iter ./include/linux/fs.h:1743
>> new_sync_write fs/read_write.c:457
>> __vfs_write+0x6c3/0x7f0 fs/read_write.c:470
>> vfs_write+0x3e4/0x770 fs/read_write.c:518
>> SYSC_write+0x12f/0x2b0 fs/read_write.c:565
>> SyS_write+0x55/0x80 fs/read_write.c:557
>> do_syscall_64+0x242/0x330 arch/x86/entry/common.c:284
>> return_from_SYSCALL_64+0x0/0x6a arch/x86/entry/entry_64.S:245
>> ================================================
>>
>> Make sure tun_get_user() doesn't touch skb->data[0] unless there is
>> actual data.
>>
>> C reproducer below:
>> ==========================
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>>
>> #define _GNU_SOURCE
>>
>> #include <fcntl.h>
>> #include <linux/if_tun.h>
>> #include <netinet/ip.h>
>> #include <net/if.h>
>> #include <string.h>
>> #include <sys/ioctl.h>
>>
>> int main()
>> {
>> int sock = socket(PF_INET, SOCK_STREAM, IPPROTO_IP);
>> int tun_fd = open("/dev/net/tun", O_RDWR);
>> struct ifreq req;
>> memset(&req, 0, sizeof(struct ifreq));
>> strcpy((char*)&req.ifr_name, "gre0");
>> req.ifr_flags = IFF_UP | IFF_MULTICAST;
>> ioctl(tun_fd, TUNSETIFF, &req);
>> ioctl(sock, SIOCSIFFLAGS, "gre0");
>> write(tun_fd, "hi", 0);
>> return 0;
>> }
>> ==========================
>>
>> Signed-off-by: Alexander Potapenko <[email protected]>
>> ---
>> drivers/net/tun.c | 2 ++
>> 1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/net/tun.c b/drivers/net/tun.c
>> index 3c9985f29950..25d96f54a729 100644
>> --- a/drivers/net/tun.c
>> +++ b/drivers/net/tun.c
>> @@ -1496,6 +1496,8 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
>> switch (tun->flags & TUN_TYPE_MASK) {
>> case IFF_TUN:
>> if (tun->flags & IFF_NO_PI) {
>> + if (!skb->len)
>> + return -EINVAL;
>> switch (skb->data[0] & 0xf0) {
>> case 0x40:
>> pi.proto = htons(ETH_P_IP);
>> --
>> 2.14.1.821.g8fa685d3b7-goog
>>
>
> Good catch, but...
>
> You are replacing an harmless read by a memory leak, which is far more
> problematic :/
Ah, you're right, I should have done kfree_skb() and incremented the
stats. That's also rx_dropped, correct?
> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to [email protected].
> For more options, visit https://groups.google.com/d/optout.
--
Alexander Potapenko
Software Engineer
Google Germany GmbH
Erika-Mann-Straße, 33
80636 München
Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg