On Wed, 27 Apr 2016, Dan Carpenter wrote:
> On Wed, Apr 27, 2016 at 07:42:04AM +0200, Julia Lawall wrote: > > > > > > On Tue, 26 Apr 2016, Kees Cook wrote: > > > > > On Mon, Apr 25, 2016 at 7:50 AM, Pengfei Wang <[email protected]> > > > wrote: > > > > Hello, > > > > > > > > I found this Double-Fetch bug in > > > > Linux-4.5/drivers/scsi/aacraid/commctrl.c > > > > when I was examining the source code. > > > > > > Thanks for these reports! I wrote a coccinelle script to find these, > > > but it requires some manual checking. For what it's worth, it found > > > your report as well: > > > > > > ./drivers/scsi/aacraid/commctrl.c:116:5-19: potentially dangerous > > > second copy_from_user() > > > > > > So I should probably get this added to the coccicheck run... Maybe it > > > can get some clean up from Julia. :) > > > > I looked a bit at the results, and didn't see anything obvious. What is > > the problem, exactly, and what would be a characteristic of a false > > positive? > > > > > copy_from_user(dest, src, sizeof(dest)); > > if (dest.extra > MAX_SIZE) > return -EINVAL; > > copy_from_user(dest, src, sizeof(dest) + dest.extra); > > for (i = 0; i < dest.extra; i++) { > dest.foo[i] = xxx; > > > We get dest.extra from the user, we verify the size, then we copy more > data from the user but that over writes dest.extra again. We use > dest.extra a second time without checking that it's still <= MAX_SIZE. OK, so the problem is when data that was checked on the first copy is used after the second copy? It would probably be possible to get rid of a lot of false positives with that. thanks, julia
