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.43; helo=mga05.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) (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 42F9B21BADAB2 for ; Tue, 16 Oct 2018 00:23:57 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Oct 2018 00:23:57 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,387,1534834800"; d="scan'208";a="97812164" Received: from shwdeopenpsi014.ccr.corp.intel.com ([10.239.9.9]) by fmsmga004.fm.intel.com with ESMTP; 16 Oct 2018 00:23:55 -0700 From: Hao Wu To: edk2-devel@lists.01.org Cc: Hao Wu , Paulo Alcantara , Ruiyu Ni , Star Zeng Date: Tue, 16 Oct 2018 15:23:40 +0800 Message-Id: <20181016072340.22068-11-hao.a.wu@intel.com> X-Mailer: git-send-email 2.12.0.windows.1 In-Reply-To: <20181016072340.22068-1-hao.a.wu@intel.com> References: <20181016072340.22068-1-hao.a.wu@intel.com> Subject: [PATCH v1 10/10] MdeModulePkg/UdfDxe: Avoid possible use of already-freed data X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 16 Oct 2018 07:23:57 -0000 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: Paulo Alcantara Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Hao Wu --- MdeModulePkg/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 7526de79b2..bf73ab4252 100644 --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c @@ -1044,6 +1044,7 @@ ReadFile ( EFI_STATUS Status; UINT32 LogicalBlockSize; VOID *Data; + VOID *DataBak; UINT64 Length; VOID *Ad; UINT64 AdOffset; @@ -1184,12 +1185,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, @@ -1200,6 +1196,13 @@ ReadFile ( &Data, &Length ); + + if (!DoFreeAed) { + DoFreeAed = TRUE; + } else { + FreePool (DataBak); + } + if (EFI_ERROR (Status)) { goto Error_Get_Aed; } -- 2.12.0.windows.1