Message ID | 20171030225011.184639-1-kraigatgoog@gmail.com |
---|---|
State | Accepted, archived |
Delegated to: | David Miller |
Headers | show |
Series | [net] tun/tap: sanitize TUNSETSNDBUF input | expand |
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 !
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.
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; }
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);