On Sun, Apr 3, 2022, 23:02 Helge Deller <deller@xxxxxx> wrote:
On 4/3/22 13:26, Zheyu Ma wrote:
> Hi,
>
> I found a bug in the function i740fb_set_par().
Nice catch!
> When the user calls the ioctl system call without setting the value to
> 'var->pixclock', the driver will throw a divide error.
>
> This bug occurs because the driver uses the value of 'var->pixclock'
> without checking it, as the following code snippet show:
>
> if ((1000000 / var->pixclock) > DACSPEED8) {
> dev_err(info->device, "requested pixclock %i MHz out of range
> (max. %i MHz at 8bpp)\n",
> 1000000 / var->pixclock, DACSPEED8);
> return -EINVAL;x
> }
>
> We can fix this by checking the value of 'var->pixclock' in the
> function i740fb_check_var() similar to commit
> b36b242d4b8ea178f7fd038965e3cac7f30c3f09, or we should set the lowest
> supported value when this field is zero.
> I have no idea about which solution is better.
Me neither.
I think a solution like commit b36b242d4b8ea178f7fd038965e3cac7f30c3f09
is sufficient.
Note that i740fb_set_par() is called in i740fb_resume() as well.
Since this doesn't comes form userspace I think adding a check for
the return value there isn't necessary.
Would you mind sending a patch like b36b242d4b8ea178f7fd038965e3cac7f30c3f09 ?
OK, and I also found similar bugs in other framebuffer drivers, I will send a patch set for them soon.
Zheyu Ma