[PATCH] hpet: Fix division by zero in hpet_time_div()

[PATCH] hpet: Fix division by zero in hpet_time_div()

[Kefeng Wang]

The base value in do_div() called by hpet_time_div() is truncated from
unsigned long to uint32_t, resulting in a divide-by-zero exception.

UBSAN: Undefined behaviour in ../drivers/char/hpet.c:572:2
division by zero
CPU: 1 PID: 23682 Comm: syz-executor.3 Not tainted 4.4.184.x86_64+ #4
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
 0000000000000000 b573382df1853d00 ffff8800a3287b98 ffffffff81ad7561
 ffff8800a3287c00 ffffffff838b35b0 ffffffff838b3860 ffff8800a3287c20
 0000000000000000 ffff8800a3287bb0 ffffffff81b8f25e ffffffff838b35a0
Call Trace:
 [<ffffffff81ad7561>] __dump_stack lib/dump_stack.c:15 [inline]
 [<ffffffff81ad7561>] dump_stack+0xc1/0x120 lib/dump_stack.c:51
 [<ffffffff81b8f25e>] ubsan_epilogue+0x12/0x8d lib/ubsan.c:166
 [<ffffffff81b900cb>] __ubsan_handle_divrem_overflow+0x282/0x2c8 lib/ubsan.c:262
 [<ffffffff823560dd>] hpet_time_div drivers/char/hpet.c:572 [inline]
 [<ffffffff823560dd>] hpet_ioctl_common drivers/char/hpet.c:663 [inline]
 [<ffffffff823560dd>] hpet_ioctl_common.cold+0xa8/0xad drivers/char/hpet.c:577
 [<ffffffff81e63d56>] hpet_ioctl+0xc6/0x180 drivers/char/hpet.c:676
 [<ffffffff81711590>] vfs_ioctl fs/ioctl.c:43 [inline]
 [<ffffffff81711590>] file_ioctl fs/ioctl.c:470 [inline]
 [<ffffffff81711590>] do_vfs_ioctl+0x6e0/0xf70 fs/ioctl.c:605
 [<ffffffff81711eb4>] SYSC_ioctl fs/ioctl.c:622 [inline]
 [<ffffffff81711eb4>] SyS_ioctl+0x94/0xc0 fs/ioctl.c:613
 [<ffffffff82846003>] tracesys_phase2+0x90/0x95

The main C reproducer autogenerated by syzkaller,

  syscall(__NR_mmap, 0x20000000, 0x1000000, 3, 0x32, -1, 0);
  memcpy((void*)0x20000100, "/dev/hpet\000", 10);
  syscall(__NR_openat, 0xffffffffffffff9c, 0x20000100, 0, 0);
  syscall(__NR_ioctl, r[0], 0x40086806, 0x40000000000000);

Fix it by using div64_ul().

Signed-off-by: Kefeng Wang <wangkefeng.wang@huawei.com>
Signed-off-by: Zhang HongJun <zhanghongjun2@huawei.com>
---
 drivers/char/hpet.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/char/hpet.c b/drivers/char/hpet.c
index 5c39f20378b8..dd52b63b62db 100644
--- a/drivers/char/hpet.c
+++ b/drivers/char/hpet.c
@@ -567,7 +567,7 @@ static inline unsigned long hpet_time_div(struct hpets *hpets,
 	unsigned long long m;
 
 	m = hpets->hp_tick_freq + (dis >> 1);
-	do_div(m, dis);
+	div64_ul(m, dis);
 	return (unsigned long)m;
 }
 
-- 
2.20.1

Re: [PATCH] hpet: Fix division by zero in hpet_time_div()

[Arnd Bergmann]

On Thu, Jul 11, 2019 at 1:20 PM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
> The base value in do_div() called by hpet_time_div() is truncated from
> unsigned long to uint32_t, resulting in a divide-by-zero exception.

Good catch!

> --- a/drivers/char/hpet.c
> +++ b/drivers/char/hpet.c
> @@ -567,7 +567,7 @@ static inline unsigned long hpet_time_div(struct hpets *hpets,
>         unsigned long long m;
>
>         m = hpets->hp_tick_freq + (dis >> 1);
> -       do_div(m, dis);
> +       div64_ul(m, dis);
>         return (unsigned long)m;
>  }

This still looks wrong to me: div64_ul() unlike do_div() does not
modify its argument, so you have to assign the output like

       return div64_ul(m, dis);

       Arnd

Re: [PATCH] hpet: Fix division by zero in hpet_time_div()

[Kefeng Wang]

On 2019/7/11 20:32, Arnd Bergmann wrote:
> On Thu, Jul 11, 2019 at 1:20 PM Kefeng Wang <wangkefeng.wang@huawei.com> wrote:
>> The base value in do_div() called by hpet_time_div() is truncated from
>> unsigned long to uint32_t, resulting in a divide-by-zero exception.
> 
> Good catch!
> 
>> --- a/drivers/char/hpet.c
>> +++ b/drivers/char/hpet.c
>> @@ -567,7 +567,7 @@ static inline unsigned long hpet_time_div(struct hpets *hpets,
>>         unsigned long long m;
>>
>>         m = hpets->hp_tick_freq + (dis >> 1);
>> -       do_div(m, dis);
>> +       div64_ul(m, dis);
>>         return (unsigned long)m;
>>  }
> 
> This still looks wrong to me: div64_ul() unlike do_div() does not
> modify its argument, so you have to assign the output like
> 
>        return div64_ul(m, dis);

right, should check div64_ul more carefully， will resend v2, thanks

> 
>        Arnd
> 
>