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 <dan.carpenter@xxxxxxxxxx> > > > > 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 dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
