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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YudX0t/P94a0LKtr@ls3530>
Date:   Mon, 1 Aug 2022 06:34:26 +0200
From:   Helge Deller <deller@....de>
To:     Zheyu Ma <zheyuma97@...il.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>
Cc:     Helge Deller <deller@....de>,
        Linux Fbdev development list <linux-fbdev@...r.kernel.org>,
        DRI Development <dri-devel@...ts.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [BUG] video: fbdev: arkfb: Found a divide-by-zero bug which may
 cause DoS

* Zheyu Ma <zheyuma97@...il.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,