Hi George,

On Tue, Oct 26, 2021 at 5:48 PM George Kennedy
[off-list ref] wrote:

On 10/26/2021 10:11 AM, Geert Uytterhoeven wrote:

quoted

On Tue, Oct 26, 2021 at 3:38 PM George Kennedy
[off-list ref] wrote:

quoted

On 10/26/2021 4:30 AM, Geert Uytterhoeven wrote:

quoted

On Mon, Oct 25, 2021 at 9:37 PM George Kennedy
[off-list ref] wrote:

quoted

On 10/25/2021 3:07 PM, Greg KH wrote:

quoted

On Mon, Oct 25, 2021 at 02:01:30PM -0500, George Kennedy wrote:

quoted

Do a sanity check on pixclock value before using it as a divisor.

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 <redacted>

--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c

@@ -477,6 +477,9 @@ static int cirrusfb_check_pixclock(const struct fb_var_screeninfo *var,
       struct cirrusfb_info *cinfo = info->par;
       unsigned maxclockidx = var->bits_per_pixel >> 3;

+    if (!var->pixclock)
+            return -EINVAL;

This is not correct: fbdev drivers should round up invalid values,
and only return an error if rounding up cannot yield a valid value.

What default value would you recommend? Here are examples of some of the
possible cirrusfb pixclock values:
40000: 25MHz
20000: 50Mhz
12500: 80Mhz

You should pick the lowest supported value.

In bestclock() the frequency value ("freq") is not allowed to go below 8000.

         if (freq < 8000)
                 freq = 8000;

If pixclock is passed in as zero to cirrusfb_check_pixclock(), is it ok
to then set the value of pixclock to 125000, which will result in "freq"
being set to 8000 (or adjust the passed in pixclock value to make sure
"freq" does not get below 8000)?

No, clock rate is the inverse of clock period.
So the smallest clock period (fb_var_screeninfo.pixclock) corresponds
to the largest clock rate (freq in bestclock()).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

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