From 68099b52b0fcc1d45864154954d776d91afb33e0 Mon Sep 17 00:00:00 2001 From: Hao Wu Date: Tue, 16 Oct 2018 13:09:43 +0800 Subject: [PATCH] MdeModulePkg/UdfDxe: Avoid possible use of already-freed data REF:https://bugzilla.tianocore.org/show_bug.cgi?id=1255 For function ReadFile(): If the line Status = GetAedAdsData ( ... ); is reached multiple times during the 'for' loop, freeing the data pointed by variable 'Data' may potentially lead to variable 'Ad' referencing the already-freed data. After calling function GetAllocationDescriptor(), 'Data' and 'Ad' may point to the same memory (with some possible offset). Hence, this commit will move the FreePool() call backwards to ensure the data will no longer be used. Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu Reviewed-by: Paulo Alcantara Acked-by: Star Zeng --- .../Universal/Disk/UdfDxe/FileSystemOperations.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c index cabb599695..b9ebddfe62 100644 --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c @@ -1078,6 +1078,7 @@ ReadFile ( EFI_STATUS Status; UINT32 LogicalBlockSize; VOID *Data; + VOID *DataBak; UINT64 Length; VOID *Ad; UINT64 AdOffset; @@ -1218,12 +1219,7 @@ ReadFile ( // Descriptor and its extents (ADs). // if (GET_EXTENT_FLAGS (RecordingFlags, Ad) == ExtentIsNextExtent) { - if (!DoFreeAed) { - DoFreeAed = TRUE; - } else { - FreePool (Data); - } - + DataBak = Data; Status = GetAedAdsData ( BlockIo, DiskIo, @@ -1234,6 +1230,13 @@ ReadFile ( &Data, &Length ); + + if (!DoFreeAed) { + DoFreeAed = TRUE; + } else { + FreePool (DataBak); + } + if (EFI_ERROR (Status)) { goto Error_Get_Aed; } -- 2.39.2