Quoting Dan Carpenter (2018-05-16 15:52:57)
> On Wed, May 16, 2018 at 03:26:07PM +0100, Chris Wilson wrote:
> > Quoting Dan Carpenter (2018-05-16 15:00:26)
> > > There is a comment here which says that DIV_ROUND_UP() and that's where
> > > the problem comes from. Say you pick:
> > >
> > > args->bpp = UINT_MAX - 7;
> > > args->width = 4;
> > > args->height = 1;
> > >
> > > The integer overflow in DIV_ROUND_UP() means "cpp" is UINT_MAX / 8 and
> > > because of how we picked args->width that means cpp < UINT_MAX / 4.
> > >
> > > I've fixed it by preventing the integer overflow in DIV_ROUND_UP(). I
> > > removed the check for !cpp because it's not possible after this change.
> > > I also changed all the 0xffffffffU references to U32_MAX.
> > >
> > > Signed-off-by: Dan Carpenter <[email protected]>
> >
> > I agree with Daniel that the !cpp check after DIV_ROUND_UP was
> > sufficient to guard the current code, but switching to a more idiomatic
> > style of overflow checking has its benefits too. Plus I like having
> > U32_MAX to compare the type ranges against.
> >
>
> I'm not totally sure what it means to say that the integer overflow is
> sufficient. The overflow check is definitely buggy but if you mean that
> it isn't exploitable then you're probably right. Anyway, let's say you
> use use the values I provided in my changelog. Then I believe we can
> reach vgem_gem_dumb_create():
But we are talking about
cpp = (U32_MAX + 8) / 8
which is 0. So the !cpp does catch the overflow.
Or am I completely off in unsigned wraparound?
-Chris
_______________________________________________
dri-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/dri-devel
