<div dir="ltr"><div>Hi Greet,</div><div><br></div><div>Thanks for your confirmation and suggestions.<br><br>I added this patch based on existing checks on var->pixclock in other drivers, such as savagefb_check_var, nvidiafb_check_var, etc.<br>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?<br><br>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.<br><br>Best,<br>Alex</div><br><div class="gmail_quote gmail_quote_container"><div dir="ltr" class="gmail_attr">On Tue, Jun 10, 2025 at 3:42 AM Geert Uytterhoeven <<a href="mailto:geert@linux-m68k.org">geert@linux-m68k.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Alex,<br>
<br>
On Sat, 7 Jun 2025 at 22:14, Alex Guo <<a href="mailto:alexguo1023@gmail.com" target="_blank">alexguo1023@gmail.com</a>> wrote:<br>
> variable var->pixclock can be set by user. In case it equals to<br>
>  zero, divide by zero would occur in pm3fb_check_var. Similar<br>
> crashes have happened in other fbdev drivers. There is no check<br>
> and modification on var->pixclock along the call chain to<br>
> pm3fb_check_var. So we fix this by checking whether 'pixclock'<br>
> is zero.<br>
><br>
> Similar commit: commit 16844e58704 ("video: fbdev: tridentfb:<br>
> Error out if 'pixclock' equals zero")<br>
><br>
> Signed-off-by: Alex Guo <<a href="mailto:alexguo1023@gmail.com" target="_blank">alexguo1023@gmail.com</a>><br>
<br>
Thanks for your patch, which is now commit 59d1fc7b3e1ae9d4<br>
("fbdev: pm3fb: fix potential divide by zero") in fbdev/for-next.<br>
<br>
> --- a/drivers/video/fbdev/pm3fb.c<br>
> +++ b/drivers/video/fbdev/pm3fb.c<br>
> @@ -998,6 +998,9 @@ static int pm3fb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)<br>
>                 return -EINVAL;<br>
>         }<br>
><br>
> +       if (!var->pixclock)<br>
> +               return -EINVAL;<br>
<br>
While this fixes the crash, this is correct behavior for an fbdev driver.<br>
When a value is invalid, it should be rounded up to a valid value instead,<br>
if possible.<br>
<br>
> +<br>
>         if (PICOS2KHZ(var->pixclock) > PM3_MAX_PIXCLOCK) {<br>
>                 DPRINTK("pixclock too high (%ldKHz)\n",<br>
>                         PICOS2KHZ(var->pixclock));<br>
<br>
Gr{oetje,eeting}s,<br>
<br>
                        Geert<br>
<br>
-- <br>
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- <a href="mailto:geert@linux-m68k.org" target="_blank">geert@linux-m68k.org</a><br>
<br>
In personal conversations with technical people, I call myself a hacker. But<br>
when I'm talking to journalists I just say "programmer" or something like that.<br>
                                -- Linus Torvalds<br>
</blockquote></div></div>