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.100; helo=mga07.intel.com; envelope-from=hao.a.wu@intel.com; receiver=edk2-devel@lists.01.org Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) (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 AEB7121B00DC1 for ; Wed, 15 Nov 2017 04:41:00 -0800 (PST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 15 Nov 2017 04:45:08 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.44,399,1505804400"; d="scan'208";a="2920522" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga001.fm.intel.com with ESMTP; 15 Nov 2017 04:45:07 -0800 Received: from fmsmsx126.amr.corp.intel.com (10.18.125.43) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 15 Nov 2017 04:45:07 -0800 Received: from shsmsx151.ccr.corp.intel.com (10.239.6.50) by FMSMSX126.amr.corp.intel.com (10.18.125.43) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 15 Nov 2017 04:45:07 -0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.152]) by SHSMSX151.ccr.corp.intel.com ([169.254.3.218]) with mapi id 14.03.0319.002; Wed, 15 Nov 2017 20:45:04 +0800 From: "Wu, Hao A" To: "Wu, Hao A" , 'Paulo Alcantara' , "Ni, Ruiyu" , "edk2-devel@lists.01.org" CC: "Dong, Eric" , "Zeng, Star" Thread-Topic: [PATCH] MdeModulePkg/PartitionDxe: Fix UDF fs access on certain CD/DVD medias Thread-Index: AQHTRCbD63VHuQqNXEukG8NsiHbW76MRyhzggAPMm0A= Date: Wed, 15 Nov 2017 12:45:04 +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: Wed, 15 Nov 2017 12:41:00 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Pushed at: 1fbe8276c4 Best Regards, Hao Wu > -----Original Message----- > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Wu= , > Hao A > Sent: Tuesday, November 14, 2017 3:49 PM > To: 'Paulo Alcantara'; Ni, Ruiyu; edk2-devel@lists.01.org > Cc: Dong, Eric; Zeng, Star > Subject: Re: [edk2] [PATCH] MdeModulePkg/PartitionDxe: Fix UDF fs access = on > certain CD/DVD medias >=20 > Hi Paulo, >=20 > Sorry for the delayed response. The patch is good to me: > Reviewed-by: Hao Wu >=20 > Hi Ray, >=20 > Do you have any other comment on this? >=20 >=20 > Best Regards, > Hao Wu >=20 >=20 > > -----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 certai= n > > CD/DVD medias > > > > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D725 > > > > 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. > > > > 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. > > > > Apparently there is no official or correct way to find the correct bloc= k > > 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_tes= t > > 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. > > > > With the below setence in UDF 2.60, 6.13.2.2 Background Physical > > Formatting: > > > > "... the second AVDP must be recorded after the Background physical > > Formatting has been finished..." > > > > Implies that the last recorded block is the one where second AVDP was > > recorded. > > > > 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. > > > > 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(-) > > > > 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 @@ > > > > #include "Partition.h" > > > > +#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. > > > > - @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. > > > > - @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. > > + @retval EFI_SUCCESS Anchor volume descriptor pointer f= ound. > > + @retval EFI_VOLUME_CORRUPTED The file system structures are > > corrupted. > > + @retval other Anchor volume descriptor pointer n= ot found. > > > > **/ > > 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; > > > > + // > > + // UDF 2.60, 2.2.3 Anchor Volume Descriptor Pointer > > + // > > + // An Anchor Volume Descriptor Pointer structure shall be recorded i= n at > > + // least 2 of the following 3 locations on the media: Logical Sector= 256, > > + // N - 256 or N, where N is the last *addressable* sector of a volum= e. > > + // > > + // To figure out what logical sector N is, the SCSI commands READ > CAPACITY > > and > > + // READ TRACK INFORMATION are used, however many drives or medias > > report their > > + // "last recorded block" wrongly. Although, READ CAPACITY returns th= e last > > + // readable data block but there might be unwritten blocks, which ar= e > > 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; > > > > - 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 UdfAnchorVolumeDescriptorPoi= nter) { > > + 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 UdfAnchorVolumeDescriptorPoi= nter && > > + ++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 UdfAnchorVolumeDescriptorPoi= nter) { > > + 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 tra= ck. > > + // > > + // 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; > > + } > > > > - 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 * BlockSiz= e); > > + > > + DescriptorTag =3D &AnchorPointPtr->DescriptorTag; > > > > // > > - // Check if read LBA has a valid AVDP descriptor. > > + // Check if read block is a valid AVDP descriptor > > // > > if (DescriptorTag->TagIdentifier =3D=3D UdfAnchorVolumeDescriptorP= ointer) { > > - 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; > > } > > > > /** > > @@ -280,16 +420,17 @@ IsLogicalVolumeDescriptorSupported ( > > Find UDF logical volume location and whether it is supported by curr= ent > EDK2 > > UDF file system implementation. > > > > - @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. > > > > - @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 supporte= d. > > + @retval other Failed to perform disk I/O. > > > > **/ > > 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; > > > > BlockSize =3D BlockIo->Media->BlockSize; > > - LastBlock =3D BlockIo->Media->LastBlock; > > ExtentAd =3D &AnchorPoint->MainVolumeDescriptorSequenceExtent; > > > > // > > @@ -328,7 +468,7 @@ FindLogicalVolumeLocation ( > > // Also make sure it does not exceed maximum number of blocks in the= disk. > > // > > SeqBlocksNum =3D DivU64x32 ((UINT64)ExtentAd->ExtentLength, BlockSiz= e); > > - if (SeqBlocksNum < 16 || (EFI_LBA)SeqBlocksNum > LastBlock + 1) { > > + if (SeqBlocksNum < 16 || (EFI_LBA)SeqBlocksNum > LastRecordedBlock += 1) > { > > return EFI_VOLUME_CORRUPTED; > > } > > > > @@ -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; > > } > > > > @@ -437,9 +577,10 @@ FindLogicalVolumeLocation ( > > *MainVdsStartBlock =3D GuardMainVdsStartBlock; > > // > > // We do not need to read either LVD or PD descriptors to know the= last > > - // valid block in the found UDF file system. It's already LastBloc= k. > > + // valid block in the found UDF file system. It's already > > + // LastRecordedBlock. > > // > > - *MainVdsEndBlock =3D LastBlock; > > + *MainVdsEndBlock =3D LastRecordedBlock; > > > > 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; > > > > // > > // Find UDF volume identifiers > > @@ -487,7 +629,12 @@ FindUdfFileSystem ( > > // > > // Find Anchor Volume Descriptor Pointer > > // > > - Status =3D FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo, > &AnchorPoint); > > + 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 >=20 > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel