From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.93; helo=mga11.intel.com; envelope-from=ruiyu.ni@intel.com; receiver=edk2-devel@lists.01.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 49F0621F833A3 for ; Mon, 8 Jan 2018 19:40:40 -0800 (PST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by fmsmga102.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 08 Jan 2018 19:45:50 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,334,1511856000"; d="scan'208";a="193298418" Received: from ray-dev.ccr.corp.intel.com (HELO [10.239.9.19]) ([10.239.9.19]) by fmsmga005.fm.intel.com with ESMTP; 08 Jan 2018 19:45:49 -0800 To: edk2-devel@lists.01.org References: <1515466566-13136-1-git-send-email-jiaxin.wu@intel.com> <1515466566-13136-2-git-send-email-jiaxin.wu@intel.com> From: "Ni, Ruiyu" Message-ID: Date: Tue, 9 Jan 2018 11:45:49 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:52.0) Gecko/20100101 Thunderbird/52.5.2 MIME-Version: 1.0 In-Reply-To: <1515466566-13136-2-git-send-email-jiaxin.wu@intel.com> Subject: Re: [Patch 1/2] MdeModulePkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it. X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Jan 2018 03:40:40 -0000 Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit On 1/9/2018 10:56 AM, Jiaxin Wu wrote: > There are two place to close the ISCSI ExitBootServiceEvent: > One is IScsiOnExitBootService callback function. > Another is ISCSI driver stop() function. > > When OS loader triggers ExitBootServiceEvent, firstly, the exit boot service > callback function will close and free the ExitBootServiceEvent, then secondly > the system will call ISCSI driver stop() function, the ExitBootServiceEvent > will be closed and freed again, the use-after-free memory access happens. > > This issue is recorded at https://bugzilla.tianocore.org/show_bug.cgi?id=742. > This patch is to resolve the issue. > > Cc: Ye Ting > Cc: Fu Siyuan > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Wu Jiaxin > --- > MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c > index ae202c3..29dfe94 100644 > --- a/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c > +++ b/MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c > @@ -1,9 +1,9 @@ > /** @file > Miscellaneous routines for iSCSI driver. > > -Copyright (c) 2004 - 2016, Intel Corporation. All rights reserved.
> +Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.
> This program and the accompanying materials > are licensed and made available under the terms and conditions of the BSD License > which accompanies this distribution. The full text of the license may be found at > http://opensource.org/licenses/bsd-license.php > > @@ -622,13 +622,14 @@ IScsiCleanDriverData ( > &Private->IScsiExtScsiPassThru > ); > } > > EXIT: > - > - gBS->CloseEvent (Private->ExitBootServiceEvent); > - > + if (Private->ExitBootServiceEvent != NULL) { > + gBS->CloseEvent (Private->ExitBootServiceEvent); > + } > + > FreePool (Private); > return Status; > } > > /** > @@ -870,12 +871,15 @@ IScsiOnExitBootService ( > ) > { > ISCSI_DRIVER_DATA *Private; > > Private = (ISCSI_DRIVER_DATA *) Context; > + > gBS->CloseEvent (Private->ExitBootServiceEvent); > > + Private->ExitBootServiceEvent = NULL; > + > IScsiSessionAbort (&Private->Session); > } > > /** > Tests whether a controller handle is being managed by IScsi driver. > Jiaxin, I agree your patch can fix the use-after-free issue in your network driver. But I think a more root fix should be in CoreCloseEvent(). You could refer to CoreValidateHandle(). It looks for the Handle in handle database for a match before deferencing it. -- Thanks, Ray