Re: [PATCH 2/2] fbdev: sbuslib: integer overflow in sbusfb_ioctl_helper()

On Mon, Oct 08, 2018 at 12:49:07PM +0200, Bartlomiej Zolnierkiewicz wrote:

On 08/31/2018 10:09 AM, Dan Carpenter wrote:

quoted

The "index + count" addition can overflow.  Both come directly from the
user.  This bug leads to an information leak.

Signed-off-by: Dan Carpenter <redacted>

Patch queued for 4.20, thanks.

quoted

---
Btw, commit 250c6c49e3b6 ("fbdev: Fixing arbitrary kernel leak in case
FBIOGETCMAP_SPARC in sbusfb_ioctl_helper().") doesn't do anything so
far as I can see.  The "cmap->len" variable is type u32, so the
comparison was already unsigned in the original code because of type
promotion.  But the commit was also harmless and nice cleanup.

Both 'index' and 'count' are controlled by user so they could be set to
i.e. -100 and 100 accordingly. Such arguments would pass the 'if' test
(because '+' happens before  type promotion) but still result in leaking
kernel memory (inside 'for' loop).

It's still basically the same when it's unsigned.

Before:  -100 + 100 => 0
 After:  -100U + 100U => 0

The result of the math is still zero.  It's hard to know how to catch
this sort of bug...

regards,
dan carpenter