Hi Zheyu,
On Wed, Jul 21, 2021 at 12:43:44PM +0000, Zheyu Ma wrote:
> The userspace program could pass any values to the driver through
> ioctl() interface. If the driver doesn't check the value of 'pixclock',
> it may cause divide error because of the 'PICOS2KHZ' macro.
>
> Fix this by checking whether 'pixclock' is zero first.
>
> The following log reveals it:
>
> [ 53.093806] divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> [ 53.093838] CPU: 3 PID: 11763 Comm: hang Not tainted 5.14.0-rc2-00478-g2734d6c1b1a0 #215
> [ 53.093859] RIP: 0010:neofb_check_var+0x80/0xe50
> [ 53.093951] Call Trace:
> [ 53.093956] ? neofb_setcolreg+0x2b0/0x2b0
> [ 53.093968] fb_set_var+0x2e4/0xeb0
> [ 53.093977] ? fb_blank+0x1a0/0x1a0
> [ 53.093984] ? lock_acquire+0x1ef/0x530
> [ 53.093996] ? lock_release+0x810/0x810
> [ 53.094005] ? lock_is_held_type+0x100/0x140
> [ 53.094016] ? ___might_sleep+0x1ee/0x2d0
> [ 53.094028] ? __mutex_lock+0x620/0x1190
> [ 53.094036] ? do_fb_ioctl+0x313/0x700
> [ 53.094044] ? mutex_lock_io_nested+0xfa0/0xfa0
> [ 53.094051] ? __this_cpu_preempt_check+0x1d/0x30
> [ 53.094060] ? _raw_spin_unlock_irqrestore+0x46/0x60
> [ 53.094069] ? lockdep_hardirqs_on+0x59/0x100
> [ 53.094076] ? _raw_spin_unlock_irqrestore+0x46/0x60
> [ 53.094085] ? trace_hardirqs_on+0x6a/0x1c0
> [ 53.094096] do_fb_ioctl+0x31e/0x700
>
> Signed-off-by: Zheyu Ma <zheyuma97@xxxxxxxxx>
I looked if we could move this check up to the caller, but it seems
better to keep it per driver.
Added the patch to drm-misc-next, it will appera in -next in around one
week.
Sam
> ---
> drivers/video/fbdev/neofb.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/neofb.c b/drivers/video/fbdev/neofb.c
> index c0f4f402da3f..966df2a07360 100644
> --- a/drivers/video/fbdev/neofb.c
> +++ b/drivers/video/fbdev/neofb.c
> @@ -585,7 +585,7 @@ neofb_check_var(struct fb_var_screeninfo *var, struct fb_info *info)
>
> DBG("neofb_check_var");
>
> - if (PICOS2KHZ(var->pixclock) > par->maxClock)
> + if (var->pixclock && PICOS2KHZ(var->pixclock) > par->maxClock)
> return -EINVAL;
>
> /* Is the mode larger than the LCD panel? */
> --
> 2.17.6
