* [PATCH] FatPkg/EnhancedFatDxe: Fix potential hang in async file IO
@ 2016-12-14 10:01 Ruiyu Ni
2016-12-14 10:03 ` Ni, Ruiyu
0 siblings, 1 reply; 2+ messages in thread
From: Ruiyu Ni @ 2016-12-14 10:01 UTC (permalink / raw)
To: edk2-devel; +Cc: Feng Tian, Boaz Kahana
FatQueueTask() is running at TPL_APPLICATION, while
FatDestroySubtask() is running at TPL_NOTIFY, it's possible
for a task containing 2 sub tasks, when the for-loop
executes GetNextNode (&Task->Subtasks, Link), the memory
occupied by Link is freed in FatDestroySubtask().
The fix stores the next link in NextLink so that the delete
in FatDestroySubtask() is safe.
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Feng Tian <feng.tian@intel.com>
Cc: Boaz Kahana <Boaz.Kahana@phoenix.com>
---
FatPkg/EnhancedFatDxe/Misc.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/FatPkg/EnhancedFatDxe/Misc.c b/FatPkg/EnhancedFatDxe/Misc.c
index c035670..cef1acd 100644
--- a/FatPkg/EnhancedFatDxe/Misc.c
+++ b/FatPkg/EnhancedFatDxe/Misc.c
@@ -1,7 +1,7 @@
/** @file
Miscellaneous functions.
-Copyright (c) 2005 - 2013, Intel Corporation. All rights reserved.<BR>
+Copyright (c) 2005 - 2016, 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
@@ -132,6 +132,7 @@ FatQueueTask (
{
EFI_STATUS Status;
LIST_ENTRY *Link;
+ LIST_ENTRY *NextLink;
FAT_SUBTASK *Subtask;
//
@@ -149,9 +150,13 @@ FatQueueTask (
EfiReleaseLock (&FatTaskLock);
Status = EFI_SUCCESS;
- for ( Link = GetFirstNode (&Task->Subtasks)
+ //
+ // Use NextLink to store the next link since Link might be freed in the end of previous loop,
+ // resulting next link cannot be retrieved from Link.
+ //
+ for ( Link = GetFirstNode (&Task->Subtasks), NextLink = GetNextNode (&Task->Subtasks, Link)
; !IsNull (&Task->Subtasks, Link)
- ; Link = GetNextNode (&Task->Subtasks, Link)
+ ; Link = NextLink, NextLink = GetNextNode (&Task->Subtasks, Link)
) {
Subtask = CR (Link, FAT_SUBTASK, Link, FAT_SUBTASK_SIGNATURE);
if (Subtask->Write) {
--
2.9.0.windows.1
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] FatPkg/EnhancedFatDxe: Fix potential hang in async file IO
2016-12-14 10:01 [PATCH] FatPkg/EnhancedFatDxe: Fix potential hang in async file IO Ruiyu Ni
@ 2016-12-14 10:03 ` Ni, Ruiyu
0 siblings, 0 replies; 2+ messages in thread
From: Ni, Ruiyu @ 2016-12-14 10:03 UTC (permalink / raw)
To: Boaz Kahana; +Cc: Tian, Feng, Ni, Ruiyu, edk2-devel@lists.01.org
Boaz,
Could you please verify this patch in your failed system?
I cannot reproduce the issue in my environment.
Thanks/Ray
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Ruiyu Ni
> Sent: Wednesday, December 14, 2016 6:01 PM
> To: edk2-devel@lists.01.org
> Cc: Tian, Feng <feng.tian@intel.com>; Boaz Kahana
> <Boaz.Kahana@phoenix.com>
> Subject: [edk2] [PATCH] FatPkg/EnhancedFatDxe: Fix potential hang in async
> file IO
>
> FatQueueTask() is running at TPL_APPLICATION, while
> FatDestroySubtask() is running at TPL_NOTIFY, it's possible for a task
> containing 2 sub tasks, when the for-loop executes GetNextNode (&Task-
> >Subtasks, Link), the memory occupied by Link is freed in
> FatDestroySubtask().
>
> The fix stores the next link in NextLink so that the delete in
> FatDestroySubtask() is safe.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Feng Tian <feng.tian@intel.com>
> Cc: Boaz Kahana <Boaz.Kahana@phoenix.com>
> ---
> FatPkg/EnhancedFatDxe/Misc.c | 11 ++++++++---
> 1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/FatPkg/EnhancedFatDxe/Misc.c b/FatPkg/EnhancedFatDxe/Misc.c
> index c035670..cef1acd 100644
> --- a/FatPkg/EnhancedFatDxe/Misc.c
> +++ b/FatPkg/EnhancedFatDxe/Misc.c
> @@ -1,7 +1,7 @@
> /** @file
> Miscellaneous functions.
>
> -Copyright (c) 2005 - 2013, Intel Corporation. All rights reserved.<BR>
> +Copyright (c) 2005 - 2016, 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
> @@ -132,6 +132,7 @@ FatQueueTask ( {
> EFI_STATUS Status;
> LIST_ENTRY *Link;
> + LIST_ENTRY *NextLink;
> FAT_SUBTASK *Subtask;
>
> //
> @@ -149,9 +150,13 @@ FatQueueTask (
> EfiReleaseLock (&FatTaskLock);
>
> Status = EFI_SUCCESS;
> - for ( Link = GetFirstNode (&Task->Subtasks)
> + //
> + // Use NextLink to store the next link since Link might be freed in
> + the end of previous loop, // resulting next link cannot be retrieved from
> Link.
> + //
> + for ( Link = GetFirstNode (&Task->Subtasks), NextLink = GetNextNode
> + (&Task->Subtasks, Link)
> ; !IsNull (&Task->Subtasks, Link)
> - ; Link = GetNextNode (&Task->Subtasks, Link)
> + ; Link = NextLink, NextLink = GetNextNode (&Task->Subtasks, Link)
> ) {
> Subtask = CR (Link, FAT_SUBTASK, Link, FAT_SUBTASK_SIGNATURE);
> if (Subtask->Write) {
> --
> 2.9.0.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2016-12-14 10:03 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-12-14 10:01 [PATCH] FatPkg/EnhancedFatDxe: Fix potential hang in async file IO Ruiyu Ni
2016-12-14 10:03 ` Ni, Ruiyu
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox