Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 1 Aug 2022 06:34:26 +0200
From:   Helge Deller <deller@gmx.de>
To:     Zheyu Ma <zheyuma97@gmail.com>,
        Geert Uytterhoeven <geert@linux-m68k.org>
Cc:     Helge Deller <deller@gmx.de>,
        Linux Fbdev development list <linux-fbdev@vger.kernel.org>,
        DRI Development <dri-devel@lists.freedesktop.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Re: [BUG] video: fbdev: arkfb: Found a divide-by-zero bug which may
 cause DoS
Message-ID: <YudX0t/P94a0LKtr@ls3530>
References: <CAMhUBjkps_2EAkbCpGuLiWVFObLkLuj=3UqbxcuENUNXMkbS9Q@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAMhUBjkps_2EAkbCpGuLiWVFObLkLuj=3UqbxcuENUNXMkbS9Q@mail.gmail.com>
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

* 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 =3D 1, hmul =3D 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->va=
r.pixclock) / hmul);
with hdiv=3D1, pixclock=3D1 and hmul=3D2 you end up with (1*1)/2 =3D (int)=
 0.
and then in
  drivers/video/fbdev/arkfb.c:504: rv =3D dac_set_freq(par->dac, 0, 100000=
0000 / 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
=2D-- 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 =3D (hdiv * info->var.pixclock) / hmul;
+	if (!value) {
+		fb_dbg(info, "invalid pixclock\n");
+		value =3D 1;
+	}
+	ark_set_pixclock(info, value);
 	svga_set_timings(par->state.vgabase, &ark_timing_regs, &(info->var), hmu=
l, hdiv,
 			 (info->var.vmode & FB_VMODE_DOUBLE)     ? 2 : 1,
 			 (info->var.vmode & FB_VMODE_INTERLACED) ? 2 : 1,