Re: [BUG] video: fbdev: arkfb: Found a divide-by-zero bug which may cause DoS

[Helge Deller <deller@gmx.de>]

* Zheyu Ma <zheyuma97@gmail.com>:
> I found a bug in the arkfb driver in the latest kernel, which may cause DoS.
>
> The reason for this bug is that the user controls some input to ioctl,
> making 'mode' 0x7 on line 704, which causes hdiv = 1, hmul = 2, and if
> the pixclock is controlled to be 1, it will cause a division error in
> the function ark_set_pixclock().

You are right.
I see in:
  drivers/video/fbdev/arkfb.c:784: ark_set_pixclock(info, (hdiv * info->var.pixclock) / hmul);
with hdiv=1, pixclock=1 and hmul=2 you end up with (1*1)/2 = (int) 0.
and then in
  drivers/video/fbdev/arkfb.c:504: rv = dac_set_freq(par->dac, 0, 1000000000 / pixclock);
you'll get a division-by-zero.

> The easiest patch is to check the value of the argument 'pixclock' in
> the ark_set_pixclock function, but this is perhaps too late, should we
> do this check earlier? I'm not sure, so I'll report this bug to you.

Yes, I think it should be done earlier.

Geert always mentioned that an invalid pixclock from userspace should be
rounded up to the next valid pixclock.
But since I don't have that hardware, I'm not sure how this can be done
best for this driver.

Do you have the hardware to test?
If so, could you check the patch below?
It should at least prevent the division-by-zero.
If it works, I'm happy if you could send a final patch...

Helge

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index eb3e47c58c5f..ed76ddc7df3d 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -781,7 +781,12 @@ static int arkfb_set_par(struct fb_info *info)
 		return -EINVAL;
 	}

-	ark_set_pixclock(info, (hdiv * info->var.pixclock) / hmul);
+	value = (hdiv * info->var.pixclock) / hmul;
+	if (!value) {
+		fb_dbg(info, "invalid pixclock\n");
+		value = 1;
+	}
+	ark_set_pixclock(info, value);
 	svga_set_timings(par->state.vgabase, &ark_timing_regs, &(info->var), hmul, hdiv,
 			 (info->var.vmode & FB_VMODE_DOUBLE)     ? 2 : 1,
 			 (info->var.vmode & FB_VMODE_INTERLACED) ? 2 : 1,