[PATCH net] tun/tap: sanitize TUNSETSNDBUF input

[PATCH net] tun/tap: sanitize TUNSETSNDBUF input

[Craig Gallek]

From: Craig Gallek <kraig@google.com>

Syzkaller found several variants of the lockup below by setting negative
values with the TUNSETSNDBUF ioctl.  This patch adds a sanity check
to both the tun and tap versions of this ioctl.

  watchdog: BUG: soft lockup - CPU#0 stuck for 22s! [repro:2389]
  Modules linked in:
  irq event stamp: 329692056
  hardirqs last  enabled at (329692055): [<ffffffff824b8381>] _raw_spin_unlock_irqrestore+0x31/0x75
  hardirqs last disabled at (329692056): [<ffffffff824b9e58>] apic_timer_interrupt+0x98/0xb0
  softirqs last  enabled at (35659740): [<ffffffff824bc958>] __do_softirq+0x328/0x48c
  softirqs last disabled at (35659731): [<ffffffff811c796c>] irq_exit+0xbc/0xd0
  CPU: 0 PID: 2389 Comm: repro Not tainted 4.14.0-rc7 #23
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
  task: ffff880009452140 task.stack: ffff880006a20000
  RIP: 0010:_raw_spin_lock_irqsave+0x11/0x80
  RSP: 0018:ffff880006a27c50 EFLAGS: 00000282 ORIG_RAX: ffffffffffffff10
  RAX: ffff880009ac68d0 RBX: ffff880006a27ce0 RCX: 0000000000000000
  RDX: 0000000000000001 RSI: ffff880006a27ce0 RDI: ffff880009ac6900
  RBP: ffff880006a27c60 R08: 0000000000000000 R09: 0000000000000000
  R10: 0000000000000001 R11: 000000000063ff00 R12: ffff880009ac6900
  R13: ffff880006a27cf8 R14: 0000000000000001 R15: ffff880006a27cf8
  FS:  00007f4be4838700(0000) GS:ffff88000cc00000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 0000000020101000 CR3: 0000000009616000 CR4: 00000000000006f0
  Call Trace:
   prepare_to_wait+0x26/0xc0
   sock_alloc_send_pskb+0x14e/0x270
   ? remove_wait_queue+0x60/0x60
   tun_get_user+0x2cc/0x19d0
   ? __tun_get+0x60/0x1b0
   tun_chr_write_iter+0x57/0x86
   __vfs_write+0x156/0x1e0
   vfs_write+0xf7/0x230
   SyS_write+0x57/0xd0
   entry_SYSCALL_64_fastpath+0x1f/0xbe
  RIP: 0033:0x7f4be4356df9
  RSP: 002b:00007ffc18101c08 EFLAGS: 00000293 ORIG_RAX: 0000000000000001
  RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f4be4356df9
  RDX: 0000000000000046 RSI: 0000000020101000 RDI: 0000000000000005
  RBP: 00007ffc18101c40 R08: 0000000000000001 R09: 0000000000000001
  R10: 0000000000000001 R11: 0000000000000293 R12: 0000559c75f64780
  R13: 00007ffc18101d30 R14: 0000000000000000 R15: 0000000000000000

Fixes: 33dccbb050bb ("tun: Limit amount of queued packets per device")
Fixes: 20d29d7a916a ("net: macvtap driver")
Signed-off-by: Craig Gallek <kraig@google.com>
---
 drivers/net/tap.c | 2 ++
 drivers/net/tun.c | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/drivers/net/tap.c b/drivers/net/tap.c
index 1b10fcc6a58d..6c0c84c33e1f 100644
--- a/drivers/net/tap.c
+++ b/drivers/net/tap.c
@@ -1032,6 +1032,8 @@ static long tap_ioctl(struct file *file, unsigned int cmd,
 	case TUNSETSNDBUF:
 		if (get_user(s, sp))
 			return -EFAULT;
+		if (s <= 0)
+			return -EINVAL;
 
 		q->sk.sk_sndbuf = s;
 		return 0;
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 5550f56cb895..42bb820a56c9 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -2429,6 +2429,10 @@ static long __tun_chr_ioctl(struct file *file, unsigned int cmd,
 			ret = -EFAULT;
 			break;
 		}
+		if (sndbuf <= 0) {
+			ret = -EINVAL;
+			break;
+		}
 
 		tun->sndbuf = sndbuf;
 		tun_set_sndbuf(tun);
-- 
2.15.0.rc2.357.g7e34df9404-goog

Re: [PATCH net] tun/tap: sanitize TUNSETSNDBUF input

[Eric Dumazet]

On Mon, 2017-10-30 at 18:50 -0400, Craig Gallek wrote:
> From: Craig Gallek <kraig@google.com>
> 
> Syzkaller found several variants of the lockup below by setting negative
> values with the TUNSETSNDBUF ioctl.  This patch adds a sanity check
> to both the tun and tap versions of this ioctl.
> 

Reviewed-by: Eric Dumazet <edumazet@google.com>

Thanks !

Re: [PATCH net] tun/tap: sanitize TUNSETSNDBUF input

[David Miller]

From: Craig Gallek <kraigatgoog@gmail.com>
Date: Mon, 30 Oct 2017 18:50:11 -0400

> From: Craig Gallek <kraig@google.com>
> 
> Syzkaller found several variants of the lockup below by setting negative
> values with the TUNSETSNDBUF ioctl.  This patch adds a sanity check
> to both the tun and tap versions of this ioctl.
 ...
> Fixes: 33dccbb050bb ("tun: Limit amount of queued packets per device")
> Fixes: 20d29d7a916a ("net: macvtap driver")
> Signed-off-by: Craig Gallek <kraig@google.com>

Applied and queued up for -stable, thanks.

Re: [PATCH net] tun/tap: sanitize TUNSETSNDBUF input

[Jason A. Donenfeld]

Here's a simple reproducer, in case Skyzaller's case was overcomplicated:

#include <stdio.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <sys/types.h>
#include <sys/ioctl.h>
#include <linux/if.h>
#include <linux/if_tun.h>

int main(int argc, char *argv[])
{
  struct ifreq ifr;
  int fd, sock;

  fd = open("/dev/net/tun", O_RDWR);
  if (fd < 0) {
    perror("open(/dev/net/tun)");
    return 1;
  }

  memset(&ifr, 0, sizeof(ifr));

  ifr.ifr_flags = IFF_TUN;
  strncpy(ifr.ifr_name, "yikes", IFNAMSIZ);

  if (ioctl(fd, TUNSETIFF, &ifr) < 0) {
    perror("TUNSETIFF");
    return 1;
  }

  sock = socket(AF_INET, SOCK_DGRAM, 0);
  if (sock < 0) {
    perror("socket");
    return 1;
  }
  ifr.ifr_flags = IFF_UP;
  if (ioctl(sock, SIOCSIFFLAGS, &ifr) < 0) {
    perror("SIOCSIFFLAGS");
    return 1;
  }
  close(sock);

  sock = -1;
  if (ioctl(fd, TUNSETSNDBUF, &sock)) {
    perror("TUNSETSNDBUF");
    return 1;
  }

  if (write(fd, &fd, sizeof(fd)) < 0) {
    perror("write");
    return 1;
  }

  return 0;
}