On Thu, Aug 17, 2017 at 07:16:03AM -0700, Jason Ekstrand wrote: > On Thu, Aug 17, 2017 at 2:56 AM, Imre Deak <imre.deak@xxxxxxxxx> wrote: > > > On Thu, Aug 17, 2017 at 12:50:37PM +0300, Dan Carpenter wrote: > > > On Thu, Aug 17, 2017 at 12:37:00PM +0300, Imre Deak wrote: > > > > On Thu, Aug 17, 2017 at 09:23:10AM +0300, Dan Carpenter wrote: > > > > > There are some potential integer overflows here on 64 bit systems. > > > > > > > > > > The condition "if (nfences > SIZE_MAX / sizeof(*fences))" can only be > > > > > true on 32 bit systems, it's a no-op on 64 bit, so let's ignore the > > > > > check for now and look a couple lines after: > > > > > > > > > > if (!access_ok(VERIFY_READ, user, nfences * 2 * sizeof(u32))) > > > > > ^^^^^^^^^^^ > > > > > "nfences" is an unsigned int, so if we set it to UINT_MAX and > > multiply > > > > > by two, it's going to have an integer overflow. > > > > > > > > AFAICS it wouldn't overflow due the promotion to unsigned long > > > > by '* sizeof(u32)'. > > > > > > > > > > It first multplies "nfences * 2" as unsigned int, then it type promotes > > > to size_t and multiplies by sizeof(). Only the first multiplication has > > > an integer overflow bug. > > > > Err, that's correct. Sorry for the noise. > > > > Why not just replace the "2 * sizeof(u32)" with a "sizeof(*user)". That's > what we really want to check. I have no idea how it ended up being "2 * > sizeof(u32)" Yeah. That's more readable. I will resend. regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
