Quoting Dan Carpenter (2018-05-16 16:15:54) > On Wed, May 16, 2018 at 03:56:55PM +0100, Chris Wilson wrote: > > 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 > > > > No, no. That's not the problem. The problem is the - 1 subtraction. I > explained poorly. The issue is when "bpp" is U32_MAX - 7. The define > looks like: > > #define __KERNEL_DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d)) > > So "n" is (U32_MAX - 7) and "d" is 8. "(n) + (d)" wraps to zero. We do > the "- 1" subtraction so that gives us U32_MAX. And finally we divide > by "d" to get "U32_MAX / 8". D'oh. Thanks for spelling it out. -Chris _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
