On Mon, Oct 08, 2018 at 12:49:07PM +0200, Bartlomiej Zolnierkiewicz wrote:
>
> On 08/31/2018 10:09 AM, Dan Carpenter wrote:
> > The "index + count" addition can overflow. Both come directly from the
> > user. This bug leads to an information leak.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
>
> Patch queued for 4.20, thanks.
>
> > ---
> > 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
