<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Aug 17, 2017 at 2:56 AM, Imre Deak <span dir="ltr"><<a href="mailto:imre.deak@intel.com" target="_blank">imre.deak@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Thu, Aug 17, 2017 at 12:50:37PM +0300, Dan Carpenter wrote:<br>
> On Thu, Aug 17, 2017 at 12:37:00PM +0300, Imre Deak wrote:<br>
> > On Thu, Aug 17, 2017 at 09:23:10AM +0300, Dan Carpenter wrote:<br>
> > > There are some potential integer overflows here on 64 bit systems.<br>
> > ><br>
> > > The condition "if (nfences > SIZE_MAX / sizeof(*fences))" can only be<br>
> > > true on 32 bit systems, it's a no-op on 64 bit, so let's ignore the<br>
> > > check for now and look a couple lines after:<br>
> > ><br>
> > >   if (!access_ok(VERIFY_READ, user, nfences * 2 * sizeof(u32)))<br>
> > >                                           ^^^^^^^^^^^<br>
> > > "nfences" is an unsigned int, so if we set it to UINT_MAX and multiply<br>
> > > by two, it's going to have an integer overflow.<br>
> ><br>
> > AFAICS it wouldn't overflow due the promotion to unsigned long<br>
> > by '* sizeof(u32)'.<br>
> ><br>
><br>
> It first multplies "nfences * 2" as unsigned int, then it type promotes<br>
> to size_t and multiplies by sizeof().  Only the first multiplication has<br>
> an integer overflow bug.<br>
<br>
</span>Err, that's correct. Sorry for the noise.<br></blockquote><div><br></div><div>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)"</div><div><br></div><div>--Jason<br></div></div><br></div></div>