* [Patch 1/2] MdeModulePkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.
2018-01-09 2:56 [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after close it Jiaxin Wu
@ 2018-01-09 2:56 ` Jiaxin Wu
2018-01-09 3:45 ` Ni, Ruiyu
2018-01-09 2:56 ` [Patch 2/2] NetworkPkg/IScsiDxe: " Jiaxin Wu
2018-01-09 6:14 ` [Patch 0/2] IScsiDxe: " Fu, Siyuan
2 siblings, 1 reply; 5+ messages in thread
From: Jiaxin Wu @ 2018-01-09 2:56 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Wu Jiaxin
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 <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
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.<BR>
+Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
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.
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Patch 1/2] MdeModulePkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.
2018-01-09 2:56 ` [Patch 1/2] MdeModulePkg/IScsiDxe: " Jiaxin Wu
@ 2018-01-09 3:45 ` Ni, Ruiyu
0 siblings, 0 replies; 5+ messages in thread
From: Ni, Ruiyu @ 2018-01-09 3:45 UTC (permalink / raw)
To: edk2-devel
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 <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
> ---
> 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.<BR>
> +Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [Patch 2/2] NetworkPkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.
2018-01-09 2:56 [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after close it Jiaxin Wu
2018-01-09 2:56 ` [Patch 1/2] MdeModulePkg/IScsiDxe: " Jiaxin Wu
@ 2018-01-09 2:56 ` Jiaxin Wu
2018-01-09 6:14 ` [Patch 0/2] IScsiDxe: " Fu, Siyuan
2 siblings, 0 replies; 5+ messages in thread
From: Jiaxin Wu @ 2018-01-09 2:56 UTC (permalink / raw)
To: edk2-devel; +Cc: Ye Ting, Fu Siyuan, Wu Jiaxin
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 <ting.ye@intel.com>
Cc: Fu Siyuan <siyuan.fu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
---
NetworkPkg/IScsiDxe/IScsiMisc.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/NetworkPkg/IScsiDxe/IScsiMisc.c b/NetworkPkg/IScsiDxe/IScsiMisc.c
index 9e4164c..9b26147 100644
--- a/NetworkPkg/IScsiDxe/IScsiMisc.c
+++ b/NetworkPkg/IScsiDxe/IScsiMisc.c
@@ -1,9 +1,9 @@
/** @file
Miscellaneous routines for iSCSI driver.
-Copyright (c) 2004 - 2017, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2004 - 2018, Intel Corporation. All rights reserved.<BR>
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
@@ -1796,12 +1796,13 @@ IScsiCleanDriverData (
mPrivate->OneSessionEstablished = FALSE;
}
}
EXIT:
-
- gBS->CloseEvent (Private->ExitBootServiceEvent);
+ if (Private->ExitBootServiceEvent != NULL) {
+ gBS->CloseEvent (Private->ExitBootServiceEvent);
+ }
mCallbackInfo->Current = NULL;
FreePool (Private);
return Status;
@@ -2483,12 +2484,15 @@ IScsiOnExitBootService (
)
{
ISCSI_DRIVER_DATA *Private;
Private = (ISCSI_DRIVER_DATA *) Context;
+
gBS->CloseEvent (Private->ExitBootServiceEvent);
-
+
+ Private->ExitBootServiceEvent = NULL;
+
if (Private->Session != NULL) {
IScsiSessionAbort (Private->Session);
}
}
--
1.9.5.msysgit.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after close it.
2018-01-09 2:56 [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after close it Jiaxin Wu
2018-01-09 2:56 ` [Patch 1/2] MdeModulePkg/IScsiDxe: " Jiaxin Wu
2018-01-09 2:56 ` [Patch 2/2] NetworkPkg/IScsiDxe: " Jiaxin Wu
@ 2018-01-09 6:14 ` Fu, Siyuan
2 siblings, 0 replies; 5+ messages in thread
From: Fu, Siyuan @ 2018-01-09 6:14 UTC (permalink / raw)
To: Wu, Jiaxin, edk2-devel@lists.01.org; +Cc: Ye, Ting
Hi, Jiaxin,
The patch is good to me.
Reviewed-by: Fu Siyuan <siyuan.fu@intel.com>
> -----Original Message-----
> From: Wu, Jiaxin
> Sent: Tuesday, January 9, 2018 10:56 AM
> To: edk2-devel@lists.01.org
> Cc: Ye, Ting <ting.ye@intel.com>; Fu, Siyuan <siyuan.fu@intel.com>; Wu,
> Jiaxin <jiaxin.wu@intel.com>
> Subject: [Patch 0/2] IScsiDxe: Set ExitBootServiceEvent to NULL after
> close it.
>
> 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 <ting.ye@intel.com>
> Cc: Fu Siyuan <siyuan.fu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Wu Jiaxin <jiaxin.wu@intel.com>
>
> *** BLURB HERE ***
>
> Jiaxin Wu (2):
> MdeModulePkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close
> it.
> NetworkPkg/IScsiDxe: Set ExitBootServiceEvent to NULL after close it.
>
> MdeModulePkg/Universal/Network/IScsiDxe/IScsiMisc.c | 12 ++++++++----
> NetworkPkg/IScsiDxe/IScsiMisc.c | 12 ++++++++----
> 2 files changed, 16 insertions(+), 8 deletions(-)
>
> --
> 1.9.5.msysgit.1
^ permalink raw reply [flat|nested] 5+ messages in thread