From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752832AbcD0FmJ (ORCPT ); Wed, 27 Apr 2016 01:42:09 -0400 Received: from mail2-relais-roc.national.inria.fr ([192.134.164.83]:20789 "EHLO mail2-relais-roc.national.inria.fr" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752074AbcD0FmH (ORCPT ); Wed, 27 Apr 2016 01:42:07 -0400 X-IronPort-AV: E=Sophos;i="5.24,540,1454972400"; d="scan'208";a="215978564" Date: Wed, 27 Apr 2016 07:42:04 +0200 (CEST) From: Julia Lawall X-X-Sender: jll@localhost6.localdomain6 To: Kees Cook cc: Pengfei Wang , "security@kernel.org" , LKML Subject: Re: Double-Fetch bug in Linux-4.5/drivers/scsi/aacraid/commctrl.c In-Reply-To: Message-ID: References: <0484FFD3-4BAB-43B9-AD56-B4A098C3E8AE@gmail.com> User-Agent: Alpine 2.02 (DEB 1266 2009-07-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, 26 Apr 2016, Kees Cook wrote: > On Mon, Apr 25, 2016 at 7:50 AM, Pengfei Wang 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? thanks, julia > virtual report > virtual org > > @cfu_twice@ > position p; > identifier src; > expression dest1, dest2, size1, size2, offset; > @@ > > *copy_from_user(dest1, src, size1) > ... when != src = offset > when != src += offset > *copy_from_user@p(dest2, src, size2) > > @script:python depends on org@ > p << cfu_twice.p; > @@ > > cocci.print_main("potentially dangerous second copy_from_user()",p) > > @script:python depends on report@ > p << cfu_twice.p; > @@ > > coccilib.report.print_report(p[0],"potentially dangerous second > copy_from_user()") > > > It would be great to have some one go through all the reports to see > which are legit. I'll send separate emails with the patch for > coccicheck and the output. > > -Kees > > > > > In function ioctl_send_fib(), the driver fetches user space data by pointer > > arg via copy_from_user(), and this happens twice at line 81 and line 116 > > respectively. The first fetched value (stored in kfib) is used to get the > > header and calculate the size at line 90 so as to copy the whole message > > later at line 116, which means the copy size of the whole message is based > > on the old value that came from the first fetch. Besides, the whole message > > copied in the second fetch also contains the header. > > > > However, when the function processes the message after the second fetch at > > line 130, it uses kfib->header.Size that came from the second fetch, which > > might be different from the one came from the first fetch as well as > > calculated the size to copy the message from user space to driver. > > > > If the kfib->header.Size is modified by a user thread under race condition > > between the fetch operations, for example changing to a very large value, > > this will lead to over-boundary access or other serious consequences in > > function aac_fib_send(). > > > > I also reported this to bugzilla, > > https://bugzilla.kernel.org/show_bug.cgi?id=116751 > > > > I am expecting a reply to confirm this, thank you! > > > > > > > > > > > > Kind regards > > Pengfei > > > > > > -- > Kees Cook > Chrome OS & Brillo Security >