2021-10-27 21:34:14

by George Kennedy

[permalink] [raw]
Subject: [PATCH v2] video: fbdev: cirrusfb: check pixclock to avoid divide by zero

Do a sanity check on pixclock value to avoid divide by zero.

If the pixclock value is zero, the cirrusfb driver will round up
pixclock to get the derived frequency as close to maxclock as
possible.

Syzkaller reported a divide error in cirrusfb_check_pixclock.

divide error: 0000 [#1] SMP KASAN PTI
CPU: 0 PID: 14938 Comm: cirrusfb_test Not tainted 5.15.0-rc6 #1
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2
RIP: 0010:cirrusfb_check_var+0x6f1/0x1260

Call Trace:
fb_set_var+0x398/0xf90
do_fb_ioctl+0x4b8/0x6f0
fb_ioctl+0xeb/0x130
__x64_sys_ioctl+0x19d/0x220
do_syscall_64+0x3a/0x80
entry_SYSCALL_64_after_hwframe+0x44/0xae

Signed-off-by: George Kennedy <[email protected]>
---
drivers/video/fbdev/cirrusfb.c | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
index 93802ab..3d47c34 100644
--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c
@@ -469,7 +469,7 @@ static int cirrusfb_check_mclk(struct fb_info *info, long freq)
return 0;
}

-static int cirrusfb_check_pixclock(const struct fb_var_screeninfo *var,
+static int cirrusfb_check_pixclock(struct fb_var_screeninfo *var,
struct fb_info *info)
{
long freq;
@@ -478,9 +478,7 @@ static int cirrusfb_check_pixclock(const struct fb_var_screeninfo *var,
unsigned maxclockidx = var->bits_per_pixel >> 3;

/* convert from ps to kHz */
- freq = PICOS2KHZ(var->pixclock);
-
- dev_dbg(info->device, "desired pixclock: %ld kHz\n", freq);
+ freq = PICOS2KHZ(var->pixclock ? : 1);

maxclock = cirrusfb_board_info[cinfo->btype].maxclock[maxclockidx];
cinfo->multiplexing = 0;
@@ -488,11 +486,13 @@ static int cirrusfb_check_pixclock(const struct fb_var_screeninfo *var,
/* If the frequency is greater than we can support, we might be able
* to use multiplexing for the video mode */
if (freq > maxclock) {
- dev_err(info->device,
- "Frequency greater than maxclock (%ld kHz)\n",
- maxclock);
- return -EINVAL;
+ var->pixclock = KHZ2PICOS(maxclock);
+
+ while ((freq = PICOS2KHZ(var->pixclock)) > maxclock)
+ var->pixclock++;
}
+ dev_dbg(info->device, "desired pixclock: %ld kHz\n", freq);
+
/*
* Additional constraint: 8bpp uses DAC clock doubling to allow maximum
* pixel clock
--
1.8.3.1


2021-10-28 08:08:10

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2] video: fbdev: cirrusfb: check pixclock to avoid divide by zero

On Wed, Oct 27, 2021 at 10:32 PM George Kennedy
<[email protected]> wrote:
> Do a sanity check on pixclock value to avoid divide by zero.
>
> If the pixclock value is zero, the cirrusfb driver will round up
> pixclock to get the derived frequency as close to maxclock as
> possible.
>
> Syzkaller reported a divide error in cirrusfb_check_pixclock.
>
> divide error: 0000 [#1] SMP KASAN PTI
> CPU: 0 PID: 14938 Comm: cirrusfb_test Not tainted 5.15.0-rc6 #1
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2
> RIP: 0010:cirrusfb_check_var+0x6f1/0x1260
>
> Call Trace:
> fb_set_var+0x398/0xf90
> do_fb_ioctl+0x4b8/0x6f0
> fb_ioctl+0xeb/0x130
> __x64_sys_ioctl+0x19d/0x220
> do_syscall_64+0x3a/0x80
> entry_SYSCALL_64_after_hwframe+0x44/0xae
>
> Signed-off-by: George Kennedy <[email protected]>

Reviewed-by: Geert Uytterhoeven <[email protected]>

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2021-12-07 19:01:53

by George Kennedy

[permalink] [raw]
Subject: Re: [PATCH v2] video: fbdev: cirrusfb: check pixclock to avoid divide by zero

Can the DRM maintainers accept this Reviewed by patch?

Links to the Reviewed by patch:

https://lkml.org/lkml/2021/10/27/982
https://lore.kernel.org/all/[email protected]/


Thank you,
George


On 10/28/2021 4:05 AM, Geert Uytterhoeven wrote:
> On Wed, Oct 27, 2021 at 10:32 PM George Kennedy
> <[email protected]> wrote:
>> Do a sanity check on pixclock value to avoid divide by zero.
>>
>> If the pixclock value is zero, the cirrusfb driver will round up
>> pixclock to get the derived frequency as close to maxclock as
>> possible.
>>
>> Syzkaller reported a divide error in cirrusfb_check_pixclock.
>>
>> divide error: 0000 [#1] SMP KASAN PTI
>> CPU: 0 PID: 14938 Comm: cirrusfb_test Not tainted 5.15.0-rc6 #1
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2
>> RIP: 0010:cirrusfb_check_var+0x6f1/0x1260
>>
>> Call Trace:
>> fb_set_var+0x398/0xf90
>> do_fb_ioctl+0x4b8/0x6f0
>> fb_ioctl+0xeb/0x130
>> __x64_sys_ioctl+0x19d/0x220
>> do_syscall_64+0x3a/0x80
>> entry_SYSCALL_64_after_hwframe+0x44/0xae
>>
>> Signed-off-by: George Kennedy <[email protected]>
> Reviewed-by: Geert Uytterhoeven <[email protected]>
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds