Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <1635188490-15082-1-git-send-email-george.kennedy@oracle.com>
 <YXcAbXJfg/jKCo5h@kroah.com> <0ddb1c19-64b0-4117-7a92-c3d2fcddfdcf@oracle.com>
 <CAMuHMdWcU7wy1dxa_MXuHoUTWebvq6ShP820Bn_v+8tx7-TEdg@mail.gmail.com>
 <c73402dd-ec3f-4df7-6a16-36fb276a50f4@oracle.com> <CAMuHMdVHt=9N274SG+B7W83JbVVnbz_U5Nkejs_PR2HqvZvJgQ@mail.gmail.com>
 <d2119d59-89e4-34aa-86df-38fee2a0c6ef@oracle.com>
In-Reply-To: <d2119d59-89e4-34aa-86df-38fee2a0c6ef@oracle.com>
From:   Geert Uytterhoeven <geert@linux-m68k.org>
Date:   Tue, 26 Oct 2021 19:12:00 +0200
Message-ID: <CAMuHMdUZq00r7QYeAKGm36UKpGwH_uEGMvwHBzRi9_nxV-dRhg@mail.gmail.com>
Subject: Re: [PATCH] video: fbdev: cirrusfb: check pixclock to avoid divide by zero
To:     George Kennedy <george.kennedy@oracle.com>
Cc:     Greg KH <gregkh@linuxfoundation.org>,
        Thomas Zimmermann <tzimmermann@suse.de>,
        Sam Ravnborg <sam@ravnborg.org>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

Hi George,

On Tue, Oct 26, 2021 at 5:48 PM George Kennedy
<george.kennedy@oracle.com> wrote:
> On 10/26/2021 10:11 AM, Geert Uytterhoeven wrote:
> > On Tue, Oct 26, 2021 at 3:38 PM George Kennedy
> > <george.kennedy@oracle.com> wrote:
> >> On 10/26/2021 4:30 AM, Geert Uytterhoeven wrote:
> >>> On Mon, Oct 25, 2021 at 9:37 PM George Kennedy
> >>> <george.kennedy@oracle.com> wrote:
> >>>> On 10/25/2021 3:07 PM, Greg KH wrote:
> >>>>> On Mon, Oct 25, 2021 at 02:01:30PM -0500, George Kennedy wrote:
> >>>>>> 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 <george.kennedy@oracle.com>
> >>>>>> --- 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