From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=134.134.136.20; helo=mga02.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) (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 0E943220D4BE0 for ; Mon, 13 Nov 2017 23:44:32 -0800 (PST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 13 Nov 2017 23:48:39 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,393,1505804400"; d="scan'208";a="7385817" Received: from fmsmsx104.amr.corp.intel.com ([10.18.124.202]) by orsmga002.jf.intel.com with ESMTP; 13 Nov 2017 23:48:39 -0800 Received: from fmsmsx156.amr.corp.intel.com (10.18.116.74) by fmsmsx104.amr.corp.intel.com (10.18.124.202) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 13 Nov 2017 23:48:39 -0800 Received: from shsmsx101.ccr.corp.intel.com (10.239.4.153) by fmsmsx156.amr.corp.intel.com (10.18.116.74) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 13 Nov 2017 23:48:38 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.152]) by SHSMSX101.ccr.corp.intel.com ([169.254.1.159]) with mapi id 14.03.0319.002; Tue, 14 Nov 2017 15:48:36 +0800 From: "Wu, Hao A" To: 'Paulo Alcantara' , "Ni, Ruiyu" , "edk2-devel@lists.01.org" CC: "Zeng, Star" , "Dong, Eric" Thread-Topic: [PATCH] MdeModulePkg/PartitionDxe: Fix UDF fs access on certain CD/DVD medias Thread-Index: AQHTRCbD63VHuQqNXEukG8NsiHbW76MRyhzg Date: Tue, 14 Nov 2017 07:48:36 +0000 Message-ID: References: In-Reply-To: Accept-Language: zh-CN, en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdeModulePkg/PartitionDxe: Fix UDF fs access on certain CD/DVD medias X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 14 Nov 2017 07:44:33 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Hi Paulo, Sorry for the delayed response. The patch is good to me: Reviewed-by: Hao Wu Hi Ray, Do you have any other comment on this? Best Regards, Hao Wu > -----Original Message----- > From: Paulo Alcantara [mailto:pcacjr@zytor.com] > Sent: Friday, October 13, 2017 9:25 PM > To: edk2-devel@lists.01.org > Cc: Paulo Alcantara; Wu, Hao A; Ni, Ruiyu; Zeng, Star; Dong, Eric > Subject: [PATCH] MdeModulePkg/PartitionDxe: Fix UDF fs access on certain > CD/DVD medias >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D725 >=20 > Historically many drives or medias do not correctly return their value > for block N - which is also referred as last addressable/recorded block. > When they do so, there is still a problem when relying on last recorded > block number returned by SCSI commands READ CAPACITY and READ TRACK > INFORMATION - that is, between block 0 and block N there may be > unwritten blocks which are located outside any track. >=20 > That said, the Partition driver was unable to find AVDP at block N on > certain medias that do not either return or report their last recorded > block number correctly. >=20 > Apparently there is no official or correct way to find the correct block > number, however tools like the Philips UDF Conformance Tool (udf_test) > apply a correction by searching for an AVDP or VAT in blocks N through > N-456 -- this can be observed by looking at the log reported by udf_test > on those CD/DVD medias. So, if the AVDP or VAT is found, then it sets > the last recorded block number to where AVDP or VAT was located. >=20 > With the below setence in UDF 2.60, 6.13.2.2 Background Physical > Formatting: >=20 > "... the second AVDP must be recorded after the Background physical > Formatting has been finished..." >=20 > Implies that the last recorded block is the one where second AVDP was > recorded. >=20 > This patch implements a similar way to correct the last recorded block > number by searching for last AVDP in blocks N-1 through N-512 on those > certain medias, as well as ensure a minimum number of 2 AVDPs found as > specified by ECMA 167 and UDF 2.60 specifications. >=20 > Cc: Hao Wu > Cc: Ruiyu Ni > Cc: Star Zeng > Cc: Eric Dong > Contributed-under: TianoCore Contribution Agreement 1.1 > Reported-by: Hao Wu > Signed-off-by: Paulo Alcantara > --- > MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 254 ++++++++++++++++-- > -- > 1 file changed, 201 insertions(+), 53 deletions(-) >=20 > diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c > b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c > index 8aee30c759..7eee748958 100644 > --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c > +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c > @@ -14,6 +14,8 @@ >=20 > #include "Partition.h" >=20 > +#define MAX_CORRECTION_BLOCKS_NUM 512u > + > // > // C5BD4D42-1A76-4996-8956-73CDA326CD0A > // > @@ -48,61 +50,199 @@ UDF_DEVICE_PATH gUdfDevicePath =3D { > /** > Find the anchor volume descriptor pointer. >=20 > - @param[in] BlockIo BlockIo interface. > - @param[in] DiskIo DiskIo interface. > - @param[out] AnchorPoint Anchor volume descriptor pointer. > + @param[in] BlockIo BlockIo interface. > + @param[in] DiskIo DiskIo interface. > + @param[out] AnchorPoint Anchor volume descriptor pointer. > + @param[out] LastRecordedBlock Last recorded block. >=20 > - @retval EFI_SUCCESS Anchor volume descriptor pointer found= . > - @retval EFI_VOLUME_CORRUPTED The file system structures are > corrupted. > - @retval other Anchor volume descriptor pointer not f= ound. > + @retval EFI_SUCCESS Anchor volume descriptor pointer fou= nd. > + @retval EFI_VOLUME_CORRUPTED The file system structures are > corrupted. > + @retval other Anchor volume descriptor pointer not= found. >=20 > **/ > EFI_STATUS > FindAnchorVolumeDescriptorPointer ( > IN EFI_BLOCK_IO_PROTOCOL *BlockIo, > IN EFI_DISK_IO_PROTOCOL *DiskIo, > - OUT UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint > + OUT UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint, > + OUT EFI_LBA *LastRecordedBlock > ) > { > - EFI_STATUS Status; > - UINT32 BlockSize; > - EFI_LBA EndLBA; > - EFI_LBA DescriptorLBAs[4]; > - UINTN Index; > - UDF_DESCRIPTOR_TAG *DescriptorTag; > + EFI_STATUS Status; > + UINT32 BlockSize; > + EFI_LBA EndLBA; > + UDF_DESCRIPTOR_TAG *DescriptorTag; > + UINTN AvdpsCount; > + UINTN Size; > + UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoints; > + INTN Index; > + UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPointPtr; > + EFI_LBA LastAvdpBlockNum; >=20 > + // > + // UDF 2.60, 2.2.3 Anchor Volume Descriptor Pointer > + // > + // An Anchor Volume Descriptor Pointer structure shall be recorded in = at > + // least 2 of the following 3 locations on the media: Logical Sector 2= 56, > + // N - 256 or N, where N is the last *addressable* sector of a volume. > + // > + // To figure out what logical sector N is, the SCSI commands READ CAPA= CITY > and > + // READ TRACK INFORMATION are used, however many drives or medias > report their > + // "last recorded block" wrongly. Although, READ CAPACITY returns the = last > + // readable data block but there might be unwritten blocks, which are > located > + // outside any track and therefore AVDP will not be found at block N. > + // > + // That said, we define a magic number of 512 blocks to be used as > correction > + // when attempting to find AVDP and define last block number. > + // > BlockSize =3D BlockIo->Media->BlockSize; > EndLBA =3D BlockIo->Media->LastBlock; > - DescriptorLBAs[0] =3D 256; > - DescriptorLBAs[1] =3D EndLBA - 256; > - DescriptorLBAs[2] =3D EndLBA; > - DescriptorLBAs[3] =3D 512; > + *LastRecordedBlock =3D EndLBA; > + AvdpsCount =3D 0; >=20 > - for (Index =3D 0; Index < ARRAY_SIZE (DescriptorLBAs); Index++) { > - Status =3D DiskIo->ReadDisk ( > - DiskIo, > - BlockIo->Media->MediaId, > - MultU64x32 (DescriptorLBAs[Index], BlockSize), > - sizeof (UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER), > - (VOID *)AnchorPoint > - ); > - if (EFI_ERROR (Status)) { > - return Status; > - } > + // > + // Find AVDP at block 256 > + // > + Status =3D DiskIo->ReadDisk ( > + DiskIo, > + BlockIo->Media->MediaId, > + MultU64x32 (256, BlockSize), > + sizeof (*AnchorPoint), > + AnchorPoint > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + DescriptorTag =3D &AnchorPoint->DescriptorTag; > + > + // > + // Check if read block is a valid AVDP descriptor > + // > + if (DescriptorTag->TagIdentifier =3D=3D UdfAnchorVolumeDescriptorPoint= er) { > + DEBUG ((DEBUG_INFO, "%a: found AVDP at block %d\n", __FUNCTION__, > 256)); > + AvdpsCount++; > + } > + > + // > + // Find AVDP at block N - 256 > + // > + Status =3D DiskIo->ReadDisk ( > + DiskIo, > + BlockIo->Media->MediaId, > + MultU64x32 ((UINT64)EndLBA - 256, BlockSize), > + sizeof (*AnchorPoint), > + AnchorPoint > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + // > + // Check if read block is a valid AVDP descriptor > + // > + if (DescriptorTag->TagIdentifier =3D=3D UdfAnchorVolumeDescriptorPoint= er && > + ++AvdpsCount =3D=3D 2) { > + DEBUG ((DEBUG_INFO, "%a: found AVDP at block %Ld\n", __FUNCTION__, > + EndLBA - 256)); > + return EFI_SUCCESS; > + } > + > + // > + // Check if at least one AVDP was found in previous locations > + // > + if (AvdpsCount =3D=3D 0) { > + return EFI_VOLUME_CORRUPTED; > + } > + > + // > + // Find AVDP at block N > + // > + Status =3D DiskIo->ReadDisk ( > + DiskIo, > + BlockIo->Media->MediaId, > + MultU64x32 ((UINT64)EndLBA, BlockSize), > + sizeof (*AnchorPoint), > + AnchorPoint > + ); > + if (EFI_ERROR (Status)) { > + return Status; > + } > + > + // > + // Check if read block is a valid AVDP descriptor > + // > + if (DescriptorTag->TagIdentifier =3D=3D UdfAnchorVolumeDescriptorPoint= er) { > + return EFI_SUCCESS; > + } > + > + // > + // No AVDP found at block N. Possibly drive/media returned bad last > recorded > + // block, or it is part of unwritten data blocks and outside any track= . > + // > + // Search backwards for an AVDP from block N-1 through > + // N-MAX_CORRECTION_BLOCKS_NUM. If any AVDP is found, then correct > last block > + // number for the new UDF partition child handle. > + // > + Size =3D MAX_CORRECTION_BLOCKS_NUM * BlockSize; > + > + AnchorPoints =3D AllocateZeroPool (Size); > + if (AnchorPoints =3D=3D NULL) { > + return EFI_OUT_OF_RESOURCES; > + } > + > + // > + // Read consecutive MAX_CORRECTION_BLOCKS_NUM disk blocks > + // > + Status =3D DiskIo->ReadDisk ( > + DiskIo, > + BlockIo->Media->MediaId, > + MultU64x32 ((UINT64)EndLBA - MAX_CORRECTION_BLOCKS_NUM, > BlockSize), > + Size, > + AnchorPoints > + ); > + if (EFI_ERROR (Status)) { > + goto Out_Free; > + } >=20 > - DescriptorTag =3D &AnchorPoint->DescriptorTag; > + Status =3D EFI_VOLUME_CORRUPTED; > + > + // > + // Search for AVDP from blocks N-1 through N- > MAX_CORRECTION_BLOCKS_NUM > + // > + for (Index =3D MAX_CORRECTION_BLOCKS_NUM - 2; Index >=3D 0; Index--) { > + AnchorPointPtr =3D (VOID *)((UINTN)AnchorPoints + Index * BlockSize)= ; > + > + DescriptorTag =3D &AnchorPointPtr->DescriptorTag; >=20 > // > - // Check if read LBA has a valid AVDP descriptor. > + // Check if read block is a valid AVDP descriptor > // > if (DescriptorTag->TagIdentifier =3D=3D UdfAnchorVolumeDescriptorPoi= nter) { > - return EFI_SUCCESS; > + // > + // Calculate last recorded block number > + // > + LastAvdpBlockNum =3D EndLBA - (MAX_CORRECTION_BLOCKS_NUM - > Index); > + DEBUG ((DEBUG_WARN, "%a: found AVDP at block %Ld\n", > __FUNCTION__, > + LastAvdpBlockNum)); > + DEBUG ((DEBUG_WARN, "%a: correcting last block from %Ld to %Ld\n", > + __FUNCTION__, EndLBA, LastAvdpBlockNum)); > + // > + // Save read AVDP from last block > + // > + CopyMem (AnchorPoint, AnchorPointPtr, sizeof (*AnchorPointPtr)); > + // > + // Set last recorded block number > + // > + *LastRecordedBlock =3D LastAvdpBlockNum; > + Status =3D EFI_SUCCESS; > + break; > } > } > - // > - // No AVDP found. > - // > - return EFI_VOLUME_CORRUPTED; > + > +Out_Free: > + FreePool (AnchorPoints); > + return Status; > } >=20 > /** > @@ -280,16 +420,17 @@ IsLogicalVolumeDescriptorSupported ( > Find UDF logical volume location and whether it is supported by curren= t EDK2 > UDF file system implementation. >=20 > - @param[in] BlockIo BlockIo interface. > - @param[in] DiskIo DiskIo interface. > - @param[in] AnchorPoint Anchor volume descriptor pointer. > - @param[out] MainVdsStartBlock Main VDS starting block number. > - @param[out] MainVdsEndBlock Main VDS ending block number. > + @param[in] BlockIo BlockIo interface. > + @param[in] DiskIo DiskIo interface. > + @param[in] AnchorPoint Anchor volume descriptor pointer. > + @param[in] LastRecordedBlock Last recorded block in media. > + @param[out] MainVdsStartBlock Main VDS starting block number. > + @param[out] MainVdsEndBlock Main VDS ending block number. >=20 > - @retval EFI_SUCCESS UDF logical volume was found. > - @retval EFI_VOLUME_CORRUPTED UDF file system structures are > corrupted. > - @retval EFI_UNSUPPORTED UDF logical volume is not supported. > - @retval other Failed to perform disk I/O. > + @retval EFI_SUCCESS UDF logical volume was found. > + @retval EFI_VOLUME_CORRUPTED UDF file system structures are > corrupted. > + @retval EFI_UNSUPPORTED UDF logical volume is not supported. > + @retval other Failed to perform disk I/O. >=20 > **/ > EFI_STATUS > @@ -297,13 +438,13 @@ FindLogicalVolumeLocation ( > IN EFI_BLOCK_IO_PROTOCOL *BlockIo, > IN EFI_DISK_IO_PROTOCOL *DiskIo, > IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint, > + IN EFI_LBA LastRecordedBlock, > OUT UINT64 *MainVdsStartBlock, > OUT UINT64 *MainVdsEndBlock > ) > { > EFI_STATUS Status; > UINT32 BlockSize; > - EFI_LBA LastBlock; > UDF_EXTENT_AD *ExtentAd; > UINT64 SeqBlocksNum; > UINT64 SeqStartBlock; > @@ -316,7 +457,6 @@ FindLogicalVolumeLocation ( > UDF_DESCRIPTOR_TAG *DescriptorTag; >=20 > BlockSize =3D BlockIo->Media->BlockSize; > - LastBlock =3D BlockIo->Media->LastBlock; > ExtentAd =3D &AnchorPoint->MainVolumeDescriptorSequenceExtent; >=20 > // > @@ -328,7 +468,7 @@ FindLogicalVolumeLocation ( > // Also make sure it does not exceed maximum number of blocks in the d= isk. > // > SeqBlocksNum =3D DivU64x32 ((UINT64)ExtentAd->ExtentLength, BlockSize)= ; > - if (SeqBlocksNum < 16 || (EFI_LBA)SeqBlocksNum > LastBlock + 1) { > + if (SeqBlocksNum < 16 || (EFI_LBA)SeqBlocksNum > LastRecordedBlock + 1= ) { > return EFI_VOLUME_CORRUPTED; > } >=20 > @@ -336,8 +476,8 @@ FindLogicalVolumeLocation ( > // Check for valid Volume Descriptor Sequence starting block number > // > SeqStartBlock =3D (UINT64)ExtentAd->ExtentLocation; > - if (SeqStartBlock > LastBlock || > - SeqStartBlock + SeqBlocksNum - 1 > LastBlock) { > + if (SeqStartBlock > LastRecordedBlock || > + SeqStartBlock + SeqBlocksNum - 1 > LastRecordedBlock) { > return EFI_VOLUME_CORRUPTED; > } >=20 > @@ -437,9 +577,10 @@ FindLogicalVolumeLocation ( > *MainVdsStartBlock =3D GuardMainVdsStartBlock; > // > // We do not need to read either LVD or PD descriptors to know the l= ast > - // valid block in the found UDF file system. It's already LastBlock. > + // valid block in the found UDF file system. It's already > + // LastRecordedBlock. > // > - *MainVdsEndBlock =3D LastBlock; > + *MainVdsEndBlock =3D LastRecordedBlock; >=20 > Status =3D EFI_SUCCESS; > } > @@ -473,8 +614,9 @@ FindUdfFileSystem ( > OUT EFI_LBA *EndingLBA > ) > { > - EFI_STATUS Status; > + EFI_STATUS Status; > UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint; > + EFI_LBA LastRecordedBlock; >=20 > // > // Find UDF volume identifiers > @@ -487,7 +629,12 @@ FindUdfFileSystem ( > // > // Find Anchor Volume Descriptor Pointer > // > - Status =3D FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo, &Anchor= Point); > + Status =3D FindAnchorVolumeDescriptorPointer ( > + BlockIo, > + DiskIo, > + &AnchorPoint, > + &LastRecordedBlock > + ); > if (EFI_ERROR (Status)) { > return Status; > } > @@ -499,6 +646,7 @@ FindUdfFileSystem ( > BlockIo, > DiskIo, > &AnchorPoint, > + LastRecordedBlock, > (UINT64 *)StartingLBA, > (UINT64 *)EndingLBA > ); > -- > 2.11.0