[PATCH] fbdev: pm3fb: Fix potential divide by zero

[Jin D]

Hi Greet,

Thanks for your confirmation and suggestions.

I added this patch based on existing checks on var->pixclock in other
drivers, such as savagefb_check_var, nvidiafb_check_var, etc.
Are you suggesting that it is better to replace an invalid value
(var->pixclock == 0) with a default valid value, instead of returning
-EINVAL? If so, could you advise what a suitable default value would be for
this case?

Actually, I have found a few similar issues in other functions as well. I
would like to make sure I am addressing them in the correct way.

Best,
Alex

On Tue, Jun 10, 2025 at 3:42 AM Geert Uytterhoeven <geert at linux-m68k.org>
wrote:

> Hi Alex,
>
> On Sat, 7 Jun 2025 at 22:14, Alex Guo <alexguo1023 at gmail.com> wrote:
> > variable var->pixclock can be set by user. In case it equals to
> >  zero, divide by zero would occur in pm3fb_check_var. Similar
> > crashes have happened in other fbdev drivers. There is no check
> > and modification on var->pixclock along the call chain to
> > pm3fb_check_var. So we fix this by checking whether 'pixclock'
> > is zero.
> >
> > Similar commit: commit 16844e58704 ("video: fbdev: tridentfb:
> > Error out if 'pixclock' equals zero")
> >
> > Signed-off-by: Alex Guo <alexguo1023 at gmail.com>
>
> Thanks for your patch, which is now commit 59d1fc7b3e1ae9d4
> ("fbdev: pm3fb: fix potential divide by zero") in fbdev/for-next.
>
> > --- a/drivers/video/fbdev/pm3fb.c
> > +++ b/drivers/video/fbdev/pm3fb.c
> > @@ -998,6 +998,9 @@ static int pm3fb_check_var(struct fb_var_screeninfo
> *var, struct fb_info *info)
> >                 return -EINVAL;
> >         }
> >
> > +       if (!var->pixclock)
> > +               return -EINVAL;
>
> While this fixes the crash, this is correct behavior for an fbdev driver.
> When a value is invalid, it should be rounded up to a valid value instead,
> if possible.
>
> > +
> >         if (PICOS2KHZ(var->pixclock) > PM3_MAX_PIXCLOCK) {
> >                 DPRINTK("pixclock too high (%ldKHz)\n",
> >                         PICOS2KHZ(var->pixclock));
>
> Gr{oetje,eeting}s,
>
>                         Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 --
> geert at 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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250611/f98e14d0/attachment.htm>