drivers/net/hamradio: divide error in hdlcdrv_ioctl

Hi,

I've got the following error report while fuzzing the kernel with syzkaller.

On commit 2ea659a9ef488125eb46da6eb571de5eae5c43f6 (4.12-rc1).

A reproducer and .config are attached.

divide error: 0000 [#1] SMP KASAN

Modules linked in:
CPU: 1 PID: 4019 Comm: a.out Not tainted 4.12.0-rc1+ #363
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88006b97c200 task.stack: ffff880062098000
RIP: 0010:hdlcdrv_ioctl+0x8a3/0x1a50 drivers/net/hamradio/hdlcdrv.c:579
RSP: 0018:ffff88006209f898 EFLAGS: 00010246
RAX: 000000007fffffff RBX: ffff88006209f8d0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff88006888abd0
RBP: ffff88006209f9b8 R08: 1ffff1000c413e8a R09: 1ffff1000c413e6a
R10: ffff88006b97c200 R11: 0000000000000001 R12: 1ffff1000c413f16
R13: ffff88006888a100 R14: 0000000000000000 R15: 00000000005b78f4
FS:  00007fb533884700(0000) GS:ffff88006cb00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 000000002026b000 CR3: 0000000065576000 CR4: 00000000000006e0
Call Trace:
 dev_ifsioc+0x53f/0x9f0 net/core/dev_ioctl.c:338
 dev_ioctl+0xc3c/0x1160 net/core/dev_ioctl.c:555
 sock_ioctl+0x169/0x440 net/socket.c:944
 vfs_ioctl fs/ioctl.c:45
 do_vfs_ioctl+0x1bf/0x1660 fs/ioctl.c:685
 SYSC_ioctl fs/ioctl.c:700
 SyS_ioctl+0x8f/0xc0 fs/ioctl.c:691
 entry_SYSCALL_64_fastpath+0x1f/0xbe arch/x86/entry/entry_64.S:204
RIP: 0033:0x7fb532f96b79
RSP: 002b:00007fffac2839f8 EFLAGS: 00000206 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 00007fffac283b60 RCX: 00007fb532f96b79
RDX: 00000000208f1fd8 RSI: 00000100000089f0 RDI: 0000000000000003
RBP: 00000000004004e0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000206 R12: 0000000000000000
R13: 00007fffac283b60 R14: 0000000000000000 R15: 0000000000000000
Code: 45 8b be 48 ff ff ff 48 89 fa 48 c1 ea 03 0f b6 04 02 84 c0 74
08 3c 03 0f 8e 8b 0e 00 00 45 8b b5 d0 0a 00 00 b8 ff ff ff 7f 99 <41>
f7 fe 41 39 c7 0f 87 89 0c 00 00 e8 ac e5 7e fe 49 8d bd d8
RIP: hdlcdrv_ioctl+0x8a3/0x1a50 RSP: ffff88006209f898
---[ end trace 204af21e5cc80963 ]---

qemu: terminating on signal 15 from pid 141905

// autogenerated by syzkaller (http://github.com/google/syzkaller)

#ifndef __NR_mmap
#define __NR_mmap 9
#endif
#ifndef __NR_socket
#define __NR_socket 41
#endif
#ifndef __NR_ioctl
#define __NR_ioctl 16
#endif

#define _GNU_SOURCE

#include <sys/ioctl.h>
#include <sys/mman.h>
#include <sys/mount.h>
#include <sys/prctl.h>
#include <sys/resource.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
#include <sys/wait.h>

#include <linux/capability.h>
#include <linux/if.h>
#include <linux/if_tun.h>
#include <linux/kvm.h>
#include <linux/sched.h>
#include <net/if_arp.h>

#include <assert.h>
#include <dirent.h>
#include <errno.h>
#include <fcntl.h>
#include <grp.h>
#include <pthread.h>
#include <setjmp.h>
#include <signal.h>
#include <stdarg.h>
#include <stdbool.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>

static uintptr_t execute_syscall(int nr, uintptr_t a0, uintptr_t a1,
uintptr_t a2, uintptr_t a3, uintptr_t a4, uintptr_t a5, uintptr_t a6, uintptr_t a7, uintptr_t a8)
{
switch (nr) {
default:

    return syscall(nr, a0, a1, a2, a3, a4, a5);

}
}

long r[6];

void loop()
{
memset(r, -1, sizeof(r));
r[0] = execute_syscall(__NR_mmap, 0x20000000ul, 0x8f2000ul, 0x3ul,

                         0x32ul, 0xfffffffffffffffful, 0x0ul, 0, 0, 0);
  r[1] = execute_syscall(__NR_socket, 0xaul, 0x5ul, 0x84ul, 0, 0, 0, 0,
                         0, 0);
  (memcpy((void*)0x208f1fd8,
                    "\x62\x63\x73\x68\x30\x00\x00\x00\x00\x00\x00\x00"
                    "\x00\x00\x00\x00",
                    16));
  (*(uint64_t*)0x208f1fe8 = (uint64_t)0x2026b000);
  (memcpy((void*)0x2026b000,
                    "\x15\x00\x00\x00\x6d\x3f\x00\x00\xf4\x78\x5b\x00"
                    "\x00\x00\x06\x00\x41\x00\x00\x00\x05\x00\x11\x00"
                    "\x80\x00\x00\xdf\x22\x68\x8a\x23",
                    32));
  r[5] = execute_syscall(__NR_ioctl, r[1], 0x100000089f0ul,
                         0x208f1fd8ul, 0, 0, 0, 0, 0, 0);
}

int main()
{
loop();
return 0;
}

+syzkaller

This should fix it.

commit 37b3fa4b617681f00cfa1f76d6d7716cc6d9f79a

Author: Alan Cox <[email protected]>
Date:   Wed May 17 21:04:27 2017 +0100

hdlcdrv: Fix division by zero when bitrate is unset

The code attempts to check for out of range calibration. What it forgets to do is check for the 0 bitrate case. As a result the range check itself oopses the kernel.

Found by Andrey Konovalov using Syzkaller.

index 8c3633c..9f34a48 100644
@@ -576,7 +576,7 @@ static int hdlcdrv_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	case HDLCDRVCTL_CALIBRATE:
 		if(!capable(CAP_SYS_RAWIO))
 			return -EPERM;
-		if (bi.data.calibrate > INT_MAX / s->par.bitrate)
+		if (!s->par.bitrate || bi.data.calibrate > INT_MAX / s->par.bitrate)
 			return -EINVAL;
 		s->hdlctx.calibrate = bi.data.calibrate * s->par.bitrate / 16;
 		return 0;

Hi Alan,

Someone else has already sent a couple of versions of a similar fix.

https://patchwork.ozlabs.org/patch/763832/

Thanks!

The divisor s->par.bitrate will always be 0 until initialized by ndo_open() and hdlcdrv_open().

In order to fix this divide zero error, check whether the netdevice was opened by ndo_open() before performing divide.And we also check the the value of bitrate in case of bad setting of it.

---
v1->v2:
Change Reported-by to Andrey Konovalov.
Send it to original report thread.

v0->v1:
Reviewed by walter harms <[email protected]>. Return ENODEV instead of EPERM if !netif_running(dev) Check if s->par.bitrate > 0.

 drivers/net/hamradio/hdlcdrv.c | 4 ++++
 1 file changed, 4 insertions(+)

index 8c3633c..b0f417f 100644
@@ -576,6 +576,10 @@ static int hdlcdrv_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	case HDLCDRVCTL_CALIBRATE:
 		if(!capable(CAP_SYS_RAWIO))
 			return -EPERM;
+		if (!netif_running(dev))
+			return -ENODEV;
+		if (!(s->par.bitrate > 0))
+			return -EINVAL;
 		if (bi.data.calibrate > INT_MAX / s->par.bitrate)
 			return -EINVAL;
 		s->hdlctx.calibrate = bi.data.calibrate * s->par.bitrate / 16;

2.9.4

From: Firo Yang <[email protected]>
Date: Fri, 19 May 2017 21:21:46 +0800

syszkaller fuzzer triggered a divide by zero, when set calibration through ioctl().

To fix it, test 'bitrate' if it is negative or 0, just return -EINVAL.

---
v2->v3:
Just fix divide error and remove netif_running() test. Refine the testing code as David Miller suggested.

v1->v2:
Change Reported-by to Andrey Konovalov.
Send it to original report thread.

v0->v1:
Reviewed by walter harms <[email protected]>. Return ENODEV instead of EPERM if !netif_running(dev) Check if s->par.bitrate > 0.

 drivers/net/hamradio/hdlcdrv.c | 2 ++
 1 file changed, 2 insertions(+)

index 8c3633c..97e3bc6 100644
@@ -576,6 +576,8 @@ static int hdlcdrv_ioctl(struct net_device *dev, struct ifreq *ifr, int cmd)
 	case HDLCDRVCTL_CALIBRATE:
 		if(!capable(CAP_SYS_RAWIO))
 			return -EPERM;
+		if (s->par.bitrate <= 0)
+			return -EINVAL;
 		if (bi.data.calibrate > INT_MAX / s->par.bitrate)
 			return -EINVAL;
 		s->hdlctx.calibrate = bi.data.calibrate * s->par.bitrate / 16;

2.9.4

From: Firo Yang <[email protected]>
Date: Fri, 26 May 2017 22:37:38 +0800

Applied, thank you.