* [PATCH v2 0/3] UDF partition driver fix
@ 2017-09-17 13:13 Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions Paulo Alcantara
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-17 13:13 UTC (permalink / raw)
To: edk2-devel
Cc: Paulo Alcantara, Michael D Kinney, Liming Gao, Laszlo Ersek,
Ruiyu Ni, Star Zeng, Jiewen Yao
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=707
Hi,
This patchset fixes a bug in Partition driver that UDF partitions
occupied the entire disk space instead of using LVD space only.
BTW, I've only tested it under OVMF and built it with GCC only. That
would be appreciable if someone could build with other toolchains and
see if this doesn't break.
I used a Windows 10 ISO image with UdfDxe disabled and enabled. The
`map -r` output seemed OK. No breakage when booting an OS off an
ElTorito partition from an UDF bridge disk.
v1->v2:
- Followed Laszlo's suggestions to submit a proper patchset. Thanks!
- As I'm still waiting for Ruiyu and Star to test this fix, I took
advantage of it and did some code cleanups :-)
Repo: https://github.com/pcacjr/edk2.git
Branch: udf-partition-fix-v2
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---
Paulo Alcantara (3):
MdePkg: Add UDF volume structure definitions
MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323 +++++++++++-
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 13 +-
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515 ++++++++------------
MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 88 +---
MdePkg/Include/IndustryStandard/Udf.h | 63 +++
6 files changed, 566 insertions(+), 443 deletions(-)
--
2.11.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
2017-09-17 13:13 [PATCH v2 0/3] UDF partition driver fix Paulo Alcantara
@ 2017-09-17 13:13 ` Paulo Alcantara
2017-09-18 0:28 ` Ni, Ruiyu
2017-09-17 13:13 ` [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition Paulo Alcantara
` (2 subsequent siblings)
3 siblings, 1 reply; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-17 13:13 UTC (permalink / raw)
To: edk2-devel
Cc: Paulo Alcantara, Michael D Kinney, Liming Gao, Laszlo Ersek,
Ruiyu Ni
This patch adds a fewe more UDF structures in order to detect Logical
Volume and Partition descriptors during Main Volume Descriptor Sequence
in Partition driver.
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---
MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
1 file changed, 63 insertions(+)
diff --git a/MdePkg/Include/IndustryStandard/Udf.h b/MdePkg/Include/IndustryStandard/Udf.h
index 0febb4bcda..6cc42f8543 100644
--- a/MdePkg/Include/IndustryStandard/Udf.h
+++ b/MdePkg/Include/IndustryStandard/Udf.h
@@ -27,9 +27,19 @@
#define _GET_TAG_ID(_Pointer) \
(((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
+#define IS_PD(_Pointer) \
+ ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
+#define IS_LVD(_Pointer) \
+ ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
+#define IS_TD(_Pointer) \
+ ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
+
#define IS_AVDP(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
+#define LV_UDF_REVISION(_Lv) \
+ *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
+
#pragma pack(1)
typedef struct {
@@ -55,6 +65,59 @@ typedef struct {
UINT8 Reserved[480];
} UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
+typedef struct {
+ UINT8 CharacterSetType;
+ UINT8 CharacterSetInfo[63];
+} UDF_CHAR_SPEC;
+
+typedef struct {
+ UINT8 Flags;
+ UINT8 Identifier[23];
+ UINT8 IdentifierSuffix[8];
+} UDF_ENTITY_ID;
+
+typedef struct {
+ UINT32 LogicalBlockNumber;
+ UINT16 PartitionReferenceNumber;
+} UDF_LB_ADDR;
+
+typedef struct {
+ UINT32 ExtentLength;
+ UDF_LB_ADDR ExtentLocation;
+ UINT8 ImplementationUse[6];
+} UDF_LONG_ALLOCATION_DESCRIPTOR;
+
+typedef struct {
+ UDF_DESCRIPTOR_TAG DescriptorTag;
+ UINT32 VolumeDescriptorSequenceNumber;
+ UDF_CHAR_SPEC DescriptorCharacterSet;
+ UINT8 LogicalVolumeIdentifier[128];
+ UINT32 LogicalBlockSize;
+ UDF_ENTITY_ID DomainIdentifier;
+ UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
+ UINT32 MapTableLength;
+ UINT32 NumberOfPartitionMaps;
+ UDF_ENTITY_ID ImplementationIdentifier;
+ UINT8 ImplementationUse[128];
+ UDF_EXTENT_AD IntegritySequenceExtent;
+ UINT8 PartitionMaps[6];
+} UDF_LOGICAL_VOLUME_DESCRIPTOR;
+
+typedef struct {
+ UDF_DESCRIPTOR_TAG DescriptorTag;
+ UINT32 VolumeDescriptorSequenceNumber;
+ UINT16 PartitionFlags;
+ UINT16 PartitionNumber;
+ UDF_ENTITY_ID PartitionContents;
+ UINT8 PartitionContentsUse[128];
+ UINT32 AccessType;
+ UINT32 PartitionStartingLocation;
+ UINT32 PartitionLength;
+ UDF_ENTITY_ID ImplementationIdentifier;
+ UINT8 ImplementationUse[128];
+ UINT8 Reserved[156];
+} UDF_PARTITION_DESCRIPTOR;
+
#pragma pack()
#endif
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-17 13:13 [PATCH v2 0/3] UDF partition driver fix Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions Paulo Alcantara
@ 2017-09-17 13:13 ` Paulo Alcantara
2017-09-18 0:54 ` Ni, Ruiyu
2017-09-17 13:13 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes Paulo Alcantara
2017-09-18 4:52 ` [PATCH v2 0/3] UDF partition driver fix Ni, Ruiyu
3 siblings, 1 reply; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-17 13:13 UTC (permalink / raw)
To: edk2-devel; +Cc: Paulo Alcantara, Eric Dong, Ruiyu Ni, Star Zeng, Laszlo Ersek
Do not use entire block device size for the UDF logical partition,
instead reserve the appropriate space (LVD space) for it.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---
MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323 ++++++++++++++++++--
1 file changed, 299 insertions(+), 24 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
index 7856b5dfc1..3e84882922 100644
--- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
@@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
return EFI_VOLUME_CORRUPTED;
}
-/**
- Check if block device supports a valid UDF file system as specified by OSTA
- Universal Disk Format Specification 2.60.
-
- @param[in] BlockIo BlockIo interface.
- @param[in] DiskIo DiskIo interface.
-
- @retval EFI_SUCCESS UDF file system found.
- @retval EFI_UNSUPPORTED UDF file system not found.
- @retval EFI_NO_MEDIA The device has no media.
- @retval EFI_DEVICE_ERROR The device reported an error.
- @retval EFI_VOLUME_CORRUPTED The file system structures are corrupted.
- @retval EFI_OUT_OF_RESOURCES The scan was not successful due to lack of
- resources.
-
-**/
EFI_STATUS
-SupportUdfFileSystem (
+FindUdfVolumeIdentifiers (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
IN EFI_DISK_IO_PROTOCOL *DiskIo
)
@@ -111,7 +95,6 @@ SupportUdfFileSystem (
UINT64 EndDiskOffset;
CDROM_VOLUME_DESCRIPTOR VolDescriptor;
CDROM_VOLUME_DESCRIPTOR TerminatingVolDescriptor;
- UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof (CDROM_VOLUME_DESCRIPTOR));
@@ -207,12 +190,302 @@ SupportUdfFileSystem (
return EFI_UNSUPPORTED;
}
+ return EFI_SUCCESS;
+}
+
+EFI_STATUS
+GetPartitionNumber (
+ IN UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc,
+ OUT UINT16 *PartitionNum
+ )
+{
+ EFI_STATUS Status;
+ UDF_LONG_ALLOCATION_DESCRIPTOR *LongAd;
+
+ Status = EFI_SUCCESS;
+
+ switch (LV_UDF_REVISION (LogicalVolDesc)) {
+ case 0x0102:
+ //
+ // UDF 1.20 only supports Type 1 Partition
+ //
+ *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
+ break;
+ case 0x0150:
+ //
+ // Ensure Type 1 Partition map
+ //
+ ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
+ LogicalVolDesc->PartitionMaps[1] == 6);
+ *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
+ break;
+ case 0x0260:
+ LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
+ *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
+ break;
+ default:
+ //
+ // Unhandled UDF revision
+ //
+ Status = EFI_VOLUME_CORRUPTED;
+ break;
+ }
+
+ return Status;
+}
+
+EFI_STATUS
+FindLogicalVolumeLocation (
+ IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
+ IN EFI_DISK_IO_PROTOCOL *DiskIo,
+ IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
+ OUT UINT64 *MainVdsStartLsn,
+ OUT UINT64 *LogicalVolEndLsn
+ )
+{
+ EFI_STATUS Status;
+ UINT32 BlockSize;
+ EFI_LBA LastBlock;
+ UDF_EXTENT_AD *ExtentAd;
+ UINT64 StartingLsn;
+ UINT64 EndingLsn;
+ VOID *Buffer;
+ UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
+ UDF_PARTITION_DESCRIPTOR *PartitionDesc;
+ UINT64 GuardMainVdsStartLsn;
+ UINT16 PartitionNum;
+
+ BlockSize = BlockIo->Media->BlockSize;
+ LastBlock = BlockIo->Media->LastBlock;
+ ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
+ StartingLsn = (UINT64)ExtentAd->ExtentLocation;
+ EndingLsn =
+ StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength, BlockSize);
+
+ LogicalVolDesc = NULL;
+ PartitionDesc = NULL;
+ GuardMainVdsStartLsn = StartingLsn;
+
+ //
+ // Allocate buffer for reading disk blocks
+ //
+ Buffer = AllocateZeroPool (BlockSize);
+ if (Buffer == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
+
+ Status = EFI_VOLUME_CORRUPTED;
+
+ //
+ // As per UDF 2.60 specification:
+ //
+ // There shall be exactly one prevailing Logical Volume Descriptor
+ // recorded per Volume Set.
+ //
+ // Start Main Volume Descriptor Sequence and find Logical Volume Descriptor
+ //
+ while (StartingLsn <= EndingLsn) {
+ //
+ // Read disk block
+ //
+ Status = DiskIo->ReadDisk (
+ DiskIo,
+ BlockIo->Media->MediaId,
+ MultU64x32 (StartingLsn, BlockSize),
+ BlockSize,
+ Buffer
+ );
+ if (EFI_ERROR (Status)) {
+ goto Out_Free;
+ }
+
+ //
+ // Check if read block is a Terminating Descriptor
+ //
+ if (IS_TD (Buffer)) {
+ //
+ // Stop Main Volume Descriptor Sequence
+ //
+ break;
+ }
+
+ //
+ // Check if read block is a Logical Volume Descriptor
+ //
+ if (IS_LVD (Buffer)) {
+ //
+ // Ensure only one LVD (Logical Volume Descriptor) is handled
+ //
+ if (LogicalVolDesc != NULL) {
+ Status = EFI_UNSUPPORTED;
+ goto Out_Free;
+ }
+
+ //
+ // As per UDF 2.60 specification:
+ //
+ // For the purpose of interchange, Partition Maps shall be limited to
+ // Partition Map type 1, except type 2 maps.
+ //
+ // NOTE: Type 1 Partitions are the only supported in this implementation.
+ //
+ LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
+ if (LogicalVolDesc == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Out_Free;
+ }
+
+ //
+ // Save Logical Volume Descriptor
+ //
+ CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
+ } else if (IS_PD (Buffer)) {
+ //
+ // Ensure only one PD (Partition Descriptor) is handled
+ //
+ if (PartitionDesc != NULL) {
+ Status = EFI_UNSUPPORTED;
+ goto Out_Free;
+ }
+
+ //
+ // Found a Partition Descriptor.
+ //
+ // As per UDF 2.60 specification:
+ //
+ // A Partition Descriptor Access Type of read-only, rewritable,
+ // overwritable, write-once and pseudo-overwritable shall be
+ // supported. There shall be exactly one prevailing Partition
+ // Descriptor recorded per volume, with one exception. For Volume
+ // Sets that consist of single volume, the volume may contain 2 non-
+ // overlapping Partitions with 2 prevailing Partition Descriptors only
+ // if one has an Access Type of read-only and the other has an
+ // Access Type of rewritable, overwritable, or write-once. The
+ // Logical Volume for this volume would consist of the contents of
+ // both partitions.
+ //
+ PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
+ if (PartitionDesc == NULL) {
+ Status = EFI_OUT_OF_RESOURCES;
+ goto Out_Free;
+ }
+
+ //
+ // Save Partition Descriptor
+ //
+ CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
+ }
+ //
+ // Go to next disk block
+ //
+ StartingLsn++;
+ }
+
+ Status = EFI_VOLUME_CORRUPTED;
+
+ //
+ // Check if LVD and PD were found
+ //
+ if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
+ //
+ // Get partition number from Partition map in LVD descriptor
+ //
+ Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
+ if (EFI_ERROR (Status)) {
+ goto Out_Free;
+ }
+
+ //
+ // Make sure we're handling expected Partition Descriptor
+ //
+ if (PartitionDesc->PartitionNumber != PartitionNum) {
+ Status = EFI_VOLUME_CORRUPTED;
+ goto Out_Free;
+ }
+
+ //
+ // Cover the main VDS area so UdfDxe driver will also be able to get LVD and
+ // PD descriptors out from the file system.
+ //
+ *MainVdsStartLsn = GuardMainVdsStartLsn;
+ *LogicalVolEndLsn = *MainVdsStartLsn + (UINT64)ExtentAd->ExtentLength;
+
+ //
+ // Cover UDF partition area
+ //
+ *LogicalVolEndLsn +=
+ ((UINT64)PartitionDesc->PartitionStartingLocation -
+ *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
+ //
+ // Ensure to not attempt reading past end of device
+ //
+ if (*LogicalVolEndLsn > LastBlock) {
+ Status = EFI_VOLUME_CORRUPTED;
+ } else {
+ Status = EFI_SUCCESS;
+ }
+ }
+
+Out_Free:
+ //
+ // Free block read buffer
+ //
+ FreePool (Buffer);
+ //
+ // Free Logical Volume Descriptor
+ //
+ if (LogicalVolDesc != NULL) {
+ FreePool (LogicalVolDesc);
+ }
+ //
+ // Free Partition Descriptor
+ //
+ if (PartitionDesc != NULL) {
+ FreePool (PartitionDesc);
+ }
+
+ return Status;
+}
+
+EFI_STATUS
+FindUdfLogicalVolume (
+ IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
+ IN EFI_DISK_IO_PROTOCOL *DiskIo,
+ OUT EFI_LBA *StartingLBA,
+ OUT EFI_LBA *EndingLBA
+ )
+{
+ EFI_STATUS Status;
+ UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
+
+ //
+ // Find UDF volume identifiers
+ //
+ Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo);
+ if (EFI_ERROR (Status)) {
+ return Status;
+ }
+
+ //
+ // Find Anchor Volume Descriptor Pointer
+ //
Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo, &AnchorPoint);
if (EFI_ERROR (Status)) {
- return EFI_UNSUPPORTED;
+ return Status;
}
- return EFI_SUCCESS;
+ //
+ // Find Logical Volume location
+ //
+ Status = FindLogicalVolumeLocation (
+ BlockIo,
+ DiskIo,
+ &AnchorPoint,
+ (UINT64 *)StartingLBA,
+ (UINT64 *)EndingLBA
+ );
+
+ return Status;
}
/**
@@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
EFI_GUID *VendorDefinedGuid;
EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
EFI_PARTITION_INFO_PROTOCOL PartitionInfo;
+ EFI_LBA StartingLBA;
+ EFI_LBA EndingLBA;
Media = BlockIo->Media;
@@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
}
//
- // Check if block device supports an UDF file system
+ // Find UDF logical volume on block device
//
- Status = SupportUdfFileSystem (BlockIo, DiskIo);
+ Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA, &EndingLBA);
if (EFI_ERROR (Status)) {
return EFI_NOT_FOUND;
}
@@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
DevicePath,
(EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
&PartitionInfo,
- 0,
- Media->LastBlock,
+ StartingLBA,
+ EndingLBA,
Media->BlockSize
);
if (!EFI_ERROR (Status)) {
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
2017-09-17 13:13 [PATCH v2 0/3] UDF partition driver fix Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition Paulo Alcantara
@ 2017-09-17 13:13 ` Paulo Alcantara
2017-09-18 1:00 ` Ni, Ruiyu
2017-09-18 4:52 ` [PATCH v2 0/3] UDF partition driver fix Ni, Ruiyu
3 siblings, 1 reply; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-17 13:13 UTC (permalink / raw)
To: edk2-devel; +Cc: Paulo Alcantara, Eric Dong, Ruiyu Ni, Star Zeng, Laszlo Ersek
This patch reworks the driver to support Partition driver changes.
Cc: Eric Dong <eric.dong@intel.com>
Cc: Paulo Alcantara <pcacjr@zytor.com>
Cc: Ruiyu Ni <ruiyu.ni@intel.com>
Cc: Star Zeng <star.zeng@intel.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
---
MdeModulePkg/Universal/Disk/UdfDxe/File.c | 13 +-
MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515 ++++++++------------
MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 88 +---
4 files changed, 204 insertions(+), 419 deletions(-)
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
index 01361141bb..f2c62967e8 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -100,6 +100,7 @@ UdfOpenVolume (
&PrivFsData->Volume,
&PrivFsData->Root
);
+ ASSERT_EFI_ERROR (Status);
if (EFI_ERROR (Status)) {
goto Error_Find_Root_Dir;
}
@@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
CleanupFileInformation (&PrivFsData->Root);
Error_Find_Root_Dir:
- CleanupVolumeInformation (&PrivFsData->Volume);
Error_Read_Udf_Volume:
Error_Invalid_Params:
@@ -528,7 +528,6 @@ UdfClose (
EFI_TPL OldTpl;
EFI_STATUS Status;
PRIVATE_UDF_FILE_DATA *PrivFileData;
- PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
@@ -541,8 +540,6 @@ UdfClose (
PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
- PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData->SimpleFs);
-
if (!PrivFileData->IsRootDirectory) {
CleanupFileInformation (&PrivFileData->File);
@@ -551,10 +548,6 @@ UdfClose (
}
}
- if (--PrivFsData->OpenFiles == 0) {
- CleanupVolumeInformation (&PrivFsData->Volume);
- }
-
FreePool ((VOID *)PrivFileData);
Exit:
@@ -787,7 +780,7 @@ UdfGetInfo (
} else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
String = VolumeLabel;
- FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
+ FileSetDesc = &PrivFsData->Volume.FileSetDesc;
OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
@@ -846,7 +839,7 @@ UdfGetInfo (
FileSystemInfo->Size = FileSystemInfoLength;
FileSystemInfo->ReadOnly = TRUE;
FileSystemInfo->BlockSize =
- LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
+ PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
FileSystemInfo->VolumeSize = VolumeSize;
FileSystemInfo->FreeSpace = FreeSpaceSize;
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
index 4609580b30..63b643e60a 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
@@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
EFI_STATUS
StartMainVolumeDescriptorSequence (
- IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
- IN EFI_DISK_IO_PROTOCOL *DiskIo,
- IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
- OUT UDF_VOLUME_INFO *Volume
+ IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
+ IN EFI_DISK_IO_PROTOCOL *DiskIo,
+ OUT UDF_VOLUME_INFO *Volume
)
{
- EFI_STATUS Status;
- UINT32 BlockSize;
- UDF_EXTENT_AD *ExtentAd;
- UINT64 StartingLsn;
- UINT64 EndingLsn;
- VOID *Buffer;
- UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
- UDF_PARTITION_DESCRIPTOR *PartitionDesc;
- UINTN Index;
- UINT32 LogicalBlockSize;
+ EFI_STATUS Status;
+ UINT32 BlockSize;
+ UINT64 BlockOffset;
+ VOID *Buffer;
+ UINT32 LogicalBlockSize;
+
+ BlockSize = BlockIo->Media->BlockSize;
//
- // We've already found an ADVP on the volume. It contains the extent
- // (MainVolumeDescriptorSequenceExtent) where the Main Volume Descriptor
- // Sequence starts. Therefore, we'll look for Logical Volume Descriptors and
- // Partitions Descriptors and save them in memory, accordingly.
- //
- // Note also that each descriptor will be aligned on a block size (BlockSize)
- // boundary, so we need to read one block at a time.
+ // Allocate buffer for reading disk blocks
//
- BlockSize = BlockIo->Media->BlockSize;
- ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
- StartingLsn = (UINT64)ExtentAd->ExtentLocation;
- EndingLsn = StartingLsn + DivU64x32 (
- (UINT64)ExtentAd->ExtentLength,
- BlockSize
- );
-
- Volume->LogicalVolDescs =
- (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool (ExtentAd->ExtentLength);
- if (Volume->LogicalVolDescs == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- Volume->PartitionDescs =
- (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd->ExtentLength);
- if (Volume->PartitionDescs == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Error_Alloc_Pds;
- }
-
- Buffer = AllocateZeroPool (BlockSize);
+ Buffer = AllocateZeroPool ((UINTN)BlockSize);
if (Buffer == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Error_Alloc_Buf;
+ return EFI_OUT_OF_RESOURCES;
}
- Volume->LogicalVolDescsNo = 0;
- Volume->PartitionDescsNo = 0;
-
- while (StartingLsn <= EndingLsn) {
+ //
+ // As per UDF 2.60 specification:
+ //
+ // ---
+ // For the purpose of interchange, Partition Maps shall be limited to
+ // Partition Map type 1, except type 2 maps.
+ // ---
+ // A Partition Descriptor Access Type of read-only, rewritable,
+ // overwritable, write-once and pseudo-overwritable shall be
+ // supported. There shall be exactly one prevailing Partition
+ // Descriptor recorded per volume, with one exception. For Volume
+ // Sets that consist of single volume, the volume may contain 2 non-
+ // overlapping Partitions with 2 prevailing Partition Descriptors only
+ // if one has an Access Type of read-only and the other has an
+ // Access Type of rewritable, overwritable, or write-once. The
+ // Logical Volume for this volume would consist of the contents of
+ // both partitions.
+ // ---
+ //
+ // The logical partition created by Partition driver is relative to the main
+ // VDS extent location, so we start the Main Volume Descriptor Sequence at
+ // LBA 0.
+ //
+ // We don't need to make sure we got both LVD and PD -- that is, if we are
+ // here, that means Partition driver was able to find them both previously.
+ //
+ for (BlockOffset = 0;
+ BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
+ BlockIo->Media->BlockSize);
+ BlockOffset += BlockSize) {
+ // Read disk block
+ //
Status = DiskIo->ReadDisk (
DiskIo,
BlockIo->Media->MediaId,
- MultU64x32 (StartingLsn, BlockSize),
+ BlockOffset,
BlockSize,
Buffer
);
if (EFI_ERROR (Status)) {
- goto Error_Read_Disk_Blk;
+ goto Out_Free;
}
+ //
+ // Check if read block is a Terminating Descriptor
+ //
if (IS_TD (Buffer)) {
//
- // Found a Terminating Descriptor. Stop the sequence then.
+ // Terminate Main Volume Descriptor Sequence
//
break;
}
if (IS_LVD (Buffer)) {
//
- // Found a Logical Volume Descriptor.
+ // Save Logical Volume Descriptor
//
- LogicalVolDesc =
- (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
- AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
- if (LogicalVolDesc == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Error_Alloc_Lvd;
- }
-
- CopyMem ((VOID *)LogicalVolDesc, Buffer,
- sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
- Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] = LogicalVolDesc;
+ CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof (Volume->LogicalVolDesc));
} else if (IS_PD (Buffer)) {
//
- // Found a Partition Descriptor.
+ // Save Partition Descriptor
//
- PartitionDesc =
- (UDF_PARTITION_DESCRIPTOR *)
- AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
- if (PartitionDesc == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Error_Alloc_Pd;
- }
-
- CopyMem ((VOID *)PartitionDesc, Buffer,
- sizeof (UDF_PARTITION_DESCRIPTOR));
- Volume->PartitionDescs[Volume->PartitionDescsNo++] = PartitionDesc;
+ CopyMem (&Volume->PartitionDesc, Buffer, sizeof (Volume->PartitionDesc));
}
-
- StartingLsn++;
}
//
- // When an UDF volume (revision 2.00 or higher) contains a File Entry rather
- // than an Extended File Entry (which is not recommended as per spec), we need
- // to make sure the size of a FE will be _at least_ 2048
- // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward compatibility.
+ // Determine FE (File Entry) size
//
- LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+ LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
- Volume->FileEntrySize = LogicalBlockSize;
+ Volume->FileEntrySize = (UINTN)LogicalBlockSize;
} else {
Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
}
- FreePool (Buffer);
-
- return EFI_SUCCESS;
-
-Error_Alloc_Pd:
-Error_Alloc_Lvd:
- for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
- FreePool ((VOID *)Volume->PartitionDescs[Index]);
- }
-
- for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
- FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
- }
+ Status = EFI_SUCCESS;
-Error_Read_Disk_Blk:
+Out_Free:
+ //
+ // Free block read buffer
+ //
FreePool (Buffer);
-Error_Alloc_Buf:
- FreePool ((VOID *)Volume->PartitionDescs);
- Volume->PartitionDescs = NULL;
-
-Error_Alloc_Pds:
- FreePool ((VOID *)Volume->LogicalVolDescs);
- Volume->LogicalVolDescs = NULL;
-
return Status;
}
@@ -223,11 +180,9 @@ GetPdFromLongAd (
)
{
UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
- UINTN Index;
- UDF_PARTITION_DESCRIPTOR *PartitionDesc;
UINT16 PartitionNum;
- LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
+ LogicalVolDesc = &Volume->LogicalVolDesc;
switch (LV_UDF_REVISION (LogicalVolDesc)) {
case 0x0102:
@@ -252,19 +207,21 @@ GetPdFromLongAd (
PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
break;
case 0x0260:
- //
- // Fall through.
- //
- default:
PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
break;
+ default:
+ //
+ // Unhandled UDF revision
+ //
+ return NULL;
}
- for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
- PartitionDesc = Volume->PartitionDescs[Index];
- if (PartitionDesc->PartitionNumber == PartitionNum) {
- return PartitionDesc;
- }
+ //
+ // Check if partition number matches Partition Descriptor found in Main Volume
+ // Descriptor Sequence.
+ //
+ if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
+ return &Volume->PartitionDesc;
}
return NULL;
@@ -284,8 +241,9 @@ GetLongAdLsn (
PartitionDesc = GetPdFromLongAd (Volume, LongAd);
ASSERT (PartitionDesc != NULL);
- return (UINT64)PartitionDesc->PartitionStartingLocation +
- LongAd->ExtentLocation.LogicalBlockNumber;
+ return (UINT64)PartitionDesc->PartitionStartingLocation -
+ Volume->MainVdsStartLocation +
+ LongAd->ExtentLocation.LogicalBlockNumber;
}
//
@@ -293,12 +251,13 @@ GetLongAdLsn (
//
UINT64
GetShortAdLsn (
+ IN UDF_VOLUME_INFO *Volume,
IN UDF_PARTITION_DESCRIPTOR *PartitionDesc,
IN UDF_SHORT_ALLOCATION_DESCRIPTOR *ShortAd
)
{
- return (UINT64)PartitionDesc->PartitionStartingLocation +
- ShortAd->ExtentPosition;
+ return (UINT64)PartitionDesc->PartitionStartingLocation -
+ Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
}
//
@@ -311,36 +270,39 @@ EFI_STATUS
FindFileSetDescriptor (
IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
IN EFI_DISK_IO_PROTOCOL *DiskIo,
- IN UDF_VOLUME_INFO *Volume,
- IN UINTN LogicalVolDescNum,
- OUT UDF_FILE_SET_DESCRIPTOR *FileSetDesc
+ IN UDF_VOLUME_INFO *Volume
)
{
EFI_STATUS Status;
UINT64 Lsn;
UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
- LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
+ LogicalVolDesc = &Volume->LogicalVolDesc;
Lsn = GetLongAdLsn (Volume, &LogicalVolDesc->LogicalVolumeContentsUse);
//
- // Read extent (Long Ad).
+ // As per UDF 2.60 specification:
+ //
+ // There shall be exactly one File Set Descriptor recorded per Logical
+ // Volume.
+ //
+ // Read disk block
//
Status = DiskIo->ReadDisk (
DiskIo,
BlockIo->Media->MediaId,
MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
- sizeof (UDF_FILE_SET_DESCRIPTOR),
- (VOID *)FileSetDesc
+ sizeof (Volume->FileSetDesc),
+ &Volume->FileSetDesc
);
if (EFI_ERROR (Status)) {
return Status;
}
//
- // Check if the read extent contains a valid FSD's tag identifier.
+ // Check if read block is a File Set Descriptor
//
- if (!IS_FSD (FileSetDesc)) {
+ if (!IS_FSD (&Volume->FileSetDesc)) {
return EFI_VOLUME_CORRUPTED;
}
@@ -348,71 +310,6 @@ FindFileSetDescriptor (
}
//
-// Get all File Set Descriptors for each Logical Volume Descriptor.
-//
-EFI_STATUS
-GetFileSetDescriptors (
- IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
- IN EFI_DISK_IO_PROTOCOL *DiskIo,
- IN OUT UDF_VOLUME_INFO *Volume
- )
-{
- EFI_STATUS Status;
- UINTN Index;
- UDF_FILE_SET_DESCRIPTOR *FileSetDesc;
- UINTN Count;
-
- Volume->FileSetDescs =
- (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
- Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
- if (Volume->FileSetDescs == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
-
- for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
- FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
- if (FileSetDesc == NULL) {
- Status = EFI_OUT_OF_RESOURCES;
- goto Error_Alloc_Fsd;
- }
-
- //
- // Find a FSD for this LVD.
- //
- Status = FindFileSetDescriptor (
- BlockIo,
- DiskIo,
- Volume,
- Index,
- FileSetDesc
- );
- if (EFI_ERROR (Status)) {
- goto Error_Find_Fsd;
- }
-
- //
- // Got one. Save it.
- //
- Volume->FileSetDescs[Index] = FileSetDesc;
- }
-
- Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
- return EFI_SUCCESS;
-
-Error_Find_Fsd:
- Count = Index + 1;
- for (Index = 0; Index < Count; Index++) {
- FreePool ((VOID *)Volume->FileSetDescs[Index]);
- }
-
- FreePool ((VOID *)Volume->FileSetDescs);
- Volume->FileSetDescs = NULL;
-
-Error_Alloc_Fsd:
- return Status;
-}
-
-//
// Read Volume and File Structure on an UDF file system.
//
EFI_STATUS
@@ -424,9 +321,10 @@ ReadVolumeFileStructure (
{
EFI_STATUS Status;
UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
+ UDF_EXTENT_AD *ExtentAd;
//
- // Find an AVDP.
+ // Find Anchor Volume Descriptor Pointer
//
Status = FindAnchorVolumeDescriptorPointer (
BlockIo,
@@ -438,12 +336,18 @@ ReadVolumeFileStructure (
}
//
- // AVDP has been found. Start MVDS.
+ // Save Main VDS start block number
+ //
+ ExtentAd = &AnchorPoint.MainVolumeDescriptorSequenceExtent;
+
+ Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
+
+ //
+ // Start Main Volume Descriptor Sequence.
//
Status = StartMainVolumeDescriptorSequence (
BlockIo,
DiskIo,
- &AnchorPoint,
Volume
);
if (EFI_ERROR (Status)) {
@@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR *)Ad);
} else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
return GetShortAdLsn (
+ Volume,
GetPdFromLongAd (Volume, ParentIcb),
(UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
);
@@ -740,7 +645,7 @@ GetAedAdsOffset (
return EFI_OUT_OF_RESOURCES;
}
- LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+ LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
//
// Read extent.
@@ -890,7 +795,7 @@ ReadFile (
UINT32 ExtentLength;
UDF_FE_RECORDING_FLAGS RecordingFlags;
- LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+ LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
DoFreeAed = FALSE;
//
@@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation (
{
EFI_STATUS Status;
+ //
+ // Read all necessary UDF volume information and keep it private to the driver
+ //
Status = ReadVolumeFileStructure (
BlockIo,
DiskIo,
@@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
return Status;
}
- Status = GetFileSetDescriptors (
- BlockIo,
- DiskIo,
- Volume
- );
+ //
+ // Find File Set Descriptor
+ //
+ Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
if (EFI_ERROR (Status)) {
- CleanupVolumeInformation (Volume);
+ return Status;
}
return Status;
@@ -1407,7 +1314,7 @@ FindRootDirectory (
BlockIo,
DiskIo,
Volume,
- &Volume->FileSetDescs[0]->RootDirectoryIcb,
+ &Volume->FileSetDesc.RootDirectoryIcb,
&File->FileEntry
);
if (EFI_ERROR (Status)) {
@@ -1424,7 +1331,7 @@ FindRootDirectory (
L"\\",
NULL,
&Parent,
- &Volume->FileSetDescs[0]->RootDirectoryIcb,
+ &Volume->FileSetDesc.RootDirectoryIcb,
File
);
if (EFI_ERROR (Status)) {
@@ -1465,7 +1372,7 @@ FindFileEntry (
UINT32 LogicalBlockSize;
Lsn = GetLongAdLsn (Volume, Icb);
- LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
+ LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
*FileEntry = AllocateZeroPool (Volume->FileEntrySize);
if (*FileEntry == NULL) {
@@ -1959,43 +1866,6 @@ Error_Find_File:
}
/**
- Clean up in-memory UDF volume information.
-
- @param[in] Volume Volume information pointer.
-
-**/
-VOID
-CleanupVolumeInformation (
- IN UDF_VOLUME_INFO *Volume
- )
-{
- UINTN Index;
-
- if (Volume->LogicalVolDescs != NULL) {
- for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
- FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
- }
- FreePool ((VOID *)Volume->LogicalVolDescs);
- }
-
- if (Volume->PartitionDescs != NULL) {
- for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
- FreePool ((VOID *)Volume->PartitionDescs[Index]);
- }
- FreePool ((VOID *)Volume->PartitionDescs);
- }
-
- if (Volume->FileSetDescs != NULL) {
- for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
- FreePool ((VOID *)Volume->FileSetDescs[Index]);
- }
- FreePool ((VOID *)Volume->FileSetDescs);
- }
-
- ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO));
-}
-
-/**
Clean up in-memory UDF file information.
@param[in] File File information pointer.
@@ -2249,91 +2119,100 @@ GetVolumeSize (
OUT UINT64 *FreeSpaceSize
)
{
- UDF_EXTENT_AD ExtentAd;
- UINT32 LogicalBlockSize;
- UINT64 Lsn;
- EFI_STATUS Status;
- UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
- UINTN Index;
- UINTN Length;
- UINT32 LsnsNo;
-
- *VolumeSize = 0;
- *FreeSpaceSize = 0;
-
- for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
- CopyMem ((VOID *)&ExtentAd,
- (VOID *)&Volume->LogicalVolDescs[Index]->IntegritySequenceExtent,
- sizeof (UDF_EXTENT_AD));
- if (ExtentAd.ExtentLength == 0) {
- continue;
- }
+ EFI_STATUS Status;
+ UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
+ UDF_EXTENT_AD *ExtentAd;
+ UINT64 Lsn;
+ UINT32 LogicalBlockSize;
+ UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
+ UINTN Index;
+ UINTN Length;
+ UINT32 LsnsNo;
- LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
+ LogicalVolDesc = &Volume->LogicalVolDesc;
- Read_Next_Sequence:
- LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
- AllocatePool (ExtentAd.ExtentLength);
- if (LogicalVolInt == NULL) {
- return EFI_OUT_OF_RESOURCES;
- }
+ ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
- Lsn = (UINT64)ExtentAd.ExtentLocation;
+ if (ExtentAd->ExtentLength == 0) {
+ return EFI_VOLUME_CORRUPTED;
+ }
- Status = DiskIo->ReadDisk (
- DiskIo,
- BlockIo->Media->MediaId,
- MultU64x32 (Lsn, LogicalBlockSize),
- ExtentAd.ExtentLength,
- (VOID *)LogicalVolInt
- );
- if (EFI_ERROR (Status)) {
- FreePool ((VOID *)LogicalVolInt);
- return Status;
- }
+ LogicalVolInt = AllocatePool (ExtentAd->ExtentLength);
+ if (LogicalVolInt == NULL) {
+ return EFI_OUT_OF_RESOURCES;
+ }
- if (!IS_LVID (LogicalVolInt)) {
- FreePool ((VOID *)LogicalVolInt);
- return EFI_VOLUME_CORRUPTED;
- }
+ //
+ // Get location of Logical Volume Integrity Descriptor
+ //
+ Lsn = (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
- Length = LogicalVolInt->NumberOfPartitions;
- for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
- LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
- if (LsnsNo == 0xFFFFFFFFUL) {
- //
- // Size not specified.
- //
- continue;
- }
+ LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
- *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
- }
+ //
+ // Read disk block
+ //
+ Status = DiskIo->ReadDisk (
+ DiskIo,
+ BlockIo->Media->MediaId,
+ MultU64x32 (Lsn, LogicalBlockSize),
+ ExtentAd->ExtentLength,
+ LogicalVolInt
+ );
+ if (EFI_ERROR (Status)) {
+ goto Out_Free;
+ }
- Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
- for (; Index < Length; Index += sizeof (UINT32)) {
- LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
- if (LsnsNo == 0xFFFFFFFFUL) {
- //
- // Size not specified.
- //
- continue;
- }
+ //
+ // Check if read block is a Logical Volume Integrity Descriptor
+ //
+ if (!IS_LVID (LogicalVolInt)) {
+ Status = EFI_VOLUME_CORRUPTED;
+ goto Out_Free;
+ }
- *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
- }
+ *VolumeSize = 0;
+ *FreeSpaceSize = 0;
- CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt->NextIntegrityExtent,
- sizeof (UDF_EXTENT_AD));
- if (ExtentAd.ExtentLength > 0) {
- FreePool ((VOID *)LogicalVolInt);
- goto Read_Next_Sequence;
+ Length = LogicalVolInt->NumberOfPartitions;
+ for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
+ LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
+ //
+ // Check if size is not specified
+ //
+ if (LsnsNo == 0xFFFFFFFFUL) {
+ continue;
}
+ //
+ // Accumulate free space size
+ //
+ *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
+ }
- FreePool ((VOID *)LogicalVolInt);
+ Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
+ for (; Index < Length; Index += sizeof (UINT32)) {
+ LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
+ //
+ // Check if size is not specified
+ //
+ if (LsnsNo == 0xFFFFFFFFUL) {
+ continue;
+ }
+ //
+ // Accumulate used volume space
+ //
+ *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
}
- return EFI_SUCCESS;
+ Status = EFI_SUCCESS;
+
+Out_Free:
+ //
+ // Free Logical Volume Integrity Descriptor
+ //
+ FreePool (LogicalVolInt);
+
+ return Status;
}
/**
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
index 49dc7077b7..d4163b89ca 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
@@ -276,13 +276,6 @@ UdfDriverBindingStop (
NULL
);
- //
- // Check if there's any open file. If so, clean them up.
- //
- if (PrivFsData->OpenFiles > 0) {
- CleanupVolumeInformation (&PrivFsData->Volume);
- }
-
FreePool ((VOID *)PrivFsData);
}
diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
index 240d420ff5..c5f83914d8 100644
--- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
+++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
@@ -49,16 +49,8 @@
{ 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A } \
}
-#define UDF_DEFAULT_LV_NUM 0
-
#define IS_PVD(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1))
-#define IS_PD(_Pointer) \
- ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
-#define IS_LVD(_Pointer) \
- ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
-#define IS_TD(_Pointer) \
- ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
#define IS_FSD(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256))
#define IS_FE(_Pointer) \
@@ -152,14 +144,8 @@ typedef enum {
#define IS_VALID_COMPRESSION_ID(_CompId) \
((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
-#define LV_BLOCK_SIZE(_Vol, _LvNum) \
- (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
-
#define UDF_STANDARD_IDENTIFIER_LENGTH 5
-#define LV_UDF_REVISION(_Lv) \
- *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
-
#pragma pack(1)
typedef struct {
@@ -186,17 +172,6 @@ typedef struct {
#pragma pack(1)
typedef struct {
- UINT8 CharacterSetType;
- UINT8 CharacterSetInfo[63];
-} UDF_CHAR_SPEC;
-
-typedef struct {
- UINT8 Flags;
- UINT8 Identifier[23];
- UINT8 IdentifierSuffix[8];
-} UDF_ENTITY_ID;
-
-typedef struct {
UINT16 TypeAndTimezone;
INT16 Year;
UINT8 Month;
@@ -210,17 +185,6 @@ typedef struct {
} UDF_TIMESTAMP;
typedef struct {
- UINT32 LogicalBlockNumber;
- UINT16 PartitionReferenceNumber;
-} UDF_LB_ADDR;
-
-typedef struct {
- UINT32 ExtentLength;
- UDF_LB_ADDR ExtentLocation;
- UINT8 ImplementationUse[6];
-} UDF_LONG_ALLOCATION_DESCRIPTOR;
-
-typedef struct {
UDF_DESCRIPTOR_TAG DescriptorTag;
UINT32 PrevAllocationExtentDescriptor;
UINT32 LengthOfAllocationDescriptors;
@@ -235,37 +199,6 @@ typedef struct {
} UDF_VOLUME_DESCRIPTOR;
typedef struct {
- UDF_DESCRIPTOR_TAG DescriptorTag;
- UINT32 VolumeDescriptorSequenceNumber;
- UINT16 PartitionFlags;
- UINT16 PartitionNumber;
- UDF_ENTITY_ID PartitionContents;
- UINT8 PartitionContentsUse[128];
- UINT32 AccessType;
- UINT32 PartitionStartingLocation;
- UINT32 PartitionLength;
- UDF_ENTITY_ID ImplementationIdentifier;
- UINT8 ImplementationUse[128];
- UINT8 Reserved[156];
-} UDF_PARTITION_DESCRIPTOR;
-
-typedef struct {
- UDF_DESCRIPTOR_TAG DescriptorTag;
- UINT32 VolumeDescriptorSequenceNumber;
- UDF_CHAR_SPEC DescriptorCharacterSet;
- UINT8 LogicalVolumeIdentifier[128];
- UINT32 LogicalBlockSize;
- UDF_ENTITY_ID DomainIdentifier;
- UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
- UINT32 MapTableLength;
- UINT32 NumberOfPartitionMaps;
- UDF_ENTITY_ID ImplementationIdentifier;
- UINT8 ImplementationUse[128];
- UDF_EXTENT_AD IntegritySequenceExtent;
- UINT8 PartitionMaps[6];
-} UDF_LOGICAL_VOLUME_DESCRIPTOR;
-
-typedef struct {
UDF_DESCRIPTOR_TAG DescriptorTag;
UDF_TIMESTAMP RecordingDateTime;
UINT32 IntegrityType;
@@ -389,12 +322,10 @@ typedef struct {
// UDF filesystem driver's private data
//
typedef struct {
- UDF_LOGICAL_VOLUME_DESCRIPTOR **LogicalVolDescs;
- UINTN LogicalVolDescsNo;
- UDF_PARTITION_DESCRIPTOR **PartitionDescs;
- UINTN PartitionDescsNo;
- UDF_FILE_SET_DESCRIPTOR **FileSetDescs;
- UINTN FileSetDescsNo;
+ UINT64 MainVdsStartLocation;
+ UDF_LOGICAL_VOLUME_DESCRIPTOR LogicalVolDesc;
+ UDF_PARTITION_DESCRIPTOR PartitionDesc;
+ UDF_FILE_SET_DESCRIPTOR FileSetDesc;
UINTN FileEntrySize;
} UDF_VOLUME_INFO;
@@ -883,17 +814,6 @@ ResolveSymlink (
);
/**
- Clean up in-memory UDF volume information.
-
- @param[in] Volume Volume information pointer.
-
-**/
-VOID
-CleanupVolumeInformation (
- IN UDF_VOLUME_INFO *Volume
- );
-
-/**
Clean up in-memory UDF file information.
@param[in] File File information pointer.
--
2.11.0
^ permalink raw reply related [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
2017-09-17 13:13 ` [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions Paulo Alcantara
@ 2017-09-18 0:28 ` Ni, Ruiyu
2017-09-18 0:42 ` Ni, Ruiyu
2017-09-18 13:50 ` Paulo Alcantara
0 siblings, 2 replies; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-18 0:28 UTC (permalink / raw)
To: Paulo Alcantara, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Gao, Liming, Laszlo Ersek
#define _GET_TAG_ID(_Pointer) \
(((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
#define IS_PD(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
#define IS_LVD(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
#define IS_TD(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
#define IS_AVDP(_Pointer) \
((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
Paulo,
If you take a look at Pci22.h in the same directory, all macros are started
as "IS_PCI_".
How about changing the above IS_xxx to IS_UDF_xxx?
Shall we change AVDP to AVD, following the same naming style as PD, LVD and TD?
How about changing _Pointer to _Tag or DescriptorTag?
Thanks/Ray
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
>
> This patch adds a fewe more UDF structures in order to detect Logical
> Volume and Partition descriptors during Main Volume Descriptor Sequence
> in Partition driver.
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
> MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
> 1 file changed, 63 insertions(+)
>
> diff --git a/MdePkg/Include/IndustryStandard/Udf.h
> b/MdePkg/Include/IndustryStandard/Udf.h
> index 0febb4bcda..6cc42f8543 100644
> --- a/MdePkg/Include/IndustryStandard/Udf.h
> +++ b/MdePkg/Include/IndustryStandard/Udf.h
> @@ -27,9 +27,19 @@
> #define _GET_TAG_ID(_Pointer) \
> (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>
> +#define IS_PD(_Pointer) \
> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> +
> #define IS_AVDP(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>
> +#define LV_UDF_REVISION(_Lv) \
> + *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> +
> #pragma pack(1)
>
> typedef struct {
> @@ -55,6 +65,59 @@ typedef struct {
> UINT8 Reserved[480];
> } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
>
> +typedef struct {
> + UINT8 CharacterSetType;
> + UINT8 CharacterSetInfo[63];
> +} UDF_CHAR_SPEC;
> +
> +typedef struct {
> + UINT8 Flags;
> + UINT8 Identifier[23];
> + UINT8 IdentifierSuffix[8];
> +} UDF_ENTITY_ID;
> +
> +typedef struct {
> + UINT32 LogicalBlockNumber;
> + UINT16 PartitionReferenceNumber;
> +} UDF_LB_ADDR;
> +
> +typedef struct {
> + UINT32 ExtentLength;
> + UDF_LB_ADDR ExtentLocation;
> + UINT8 ImplementationUse[6];
> +} UDF_LONG_ALLOCATION_DESCRIPTOR;
> +
> +typedef struct {
> + UDF_DESCRIPTOR_TAG DescriptorTag;
> + UINT32 VolumeDescriptorSequenceNumber;
> + UDF_CHAR_SPEC DescriptorCharacterSet;
> + UINT8 LogicalVolumeIdentifier[128];
> + UINT32 LogicalBlockSize;
> + UDF_ENTITY_ID DomainIdentifier;
> + UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
> + UINT32 MapTableLength;
> + UINT32 NumberOfPartitionMaps;
> + UDF_ENTITY_ID ImplementationIdentifier;
> + UINT8 ImplementationUse[128];
> + UDF_EXTENT_AD IntegritySequenceExtent;
> + UINT8 PartitionMaps[6];
> +} UDF_LOGICAL_VOLUME_DESCRIPTOR;
> +
> +typedef struct {
> + UDF_DESCRIPTOR_TAG DescriptorTag;
> + UINT32 VolumeDescriptorSequenceNumber;
> + UINT16 PartitionFlags;
> + UINT16 PartitionNumber;
> + UDF_ENTITY_ID PartitionContents;
> + UINT8 PartitionContentsUse[128];
> + UINT32 AccessType;
> + UINT32 PartitionStartingLocation;
> + UINT32 PartitionLength;
> + UDF_ENTITY_ID ImplementationIdentifier;
> + UINT8 ImplementationUse[128];
> + UINT8 Reserved[156];
> +} UDF_PARTITION_DESCRIPTOR;
> +
> #pragma pack()
>
> #endif
> --
> 2.11.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
2017-09-18 0:28 ` Ni, Ruiyu
@ 2017-09-18 0:42 ` Ni, Ruiyu
2017-09-18 13:52 ` Paulo Alcantara
2017-09-18 13:50 ` Paulo Alcantara
1 sibling, 1 reply; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-18 0:42 UTC (permalink / raw)
To: Ni, Ruiyu, Paulo Alcantara, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Laszlo Ersek, Gao, Liming
Continue reading the patch #2, I think we can change IS_PD to:
#define IS_UDF_PD(Tag) ((Tag)->TagIdentifier == 5)
Using the above way, we can avoid caller to supply an invalid buffer.
Thanks/Ray
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
> Ruiyu
> Sent: Monday, September 18, 2017 8:29 AM
> To: Paulo Alcantara <pcacjr@zytor.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
> Subject: Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure
> definitions
>
> #define _GET_TAG_ID(_Pointer) \
> (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>
> #define IS_PD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
> #define IS_LVD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
> #define IS_TD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>
> #define IS_AVDP(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>
> Paulo,
> If you take a look at Pci22.h in the same directory, all macros are started
> as "IS_PCI_".
> How about changing the above IS_xxx to IS_UDF_xxx?
> Shall we change AVDP to AVD, following the same naming style as PD, LVD
> and TD?
> How about changing _Pointer to _Tag or DescriptorTag?
>
>
> Thanks/Ray
>
> > -----Original Message-----
> > From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> > Sent: Sunday, September 17, 2017 9:13 PM
> > To: edk2-devel@lists.01.org
> > Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
> > <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
> Laszlo
> > Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
> > Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
> >
> > This patch adds a fewe more UDF structures in order to detect Logical
> > Volume and Partition descriptors during Main Volume Descriptor Sequence
> > in Partition driver.
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Laszlo Ersek <lersek@redhat.com>
> > Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> > ---
> > MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
> > 1 file changed, 63 insertions(+)
> >
> > diff --git a/MdePkg/Include/IndustryStandard/Udf.h
> > b/MdePkg/Include/IndustryStandard/Udf.h
> > index 0febb4bcda..6cc42f8543 100644
> > --- a/MdePkg/Include/IndustryStandard/Udf.h
> > +++ b/MdePkg/Include/IndustryStandard/Udf.h
> > @@ -27,9 +27,19 @@
> > #define _GET_TAG_ID(_Pointer) \
> > (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
> >
> > +#define IS_PD(_Pointer) \
> > + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
> > + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
> > + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
> > +
> > #define IS_AVDP(_Pointer) \
> > ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
> >
> > +#define LV_UDF_REVISION(_Lv) \
> > + *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> > +
> > #pragma pack(1)
> >
> > typedef struct {
> > @@ -55,6 +65,59 @@ typedef struct {
> > UINT8 Reserved[480];
> > } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
> >
> > +typedef struct {
> > + UINT8 CharacterSetType;
> > + UINT8 CharacterSetInfo[63];
> > +} UDF_CHAR_SPEC;
> > +
> > +typedef struct {
> > + UINT8 Flags;
> > + UINT8 Identifier[23];
> > + UINT8 IdentifierSuffix[8];
> > +} UDF_ENTITY_ID;
> > +
> > +typedef struct {
> > + UINT32 LogicalBlockNumber;
> > + UINT16 PartitionReferenceNumber;
> > +} UDF_LB_ADDR;
> > +
> > +typedef struct {
> > + UINT32 ExtentLength;
> > + UDF_LB_ADDR ExtentLocation;
> > + UINT8 ImplementationUse[6];
> > +} UDF_LONG_ALLOCATION_DESCRIPTOR;
> > +
> > +typedef struct {
> > + UDF_DESCRIPTOR_TAG DescriptorTag;
> > + UINT32 VolumeDescriptorSequenceNumber;
> > + UDF_CHAR_SPEC DescriptorCharacterSet;
> > + UINT8 LogicalVolumeIdentifier[128];
> > + UINT32 LogicalBlockSize;
> > + UDF_ENTITY_ID DomainIdentifier;
> > + UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
> > + UINT32 MapTableLength;
> > + UINT32 NumberOfPartitionMaps;
> > + UDF_ENTITY_ID ImplementationIdentifier;
> > + UINT8 ImplementationUse[128];
> > + UDF_EXTENT_AD IntegritySequenceExtent;
> > + UINT8 PartitionMaps[6];
> > +} UDF_LOGICAL_VOLUME_DESCRIPTOR;
> > +
> > +typedef struct {
> > + UDF_DESCRIPTOR_TAG DescriptorTag;
> > + UINT32 VolumeDescriptorSequenceNumber;
> > + UINT16 PartitionFlags;
> > + UINT16 PartitionNumber;
> > + UDF_ENTITY_ID PartitionContents;
> > + UINT8 PartitionContentsUse[128];
> > + UINT32 AccessType;
> > + UINT32 PartitionStartingLocation;
> > + UINT32 PartitionLength;
> > + UDF_ENTITY_ID ImplementationIdentifier;
> > + UINT8 ImplementationUse[128];
> > + UINT8 Reserved[156];
> > +} UDF_PARTITION_DESCRIPTOR;
> > +
> > #pragma pack()
> >
> > #endif
> > --
> > 2.11.0
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-17 13:13 ` [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition Paulo Alcantara
@ 2017-09-18 0:54 ` Ni, Ruiyu
2017-09-18 16:38 ` Paulo Alcantara
0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-18 0:54 UTC (permalink / raw)
To: Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star, Laszlo Ersek
Paulo,
Several comments:
1. Can below logic be removed in PartitionDxe/Udf.c?
while (!IsDevicePathEnd (DevicePathNode)) {
//
// Do not allow checking for UDF file systems in CDROM "El Torito"
// partitions, and skip duplicate installation of UDF file system child
// nodes.
//
if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
return EFI_NOT_FOUND;
}
if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
return EFI_NOT_FOUND;
}
}
}
//
// Try next device path node
//
DevicePathNode = NextDevicePathNode (DevicePathNode);
}
2. Can you add function header comments for the newly added functions?
3. Change the consuming code of IS_UDF_XXX macro after patch #1 is changed.
4. Perhaps to add more checks for these numbers read from the UDF CDROM.
As Jiewen mentioned in another mail, secure code needs to validate all external inputs.
We shouldn't assume the UDF CDROM is a well-formatted CDROM.
A hacker may create a UDF CDROM to hack the system by using mechanism like
buffer overflow, integer overflow, etc.
Thanks/Ray
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF
> logical partition
>
> Do not use entire block device size for the UDF logical partition, instead
> reserve the appropriate space (LVD space) for it.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
> ++++++++++++++++++--
> 1 file changed, 299 insertions(+), 24 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> index 7856b5dfc1..3e84882922 100644
> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> @@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
> return EFI_VOLUME_CORRUPTED;
> }
>
> -/**
> - Check if block device supports a valid UDF file system as specified by OSTA
> - Universal Disk Format Specification 2.60.
> -
> - @param[in] BlockIo BlockIo interface.
> - @param[in] DiskIo DiskIo interface.
> -
> - @retval EFI_SUCCESS UDF file system found.
> - @retval EFI_UNSUPPORTED UDF file system not found.
> - @retval EFI_NO_MEDIA The device has no media.
> - @retval EFI_DEVICE_ERROR The device reported an error.
> - @retval EFI_VOLUME_CORRUPTED The file system structures are
> corrupted.
> - @retval EFI_OUT_OF_RESOURCES The scan was not successful due to lack
> of
> - resources.
> -
> -**/
> EFI_STATUS
> -SupportUdfFileSystem (
> +FindUdfVolumeIdentifiers (
> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> IN EFI_DISK_IO_PROTOCOL *DiskIo
> )
> @@ -111,7 +95,6 @@ SupportUdfFileSystem (
> UINT64 EndDiskOffset;
> CDROM_VOLUME_DESCRIPTOR VolDescriptor;
> CDROM_VOLUME_DESCRIPTOR TerminatingVolDescriptor;
> - UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
>
> ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof
> (CDROM_VOLUME_DESCRIPTOR));
>
> @@ -207,12 +190,302 @@ SupportUdfFileSystem (
> return EFI_UNSUPPORTED;
> }
>
> + return EFI_SUCCESS;
> +}
> +
> +EFI_STATUS
> +GetPartitionNumber (
> + IN UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc,
> + OUT UINT16 *PartitionNum
> + )
> +{
> + EFI_STATUS Status;
> + UDF_LONG_ALLOCATION_DESCRIPTOR *LongAd;
> +
> + Status = EFI_SUCCESS;
> +
> + switch (LV_UDF_REVISION (LogicalVolDesc)) { case 0x0102:
> + //
> + // UDF 1.20 only supports Type 1 Partition
> + //
> + *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >PartitionMaps[4]);
> + break;
> + case 0x0150:
> + //
> + // Ensure Type 1 Partition map
> + //
> + ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
> + LogicalVolDesc->PartitionMaps[1] == 6);
> + *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >PartitionMaps[4]);
> + break;
> + case 0x0260:
> + LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
> + *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
> + break;
> + default:
> + //
> + // Unhandled UDF revision
> + //
> + Status = EFI_VOLUME_CORRUPTED;
> + break;
> + }
> +
> + return Status;
> +}
> +
> +EFI_STATUS
> +FindLogicalVolumeLocation (
> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> + IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
> + OUT UINT64 *MainVdsStartLsn,
> + OUT UINT64 *LogicalVolEndLsn
> + )
> +{
> + EFI_STATUS Status;
> + UINT32 BlockSize;
> + EFI_LBA LastBlock;
> + UDF_EXTENT_AD *ExtentAd;
> + UINT64 StartingLsn;
> + UINT64 EndingLsn;
> + VOID *Buffer;
> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> + UDF_PARTITION_DESCRIPTOR *PartitionDesc;
> + UINT64 GuardMainVdsStartLsn;
> + UINT16 PartitionNum;
> +
> + BlockSize = BlockIo->Media->BlockSize;
> + LastBlock = BlockIo->Media->LastBlock;
> + ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
> + StartingLsn = (UINT64)ExtentAd->ExtentLocation;
> + EndingLsn =
> + StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength,
> + BlockSize);
> +
> + LogicalVolDesc = NULL;
> + PartitionDesc = NULL;
> + GuardMainVdsStartLsn = StartingLsn;
> +
> + //
> + // Allocate buffer for reading disk blocks // Buffer =
> + AllocateZeroPool (BlockSize); if (Buffer == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
> +
> + Status = EFI_VOLUME_CORRUPTED;
> +
> + //
> + // As per UDF 2.60 specification:
> + //
> + // There shall be exactly one prevailing Logical Volume Descriptor
> + // recorded per Volume Set.
> + //
> + // Start Main Volume Descriptor Sequence and find Logical Volume
> + Descriptor // while (StartingLsn <= EndingLsn) {
> + //
> + // Read disk block
> + //
> + Status = DiskIo->ReadDisk (
> + DiskIo,
> + BlockIo->Media->MediaId,
> + MultU64x32 (StartingLsn, BlockSize),
> + BlockSize,
> + Buffer
> + );
> + if (EFI_ERROR (Status)) {
> + goto Out_Free;
> + }
> +
> + //
> + // Check if read block is a Terminating Descriptor
> + //
> + if (IS_TD (Buffer)) {
> + //
> + // Stop Main Volume Descriptor Sequence
> + //
> + break;
> + }
> +
> + //
> + // Check if read block is a Logical Volume Descriptor
> + //
> + if (IS_LVD (Buffer)) {
> + //
> + // Ensure only one LVD (Logical Volume Descriptor) is handled
> + //
> + if (LogicalVolDesc != NULL) {
> + Status = EFI_UNSUPPORTED;
> + goto Out_Free;
> + }
> +
> + //
> + // As per UDF 2.60 specification:
> + //
> + // For the purpose of interchange, Partition Maps shall be limited to
> + // Partition Map type 1, except type 2 maps.
> + //
> + // NOTE: Type 1 Partitions are the only supported in this implementation.
> + //
> + LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
> + if (LogicalVolDesc == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto Out_Free;
> + }
> +
> + //
> + // Save Logical Volume Descriptor
> + //
> + CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
> + } else if (IS_PD (Buffer)) {
> + //
> + // Ensure only one PD (Partition Descriptor) is handled
> + //
> + if (PartitionDesc != NULL) {
> + Status = EFI_UNSUPPORTED;
> + goto Out_Free;
> + }
> +
> + //
> + // Found a Partition Descriptor.
> + //
> + // As per UDF 2.60 specification:
> + //
> + // A Partition Descriptor Access Type of read-only, rewritable,
> + // overwritable, write-once and pseudo-overwritable shall be
> + // supported. There shall be exactly one prevailing Partition
> + // Descriptor recorded per volume, with one exception. For Volume
> + // Sets that consist of single volume, the volume may contain 2 non-
> + // overlapping Partitions with 2 prevailing Partition Descriptors only
> + // if one has an Access Type of read-only and the other has an
> + // Access Type of rewritable, overwritable, or write-once. The
> + // Logical Volume for this volume would consist of the contents of
> + // both partitions.
> + //
> + PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
> + if (PartitionDesc == NULL) {
> + Status = EFI_OUT_OF_RESOURCES;
> + goto Out_Free;
> + }
> +
> + //
> + // Save Partition Descriptor
> + //
> + CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
> + }
> + //
> + // Go to next disk block
> + //
> + StartingLsn++;
> + }
> +
> + Status = EFI_VOLUME_CORRUPTED;
> +
> + //
> + // Check if LVD and PD were found
> + //
> + if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
> + //
> + // Get partition number from Partition map in LVD descriptor
> + //
> + Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
> + if (EFI_ERROR (Status)) {
> + goto Out_Free;
> + }
> +
> + //
> + // Make sure we're handling expected Partition Descriptor
> + //
> + if (PartitionDesc->PartitionNumber != PartitionNum) {
> + Status = EFI_VOLUME_CORRUPTED;
> + goto Out_Free;
> + }
> +
> + //
> + // Cover the main VDS area so UdfDxe driver will also be able to get LVD
> and
> + // PD descriptors out from the file system.
> + //
> + *MainVdsStartLsn = GuardMainVdsStartLsn;
> + *LogicalVolEndLsn = *MainVdsStartLsn +
> + (UINT64)ExtentAd->ExtentLength;
> +
> + //
> + // Cover UDF partition area
> + //
> + *LogicalVolEndLsn +=
> + ((UINT64)PartitionDesc->PartitionStartingLocation -
> + *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
> + //
> + // Ensure to not attempt reading past end of device
> + //
> + if (*LogicalVolEndLsn > LastBlock) {
> + Status = EFI_VOLUME_CORRUPTED;
> + } else {
> + Status = EFI_SUCCESS;
> + }
> + }
> +
> +Out_Free:
> + //
> + // Free block read buffer
> + //
> + FreePool (Buffer);
> + //
> + // Free Logical Volume Descriptor
> + //
> + if (LogicalVolDesc != NULL) {
> + FreePool (LogicalVolDesc);
> + }
> + //
> + // Free Partition Descriptor
> + //
> + if (PartitionDesc != NULL) {
> + FreePool (PartitionDesc);
> + }
> +
> + return Status;
> +}
> +
> +EFI_STATUS
> +FindUdfLogicalVolume (
> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> + OUT EFI_LBA *StartingLBA,
> + OUT EFI_LBA *EndingLBA
> + )
> +{
> + EFI_STATUS Status;
> + UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
> +
> + //
> + // Find UDF volume identifiers
> + //
> + Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo); if (EFI_ERROR
> + (Status)) {
> + return Status;
> + }
> +
> + //
> + // Find Anchor Volume Descriptor Pointer //
> Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo,
> &AnchorPoint);
> if (EFI_ERROR (Status)) {
> - return EFI_UNSUPPORTED;
> + return Status;
> }
>
> - return EFI_SUCCESS;
> + //
> + // Find Logical Volume location
> + //
> + Status = FindLogicalVolumeLocation (
> + BlockIo,
> + DiskIo,
> + &AnchorPoint,
> + (UINT64 *)StartingLBA,
> + (UINT64 *)EndingLBA
> + );
> +
> + return Status;
> }
>
> /**
> @@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
> EFI_GUID *VendorDefinedGuid;
> EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
> EFI_PARTITION_INFO_PROTOCOL PartitionInfo;
> + EFI_LBA StartingLBA;
> + EFI_LBA EndingLBA;
>
> Media = BlockIo->Media;
>
> @@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
> }
>
> //
> - // Check if block device supports an UDF file system
> + // Find UDF logical volume on block device
> //
> - Status = SupportUdfFileSystem (BlockIo, DiskIo);
> + Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA,
> + &EndingLBA);
> if (EFI_ERROR (Status)) {
> return EFI_NOT_FOUND;
> }
> @@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
> DevicePath,
> (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
> &PartitionInfo,
> - 0,
> - Media->LastBlock,
> + StartingLBA,
> + EndingLBA,
> Media->BlockSize
> );
> if (!EFI_ERROR (Status)) {
> --
> 2.11.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
2017-09-17 13:13 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes Paulo Alcantara
@ 2017-09-18 1:00 ` Ni, Ruiyu
2017-09-18 16:51 ` Paulo Alcantara
0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-18 1:00 UTC (permalink / raw)
To: Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star, Laszlo Ersek
Paulo,
With the change in partition driver, I suppose UdfDxe driver
only needs to take care of area covered by the partition descriptor.
But why StartMainVolumeDescriptorSequence() still reads
LVD, TD, and PD?
I thought the UdfDxe driver's logic can be simplified a lot.
There should be no duplicated logic in Partition driver and udf driver.
Can you explain more?
Thanks/Ray
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
> Ersek <lersek@redhat.com>
> Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support
> PartitionDxe changes
>
> This patch reworks the driver to support Partition driver changes.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Paulo Alcantara <pcacjr@zytor.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 13 +-
> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
> ++++++++------------
> MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 88 +---
> 4 files changed, 204 insertions(+), 419 deletions(-)
>
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> index 01361141bb..f2c62967e8 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> @@ -100,6 +100,7 @@ UdfOpenVolume (
> &PrivFsData->Volume,
> &PrivFsData->Root
> );
> + ASSERT_EFI_ERROR (Status);
> if (EFI_ERROR (Status)) {
> goto Error_Find_Root_Dir;
> }
> @@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
> CleanupFileInformation (&PrivFsData->Root);
>
> Error_Find_Root_Dir:
> - CleanupVolumeInformation (&PrivFsData->Volume);
>
> Error_Read_Udf_Volume:
> Error_Invalid_Params:
> @@ -528,7 +528,6 @@ UdfClose (
> EFI_TPL OldTpl;
> EFI_STATUS Status;
> PRIVATE_UDF_FILE_DATA *PrivFileData;
> - PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
>
> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>
> @@ -541,8 +540,6 @@ UdfClose (
>
> PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
>
> - PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData-
> >SimpleFs);
> -
> if (!PrivFileData->IsRootDirectory) {
> CleanupFileInformation (&PrivFileData->File);
>
> @@ -551,10 +548,6 @@ UdfClose (
> }
> }
>
> - if (--PrivFsData->OpenFiles == 0) {
> - CleanupVolumeInformation (&PrivFsData->Volume);
> - }
> -
> FreePool ((VOID *)PrivFileData);
>
> Exit:
> @@ -787,7 +780,7 @@ UdfGetInfo (
> } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
> String = VolumeLabel;
>
> - FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
> + FileSetDesc = &PrivFsData->Volume.FileSetDesc;
>
> OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
>
> @@ -846,7 +839,7 @@ UdfGetInfo (
> FileSystemInfo->Size = FileSystemInfoLength;
> FileSystemInfo->ReadOnly = TRUE;
> FileSystemInfo->BlockSize =
> - LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
> + PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
> FileSystemInfo->VolumeSize = VolumeSize;
> FileSystemInfo->FreeSpace = FreeSpaceSize;
>
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> index 4609580b30..63b643e60a 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> @@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
>
> EFI_STATUS
> StartMainVolumeDescriptorSequence (
> - IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> - IN EFI_DISK_IO_PROTOCOL *DiskIo,
> - IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
> - OUT UDF_VOLUME_INFO *Volume
> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> + OUT UDF_VOLUME_INFO *Volume
> )
> {
> - EFI_STATUS Status;
> - UINT32 BlockSize;
> - UDF_EXTENT_AD *ExtentAd;
> - UINT64 StartingLsn;
> - UINT64 EndingLsn;
> - VOID *Buffer;
> - UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
> - UINTN Index;
> - UINT32 LogicalBlockSize;
> + EFI_STATUS Status;
> + UINT32 BlockSize;
> + UINT64 BlockOffset;
> + VOID *Buffer;
> + UINT32 LogicalBlockSize;
> +
> + BlockSize = BlockIo->Media->BlockSize;
>
> //
> - // We've already found an ADVP on the volume. It contains the extent
> - // (MainVolumeDescriptorSequenceExtent) where the Main Volume
> Descriptor
> - // Sequence starts. Therefore, we'll look for Logical Volume Descriptors and
> - // Partitions Descriptors and save them in memory, accordingly.
> - //
> - // Note also that each descriptor will be aligned on a block size (BlockSize)
> - // boundary, so we need to read one block at a time.
> + // Allocate buffer for reading disk blocks
> //
> - BlockSize = BlockIo->Media->BlockSize;
> - ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
> - StartingLsn = (UINT64)ExtentAd->ExtentLocation;
> - EndingLsn = StartingLsn + DivU64x32 (
> - (UINT64)ExtentAd->ExtentLength,
> - BlockSize
> - );
> -
> - Volume->LogicalVolDescs =
> - (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
> >ExtentLength);
> - if (Volume->LogicalVolDescs == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> - }
> -
> - Volume->PartitionDescs =
> - (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
> >ExtentLength);
> - if (Volume->PartitionDescs == NULL) {
> - Status = EFI_OUT_OF_RESOURCES;
> - goto Error_Alloc_Pds;
> - }
> -
> - Buffer = AllocateZeroPool (BlockSize);
> + Buffer = AllocateZeroPool ((UINTN)BlockSize);
> if (Buffer == NULL) {
> - Status = EFI_OUT_OF_RESOURCES;
> - goto Error_Alloc_Buf;
> + return EFI_OUT_OF_RESOURCES;
> }
>
> - Volume->LogicalVolDescsNo = 0;
> - Volume->PartitionDescsNo = 0;
> -
> - while (StartingLsn <= EndingLsn) {
> + //
> + // As per UDF 2.60 specification:
> + //
> + // ---
> + // For the purpose of interchange, Partition Maps shall be limited to
> + // Partition Map type 1, except type 2 maps.
> + // ---
> + // A Partition Descriptor Access Type of read-only, rewritable, //
> + overwritable, write-once and pseudo-overwritable shall be //
> + supported. There shall be exactly one prevailing Partition //
> + Descriptor recorded per volume, with one exception. For Volume //
> + Sets that consist of single volume, the volume may contain 2 non- //
> + overlapping Partitions with 2 prevailing Partition Descriptors only
> + // if one has an Access Type of read-only and the other has an //
> + Access Type of rewritable, overwritable, or write-once. The //
> + Logical Volume for this volume would consist of the contents of //
> + both partitions.
> + // ---
> + //
> + // The logical partition created by Partition driver is relative to
> + the main // VDS extent location, so we start the Main Volume
> + Descriptor Sequence at // LBA 0.
> + //
> + // We don't need to make sure we got both LVD and PD -- that is, if
> + we are // here, that means Partition driver was able to find them both
> previously.
> + //
> + for (BlockOffset = 0;
> + BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
> + BlockIo->Media->BlockSize);
> + BlockOffset += BlockSize) {
> + // Read disk block
> + //
> Status = DiskIo->ReadDisk (
> DiskIo,
> BlockIo->Media->MediaId,
> - MultU64x32 (StartingLsn, BlockSize),
> + BlockOffset,
> BlockSize,
> Buffer
> );
> if (EFI_ERROR (Status)) {
> - goto Error_Read_Disk_Blk;
> + goto Out_Free;
> }
>
> + //
> + // Check if read block is a Terminating Descriptor
> + //
> if (IS_TD (Buffer)) {
> //
> - // Found a Terminating Descriptor. Stop the sequence then.
> + // Terminate Main Volume Descriptor Sequence
> //
> break;
> }
>
> if (IS_LVD (Buffer)) {
> //
> - // Found a Logical Volume Descriptor.
> + // Save Logical Volume Descriptor
> //
> - LogicalVolDesc =
> - (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
> - AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> - if (LogicalVolDesc == NULL) {
> - Status = EFI_OUT_OF_RESOURCES;
> - goto Error_Alloc_Lvd;
> - }
> -
> - CopyMem ((VOID *)LogicalVolDesc, Buffer,
> - sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> - Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
> LogicalVolDesc;
> + CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof
> + (Volume->LogicalVolDesc));
> } else if (IS_PD (Buffer)) {
> //
> - // Found a Partition Descriptor.
> + // Save Partition Descriptor
> //
> - PartitionDesc =
> - (UDF_PARTITION_DESCRIPTOR *)
> - AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
> - if (PartitionDesc == NULL) {
> - Status = EFI_OUT_OF_RESOURCES;
> - goto Error_Alloc_Pd;
> - }
> -
> - CopyMem ((VOID *)PartitionDesc, Buffer,
> - sizeof (UDF_PARTITION_DESCRIPTOR));
> - Volume->PartitionDescs[Volume->PartitionDescsNo++] = PartitionDesc;
> + CopyMem (&Volume->PartitionDesc, Buffer, sizeof
> + (Volume->PartitionDesc));
> }
> -
> - StartingLsn++;
> }
>
> //
> - // When an UDF volume (revision 2.00 or higher) contains a File Entry rather
> - // than an Extended File Entry (which is not recommended as per spec), we
> need
> - // to make sure the size of a FE will be _at least_ 2048
> - // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward compatibility.
> + // Determine FE (File Entry) size
> //
> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
> - Volume->FileEntrySize = LogicalBlockSize;
> + Volume->FileEntrySize = (UINTN)LogicalBlockSize;
> } else {
> Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
> }
>
> - FreePool (Buffer);
> -
> - return EFI_SUCCESS;
> -
> -Error_Alloc_Pd:
> -Error_Alloc_Lvd:
> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> - FreePool ((VOID *)Volume->PartitionDescs[Index]);
> - }
> -
> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> - FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> - }
> + Status = EFI_SUCCESS;
>
> -Error_Read_Disk_Blk:
> +Out_Free:
> + //
> + // Free block read buffer
> + //
> FreePool (Buffer);
>
> -Error_Alloc_Buf:
> - FreePool ((VOID *)Volume->PartitionDescs);
> - Volume->PartitionDescs = NULL;
> -
> -Error_Alloc_Pds:
> - FreePool ((VOID *)Volume->LogicalVolDescs);
> - Volume->LogicalVolDescs = NULL;
> -
> return Status;
> }
>
> @@ -223,11 +180,9 @@ GetPdFromLongAd (
> )
> {
> UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> - UINTN Index;
> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
> UINT16 PartitionNum;
>
> - LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
> + LogicalVolDesc = &Volume->LogicalVolDesc;
>
> switch (LV_UDF_REVISION (LogicalVolDesc)) {
> case 0x0102:
> @@ -252,19 +207,21 @@ GetPdFromLongAd (
> PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
> break;
> case 0x0260:
> - //
> - // Fall through.
> - //
> - default:
> PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
> break;
> + default:
> + //
> + // Unhandled UDF revision
> + //
> + return NULL;
> }
>
> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> - PartitionDesc = Volume->PartitionDescs[Index];
> - if (PartitionDesc->PartitionNumber == PartitionNum) {
> - return PartitionDesc;
> - }
> + //
> + // Check if partition number matches Partition Descriptor found in
> + Main Volume // Descriptor Sequence.
> + //
> + if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
> + return &Volume->PartitionDesc;
> }
>
> return NULL;
> @@ -284,8 +241,9 @@ GetLongAdLsn (
> PartitionDesc = GetPdFromLongAd (Volume, LongAd);
> ASSERT (PartitionDesc != NULL);
>
> - return (UINT64)PartitionDesc->PartitionStartingLocation +
> - LongAd->ExtentLocation.LogicalBlockNumber;
> + return (UINT64)PartitionDesc->PartitionStartingLocation -
> + Volume->MainVdsStartLocation +
> + LongAd->ExtentLocation.LogicalBlockNumber;
> }
>
> //
> @@ -293,12 +251,13 @@ GetLongAdLsn (
> //
> UINT64
> GetShortAdLsn (
> + IN UDF_VOLUME_INFO *Volume,
> IN UDF_PARTITION_DESCRIPTOR *PartitionDesc,
> IN UDF_SHORT_ALLOCATION_DESCRIPTOR *ShortAd
> )
> {
> - return (UINT64)PartitionDesc->PartitionStartingLocation +
> - ShortAd->ExtentPosition;
> + return (UINT64)PartitionDesc->PartitionStartingLocation -
> + Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
> }
>
> //
> @@ -311,36 +270,39 @@ EFI_STATUS
> FindFileSetDescriptor (
> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> IN EFI_DISK_IO_PROTOCOL *DiskIo,
> - IN UDF_VOLUME_INFO *Volume,
> - IN UINTN LogicalVolDescNum,
> - OUT UDF_FILE_SET_DESCRIPTOR *FileSetDesc
> + IN UDF_VOLUME_INFO *Volume
> )
> {
> EFI_STATUS Status;
> UINT64 Lsn;
> UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>
> - LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
> + LogicalVolDesc = &Volume->LogicalVolDesc;
> Lsn = GetLongAdLsn (Volume, &LogicalVolDesc-
> >LogicalVolumeContentsUse);
>
> //
> - // Read extent (Long Ad).
> + // As per UDF 2.60 specification:
> + //
> + // There shall be exactly one File Set Descriptor recorded per
> + Logical // Volume.
> + //
> + // Read disk block
> //
> Status = DiskIo->ReadDisk (
> DiskIo,
> BlockIo->Media->MediaId,
> MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
> - sizeof (UDF_FILE_SET_DESCRIPTOR),
> - (VOID *)FileSetDesc
> + sizeof (Volume->FileSetDesc),
> + &Volume->FileSetDesc
> );
> if (EFI_ERROR (Status)) {
> return Status;
> }
>
> //
> - // Check if the read extent contains a valid FSD's tag identifier.
> + // Check if read block is a File Set Descriptor
> //
> - if (!IS_FSD (FileSetDesc)) {
> + if (!IS_FSD (&Volume->FileSetDesc)) {
> return EFI_VOLUME_CORRUPTED;
> }
>
> @@ -348,71 +310,6 @@ FindFileSetDescriptor ( }
>
> //
> -// Get all File Set Descriptors for each Logical Volume Descriptor.
> -//
> -EFI_STATUS
> -GetFileSetDescriptors (
> - IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> - IN EFI_DISK_IO_PROTOCOL *DiskIo,
> - IN OUT UDF_VOLUME_INFO *Volume
> - )
> -{
> - EFI_STATUS Status;
> - UINTN Index;
> - UDF_FILE_SET_DESCRIPTOR *FileSetDesc;
> - UINTN Count;
> -
> - Volume->FileSetDescs =
> - (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
> - Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
> - if (Volume->FileSetDescs == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> - }
> -
> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> - FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
> - if (FileSetDesc == NULL) {
> - Status = EFI_OUT_OF_RESOURCES;
> - goto Error_Alloc_Fsd;
> - }
> -
> - //
> - // Find a FSD for this LVD.
> - //
> - Status = FindFileSetDescriptor (
> - BlockIo,
> - DiskIo,
> - Volume,
> - Index,
> - FileSetDesc
> - );
> - if (EFI_ERROR (Status)) {
> - goto Error_Find_Fsd;
> - }
> -
> - //
> - // Got one. Save it.
> - //
> - Volume->FileSetDescs[Index] = FileSetDesc;
> - }
> -
> - Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
> - return EFI_SUCCESS;
> -
> -Error_Find_Fsd:
> - Count = Index + 1;
> - for (Index = 0; Index < Count; Index++) {
> - FreePool ((VOID *)Volume->FileSetDescs[Index]);
> - }
> -
> - FreePool ((VOID *)Volume->FileSetDescs);
> - Volume->FileSetDescs = NULL;
> -
> -Error_Alloc_Fsd:
> - return Status;
> -}
> -
> -//
> // Read Volume and File Structure on an UDF file system.
> //
> EFI_STATUS
> @@ -424,9 +321,10 @@ ReadVolumeFileStructure ( {
> EFI_STATUS Status;
> UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
> + UDF_EXTENT_AD *ExtentAd;
>
> //
> - // Find an AVDP.
> + // Find Anchor Volume Descriptor Pointer
> //
> Status = FindAnchorVolumeDescriptorPointer (
> BlockIo,
> @@ -438,12 +336,18 @@ ReadVolumeFileStructure (
> }
>
> //
> - // AVDP has been found. Start MVDS.
> + // Save Main VDS start block number
> + //
> + ExtentAd = &AnchorPoint.MainVolumeDescriptorSequenceExtent;
> +
> + Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
> +
> + //
> + // Start Main Volume Descriptor Sequence.
> //
> Status = StartMainVolumeDescriptorSequence (
> BlockIo,
> DiskIo,
> - &AnchorPoint,
> Volume
> );
> if (EFI_ERROR (Status)) {
> @@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
> return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR
> *)Ad);
> } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
> return GetShortAdLsn (
> + Volume,
> GetPdFromLongAd (Volume, ParentIcb),
> (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
> );
> @@ -740,7 +645,7 @@ GetAedAdsOffset (
> return EFI_OUT_OF_RESOURCES;
> }
>
> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>
> //
> // Read extent.
> @@ -890,7 +795,7 @@ ReadFile (
> UINT32 ExtentLength;
> UDF_FE_RECORDING_FLAGS RecordingFlags;
>
> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> DoFreeAed = FALSE;
>
> //
> @@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation ( {
> EFI_STATUS Status;
>
> + //
> + // Read all necessary UDF volume information and keep it private to
> + the driver //
> Status = ReadVolumeFileStructure (
> BlockIo,
> DiskIo,
> @@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
> return Status;
> }
>
> - Status = GetFileSetDescriptors (
> - BlockIo,
> - DiskIo,
> - Volume
> - );
> + //
> + // Find File Set Descriptor
> + //
> + Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
> if (EFI_ERROR (Status)) {
> - CleanupVolumeInformation (Volume);
> + return Status;
> }
>
> return Status;
> @@ -1407,7 +1314,7 @@ FindRootDirectory (
> BlockIo,
> DiskIo,
> Volume,
> - &Volume->FileSetDescs[0]->RootDirectoryIcb,
> + &Volume->FileSetDesc.RootDirectoryIcb,
> &File->FileEntry
> );
> if (EFI_ERROR (Status)) {
> @@ -1424,7 +1331,7 @@ FindRootDirectory (
> L"\\",
> NULL,
> &Parent,
> - &Volume->FileSetDescs[0]->RootDirectoryIcb,
> + &Volume->FileSetDesc.RootDirectoryIcb,
> File
> );
> if (EFI_ERROR (Status)) {
> @@ -1465,7 +1372,7 @@ FindFileEntry (
> UINT32 LogicalBlockSize;
>
> Lsn = GetLongAdLsn (Volume, Icb);
> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>
> *FileEntry = AllocateZeroPool (Volume->FileEntrySize);
> if (*FileEntry == NULL) {
> @@ -1959,43 +1866,6 @@ Error_Find_File:
> }
>
> /**
> - Clean up in-memory UDF volume information.
> -
> - @param[in] Volume Volume information pointer.
> -
> -**/
> -VOID
> -CleanupVolumeInformation (
> - IN UDF_VOLUME_INFO *Volume
> - )
> -{
> - UINTN Index;
> -
> - if (Volume->LogicalVolDescs != NULL) {
> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> - FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> - }
> - FreePool ((VOID *)Volume->LogicalVolDescs);
> - }
> -
> - if (Volume->PartitionDescs != NULL) {
> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> - FreePool ((VOID *)Volume->PartitionDescs[Index]);
> - }
> - FreePool ((VOID *)Volume->PartitionDescs);
> - }
> -
> - if (Volume->FileSetDescs != NULL) {
> - for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
> - FreePool ((VOID *)Volume->FileSetDescs[Index]);
> - }
> - FreePool ((VOID *)Volume->FileSetDescs);
> - }
> -
> - ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO)); -}
> -
> -/**
> Clean up in-memory UDF file information.
>
> @param[in] File File information pointer.
> @@ -2249,91 +2119,100 @@ GetVolumeSize (
> OUT UINT64 *FreeSpaceSize
> )
> {
> - UDF_EXTENT_AD ExtentAd;
> - UINT32 LogicalBlockSize;
> - UINT64 Lsn;
> - EFI_STATUS Status;
> - UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
> - UINTN Index;
> - UINTN Length;
> - UINT32 LsnsNo;
> -
> - *VolumeSize = 0;
> - *FreeSpaceSize = 0;
> -
> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> - CopyMem ((VOID *)&ExtentAd,
> - (VOID *)&Volume->LogicalVolDescs[Index]-
> >IntegritySequenceExtent,
> - sizeof (UDF_EXTENT_AD));
> - if (ExtentAd.ExtentLength == 0) {
> - continue;
> - }
> + EFI_STATUS Status;
> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> + UDF_EXTENT_AD *ExtentAd;
> + UINT64 Lsn;
> + UINT32 LogicalBlockSize;
> + UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
> + UINTN Index;
> + UINTN Length;
> + UINT32 LsnsNo;
>
> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
> + LogicalVolDesc = &Volume->LogicalVolDesc;
>
> - Read_Next_Sequence:
> - LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
> - AllocatePool (ExtentAd.ExtentLength);
> - if (LogicalVolInt == NULL) {
> - return EFI_OUT_OF_RESOURCES;
> - }
> + ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
>
> - Lsn = (UINT64)ExtentAd.ExtentLocation;
> + if (ExtentAd->ExtentLength == 0) {
> + return EFI_VOLUME_CORRUPTED;
> + }
>
> - Status = DiskIo->ReadDisk (
> - DiskIo,
> - BlockIo->Media->MediaId,
> - MultU64x32 (Lsn, LogicalBlockSize),
> - ExtentAd.ExtentLength,
> - (VOID *)LogicalVolInt
> - );
> - if (EFI_ERROR (Status)) {
> - FreePool ((VOID *)LogicalVolInt);
> - return Status;
> - }
> + LogicalVolInt = AllocatePool (ExtentAd->ExtentLength); if
> + (LogicalVolInt == NULL) {
> + return EFI_OUT_OF_RESOURCES;
> + }
>
> - if (!IS_LVID (LogicalVolInt)) {
> - FreePool ((VOID *)LogicalVolInt);
> - return EFI_VOLUME_CORRUPTED;
> - }
> + //
> + // Get location of Logical Volume Integrity Descriptor // Lsn =
> + (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
>
> - Length = LogicalVolInt->NumberOfPartitions;
> - for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> - LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> - if (LsnsNo == 0xFFFFFFFFUL) {
> - //
> - // Size not specified.
> - //
> - continue;
> - }
> + LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
>
> - *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> - }
> + //
> + // Read disk block
> + //
> + Status = DiskIo->ReadDisk (
> + DiskIo,
> + BlockIo->Media->MediaId,
> + MultU64x32 (Lsn, LogicalBlockSize),
> + ExtentAd->ExtentLength,
> + LogicalVolInt
> + );
> + if (EFI_ERROR (Status)) {
> + goto Out_Free;
> + }
>
> - Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
> - for (; Index < Length; Index += sizeof (UINT32)) {
> - LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> - if (LsnsNo == 0xFFFFFFFFUL) {
> - //
> - // Size not specified.
> - //
> - continue;
> - }
> + //
> + // Check if read block is a Logical Volume Integrity Descriptor //
> + if (!IS_LVID (LogicalVolInt)) {
> + Status = EFI_VOLUME_CORRUPTED;
> + goto Out_Free;
> + }
>
> - *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> - }
> + *VolumeSize = 0;
> + *FreeSpaceSize = 0;
>
> - CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt-
> >NextIntegrityExtent,
> - sizeof (UDF_EXTENT_AD));
> - if (ExtentAd.ExtentLength > 0) {
> - FreePool ((VOID *)LogicalVolInt);
> - goto Read_Next_Sequence;
> + Length = LogicalVolInt->NumberOfPartitions;
> + for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> + LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> + //
> + // Check if size is not specified
> + //
> + if (LsnsNo == 0xFFFFFFFFUL) {
> + continue;
> }
> + //
> + // Accumulate free space size
> + //
> + *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize); }
>
> - FreePool ((VOID *)LogicalVolInt);
> + Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
> + for (; Index < Length; Index += sizeof (UINT32)) {
> + LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> + //
> + // Check if size is not specified
> + //
> + if (LsnsNo == 0xFFFFFFFFUL) {
> + continue;
> + }
> + //
> + // Accumulate used volume space
> + //
> + *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> }
>
> - return EFI_SUCCESS;
> + Status = EFI_SUCCESS;
> +
> +Out_Free:
> + //
> + // Free Logical Volume Integrity Descriptor
> + //
> + FreePool (LogicalVolInt);
> +
> + return Status;
> }
>
> /**
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> index 49dc7077b7..d4163b89ca 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> @@ -276,13 +276,6 @@ UdfDriverBindingStop (
> NULL
> );
>
> - //
> - // Check if there's any open file. If so, clean them up.
> - //
> - if (PrivFsData->OpenFiles > 0) {
> - CleanupVolumeInformation (&PrivFsData->Volume);
> - }
> -
> FreePool ((VOID *)PrivFsData);
> }
>
> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> index 240d420ff5..c5f83914d8 100644
> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> @@ -49,16 +49,8 @@
> { 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A } \
> }
>
> -#define UDF_DEFAULT_LV_NUM 0
> -
> #define IS_PVD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1)) -#define IS_PD(_Pointer) \
> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) -#define IS_LVD(_Pointer) \
> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) -#define IS_TD(_Pointer) \
> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8)) #define IS_FSD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256)) #define IS_FE(_Pointer) \
> @@ -152,14 +144,8 @@ typedef enum { #define
> IS_VALID_COMPRESSION_ID(_CompId) \
> ((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
>
> -#define LV_BLOCK_SIZE(_Vol, _LvNum) \
> - (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
> -
> #define UDF_STANDARD_IDENTIFIER_LENGTH 5
>
> -#define LV_UDF_REVISION(_Lv) \
> - *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> -
> #pragma pack(1)
>
> typedef struct {
> @@ -186,17 +172,6 @@ typedef struct {
> #pragma pack(1)
>
> typedef struct {
> - UINT8 CharacterSetType;
> - UINT8 CharacterSetInfo[63];
> -} UDF_CHAR_SPEC;
> -
> -typedef struct {
> - UINT8 Flags;
> - UINT8 Identifier[23];
> - UINT8 IdentifierSuffix[8];
> -} UDF_ENTITY_ID;
> -
> -typedef struct {
> UINT16 TypeAndTimezone;
> INT16 Year;
> UINT8 Month;
> @@ -210,17 +185,6 @@ typedef struct {
> } UDF_TIMESTAMP;
>
> typedef struct {
> - UINT32 LogicalBlockNumber;
> - UINT16 PartitionReferenceNumber;
> -} UDF_LB_ADDR;
> -
> -typedef struct {
> - UINT32 ExtentLength;
> - UDF_LB_ADDR ExtentLocation;
> - UINT8 ImplementationUse[6];
> -} UDF_LONG_ALLOCATION_DESCRIPTOR;
> -
> -typedef struct {
> UDF_DESCRIPTOR_TAG DescriptorTag;
> UINT32 PrevAllocationExtentDescriptor;
> UINT32 LengthOfAllocationDescriptors;
> @@ -235,37 +199,6 @@ typedef struct {
> } UDF_VOLUME_DESCRIPTOR;
>
> typedef struct {
> - UDF_DESCRIPTOR_TAG DescriptorTag;
> - UINT32 VolumeDescriptorSequenceNumber;
> - UINT16 PartitionFlags;
> - UINT16 PartitionNumber;
> - UDF_ENTITY_ID PartitionContents;
> - UINT8 PartitionContentsUse[128];
> - UINT32 AccessType;
> - UINT32 PartitionStartingLocation;
> - UINT32 PartitionLength;
> - UDF_ENTITY_ID ImplementationIdentifier;
> - UINT8 ImplementationUse[128];
> - UINT8 Reserved[156];
> -} UDF_PARTITION_DESCRIPTOR;
> -
> -typedef struct {
> - UDF_DESCRIPTOR_TAG DescriptorTag;
> - UINT32 VolumeDescriptorSequenceNumber;
> - UDF_CHAR_SPEC DescriptorCharacterSet;
> - UINT8 LogicalVolumeIdentifier[128];
> - UINT32 LogicalBlockSize;
> - UDF_ENTITY_ID DomainIdentifier;
> - UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
> - UINT32 MapTableLength;
> - UINT32 NumberOfPartitionMaps;
> - UDF_ENTITY_ID ImplementationIdentifier;
> - UINT8 ImplementationUse[128];
> - UDF_EXTENT_AD IntegritySequenceExtent;
> - UINT8 PartitionMaps[6];
> -} UDF_LOGICAL_VOLUME_DESCRIPTOR;
> -
> -typedef struct {
> UDF_DESCRIPTOR_TAG DescriptorTag;
> UDF_TIMESTAMP RecordingDateTime;
> UINT32 IntegrityType;
> @@ -389,12 +322,10 @@ typedef struct {
> // UDF filesystem driver's private data // typedef struct {
> - UDF_LOGICAL_VOLUME_DESCRIPTOR **LogicalVolDescs;
> - UINTN LogicalVolDescsNo;
> - UDF_PARTITION_DESCRIPTOR **PartitionDescs;
> - UINTN PartitionDescsNo;
> - UDF_FILE_SET_DESCRIPTOR **FileSetDescs;
> - UINTN FileSetDescsNo;
> + UINT64 MainVdsStartLocation;
> + UDF_LOGICAL_VOLUME_DESCRIPTOR LogicalVolDesc;
> + UDF_PARTITION_DESCRIPTOR PartitionDesc;
> + UDF_FILE_SET_DESCRIPTOR FileSetDesc;
> UINTN FileEntrySize;
> } UDF_VOLUME_INFO;
>
> @@ -883,17 +814,6 @@ ResolveSymlink (
> );
>
> /**
> - Clean up in-memory UDF volume information.
> -
> - @param[in] Volume Volume information pointer.
> -
> -**/
> -VOID
> -CleanupVolumeInformation (
> - IN UDF_VOLUME_INFO *Volume
> - );
> -
> -/**
> Clean up in-memory UDF file information.
>
> @param[in] File File information pointer.
> --
> 2.11.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] UDF partition driver fix
2017-09-17 13:13 [PATCH v2 0/3] UDF partition driver fix Paulo Alcantara
` (2 preceding siblings ...)
2017-09-17 13:13 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes Paulo Alcantara
@ 2017-09-18 4:52 ` Ni, Ruiyu
2017-09-18 17:49 ` Paulo Alcantara
3 siblings, 1 reply; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-18 4:52 UTC (permalink / raw)
To: Paulo Alcantara, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Gao, Liming, Laszlo Ersek, Zeng, Star,
Yao, Jiewen
Paulo,
Could you please paste a "map -r" output on a CDROM which
contains Eltorito volume?
I want to confirm that the result is expected.
Thanks/Ray
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Sunday, September 17, 2017 9:13 PM
> To: edk2-devel@lists.01.org
> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: [PATCH v2 0/3] UDF partition driver fix
>
> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=707
>
> Hi,
>
> This patchset fixes a bug in Partition driver that UDF partitions occupied the
> entire disk space instead of using LVD space only.
>
> BTW, I've only tested it under OVMF and built it with GCC only. That would
> be appreciable if someone could build with other toolchains and see if this
> doesn't break.
>
> I used a Windows 10 ISO image with UdfDxe disabled and enabled. The `map
> -r` output seemed OK. No breakage when booting an OS off an ElTorito
> partition from an UDF bridge disk.
>
> v1->v2:
> - Followed Laszlo's suggestions to submit a proper patchset. Thanks!
> - As I'm still waiting for Ruiyu and Star to test this fix, I took
> advantage of it and did some code cleanups :-)
>
> Repo: https://github.com/pcacjr/edk2.git
> Branch: udf-partition-fix-v2
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> ---
>
> Paulo Alcantara (3):
> MdePkg: Add UDF volume structure definitions
> MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
> MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
>
> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
> +++++++++++-
> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 13 +-
> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
> ++++++++------------
> MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 88 +---
> MdePkg/Include/IndustryStandard/Udf.h | 63 +++
> 6 files changed, 566 insertions(+), 443 deletions(-)
>
> --
> 2.11.0
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
2017-09-18 0:28 ` Ni, Ruiyu
2017-09-18 0:42 ` Ni, Ruiyu
@ 2017-09-18 13:50 ` Paulo Alcantara
1 sibling, 0 replies; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-18 13:50 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Gao, Liming, Laszlo Ersek
Hi Ruiyu,
On 9/17/2017 9:28 PM, Ni, Ruiyu wrote:
> #define _GET_TAG_ID(_Pointer) \
> (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>
> #define IS_PD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
> #define IS_LVD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
> #define IS_TD(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>
> #define IS_AVDP(_Pointer) \
> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>
> Paulo,
> If you take a look at Pci22.h in the same directory, all macros are started
> as "IS_PCI_".
> How about changing the above IS_xxx to IS_UDF_xxx?
Looks good to me. I'll change them.
> Shall we change AVDP to AVD, following the same naming style as PD, LVD and TD?
There is no "Anchor Volume Descriptor" but "Anchor Volume Descriptor
Pointer", so we still need to keep 'P'.
> How about changing _Pointer to _Tag or DescriptorTag?
I will change it to _Tag. _Pointer is really a bad name. Thanks! :-)
Paulo
>
>
> Thanks/Ray
>
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
>> Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
>>
>> This patch adds a fewe more UDF structures in order to detect Logical
>> Volume and Partition descriptors during Main Volume Descriptor Sequence
>> in Partition driver.
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>> MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
>> 1 file changed, 63 insertions(+)
>>
>> diff --git a/MdePkg/Include/IndustryStandard/Udf.h
>> b/MdePkg/Include/IndustryStandard/Udf.h
>> index 0febb4bcda..6cc42f8543 100644
>> --- a/MdePkg/Include/IndustryStandard/Udf.h
>> +++ b/MdePkg/Include/IndustryStandard/Udf.h
>> @@ -27,9 +27,19 @@
>> #define _GET_TAG_ID(_Pointer) \
>> (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>>
>> +#define IS_PD(_Pointer) \
>> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
>> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
>> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>> +
>> #define IS_AVDP(_Pointer) \
>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>>
>> +#define LV_UDF_REVISION(_Lv) \
>> + *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
>> +
>> #pragma pack(1)
>>
>> typedef struct {
>> @@ -55,6 +65,59 @@ typedef struct {
>> UINT8 Reserved[480];
>> } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
>>
>> +typedef struct {
>> + UINT8 CharacterSetType;
>> + UINT8 CharacterSetInfo[63];
>> +} UDF_CHAR_SPEC;
>> +
>> +typedef struct {
>> + UINT8 Flags;
>> + UINT8 Identifier[23];
>> + UINT8 IdentifierSuffix[8];
>> +} UDF_ENTITY_ID;
>> +
>> +typedef struct {
>> + UINT32 LogicalBlockNumber;
>> + UINT16 PartitionReferenceNumber;
>> +} UDF_LB_ADDR;
>> +
>> +typedef struct {
>> + UINT32 ExtentLength;
>> + UDF_LB_ADDR ExtentLocation;
>> + UINT8 ImplementationUse[6];
>> +} UDF_LONG_ALLOCATION_DESCRIPTOR;
>> +
>> +typedef struct {
>> + UDF_DESCRIPTOR_TAG DescriptorTag;
>> + UINT32 VolumeDescriptorSequenceNumber;
>> + UDF_CHAR_SPEC DescriptorCharacterSet;
>> + UINT8 LogicalVolumeIdentifier[128];
>> + UINT32 LogicalBlockSize;
>> + UDF_ENTITY_ID DomainIdentifier;
>> + UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
>> + UINT32 MapTableLength;
>> + UINT32 NumberOfPartitionMaps;
>> + UDF_ENTITY_ID ImplementationIdentifier;
>> + UINT8 ImplementationUse[128];
>> + UDF_EXTENT_AD IntegritySequenceExtent;
>> + UINT8 PartitionMaps[6];
>> +} UDF_LOGICAL_VOLUME_DESCRIPTOR;
>> +
>> +typedef struct {
>> + UDF_DESCRIPTOR_TAG DescriptorTag;
>> + UINT32 VolumeDescriptorSequenceNumber;
>> + UINT16 PartitionFlags;
>> + UINT16 PartitionNumber;
>> + UDF_ENTITY_ID PartitionContents;
>> + UINT8 PartitionContentsUse[128];
>> + UINT32 AccessType;
>> + UINT32 PartitionStartingLocation;
>> + UINT32 PartitionLength;
>> + UDF_ENTITY_ID ImplementationIdentifier;
>> + UINT8 ImplementationUse[128];
>> + UINT8 Reserved[156];
>> +} UDF_PARTITION_DESCRIPTOR;
>> +
>> #pragma pack()
>>
>> #endif
>> --
>> 2.11.0
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
2017-09-18 0:42 ` Ni, Ruiyu
@ 2017-09-18 13:52 ` Paulo Alcantara
0 siblings, 0 replies; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-18 13:52 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Laszlo Ersek, Gao, Liming
Hi,
On 9/17/2017 9:42 PM, Ni, Ruiyu wrote:
> Continue reading the patch #2, I think we can change IS_PD to:
> #define IS_UDF_PD(Tag) ((Tag)->TagIdentifier == 5)
>
> Using the above way, we can avoid caller to supply an invalid buffer.
Good point. I'll fix it.
Thanks!
Paulo
>
> Thanks/Ray
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Ni,
>> Ruiyu
>> Sent: Monday, September 18, 2017 8:29 AM
>> To: Paulo Alcantara <pcacjr@zytor.com>; edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Laszlo Ersek
>> <lersek@redhat.com>; Gao, Liming <liming.gao@intel.com>
>> Subject: Re: [edk2] [PATCH v2 1/3] MdePkg: Add UDF volume structure
>> definitions
>>
>> #define _GET_TAG_ID(_Pointer) \
>> (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>>
>> #define IS_PD(_Pointer) \
>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5))
>> #define IS_LVD(_Pointer) \
>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6))
>> #define IS_TD(_Pointer) \
>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>>
>> #define IS_AVDP(_Pointer) \
>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>>
>> Paulo,
>> If you take a look at Pci22.h in the same directory, all macros are started
>> as "IS_PCI_".
>> How about changing the above IS_xxx to IS_UDF_xxx?
>> Shall we change AVDP to AVD, following the same naming style as PD, LVD
>> and TD?
>> How about changing _Pointer to _Tag or DescriptorTag?
>>
>>
>> Thanks/Ray
>>
>>> -----Original Message-----
>>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>>> Sent: Sunday, September 17, 2017 9:13 PM
>>> To: edk2-devel@lists.01.org
>>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
>>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>;
>> Laszlo
>>> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>
>>> Subject: [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions
>>>
>>> This patch adds a fewe more UDF structures in order to detect Logical
>>> Volume and Partition descriptors during Main Volume Descriptor Sequence
>>> in Partition driver.
>>>
>>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>>> Cc: Liming Gao <liming.gao@intel.com>
>>> Cc: Laszlo Ersek <lersek@redhat.com>
>>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>>> Contributed-under: TianoCore Contribution Agreement 1.1
>>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>>> ---
>>> MdePkg/Include/IndustryStandard/Udf.h | 63 ++++++++++++++++++++
>>> 1 file changed, 63 insertions(+)
>>>
>>> diff --git a/MdePkg/Include/IndustryStandard/Udf.h
>>> b/MdePkg/Include/IndustryStandard/Udf.h
>>> index 0febb4bcda..6cc42f8543 100644
>>> --- a/MdePkg/Include/IndustryStandard/Udf.h
>>> +++ b/MdePkg/Include/IndustryStandard/Udf.h
>>> @@ -27,9 +27,19 @@
>>> #define _GET_TAG_ID(_Pointer) \
>>> (((UDF_DESCRIPTOR_TAG *)(_Pointer))->TagIdentifier)
>>>
>>> +#define IS_PD(_Pointer) \
>>> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) #define IS_LVD(_Pointer) \
>>> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) #define IS_TD(_Pointer) \
>>> + ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8))
>>> +
>>> #define IS_AVDP(_Pointer) \
>>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 2))
>>>
>>> +#define LV_UDF_REVISION(_Lv) \
>>> + *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
>>> +
>>> #pragma pack(1)
>>>
>>> typedef struct {
>>> @@ -55,6 +65,59 @@ typedef struct {
>>> UINT8 Reserved[480];
>>> } UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER;
>>>
>>> +typedef struct {
>>> + UINT8 CharacterSetType;
>>> + UINT8 CharacterSetInfo[63];
>>> +} UDF_CHAR_SPEC;
>>> +
>>> +typedef struct {
>>> + UINT8 Flags;
>>> + UINT8 Identifier[23];
>>> + UINT8 IdentifierSuffix[8];
>>> +} UDF_ENTITY_ID;
>>> +
>>> +typedef struct {
>>> + UINT32 LogicalBlockNumber;
>>> + UINT16 PartitionReferenceNumber;
>>> +} UDF_LB_ADDR;
>>> +
>>> +typedef struct {
>>> + UINT32 ExtentLength;
>>> + UDF_LB_ADDR ExtentLocation;
>>> + UINT8 ImplementationUse[6];
>>> +} UDF_LONG_ALLOCATION_DESCRIPTOR;
>>> +
>>> +typedef struct {
>>> + UDF_DESCRIPTOR_TAG DescriptorTag;
>>> + UINT32 VolumeDescriptorSequenceNumber;
>>> + UDF_CHAR_SPEC DescriptorCharacterSet;
>>> + UINT8 LogicalVolumeIdentifier[128];
>>> + UINT32 LogicalBlockSize;
>>> + UDF_ENTITY_ID DomainIdentifier;
>>> + UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
>>> + UINT32 MapTableLength;
>>> + UINT32 NumberOfPartitionMaps;
>>> + UDF_ENTITY_ID ImplementationIdentifier;
>>> + UINT8 ImplementationUse[128];
>>> + UDF_EXTENT_AD IntegritySequenceExtent;
>>> + UINT8 PartitionMaps[6];
>>> +} UDF_LOGICAL_VOLUME_DESCRIPTOR;
>>> +
>>> +typedef struct {
>>> + UDF_DESCRIPTOR_TAG DescriptorTag;
>>> + UINT32 VolumeDescriptorSequenceNumber;
>>> + UINT16 PartitionFlags;
>>> + UINT16 PartitionNumber;
>>> + UDF_ENTITY_ID PartitionContents;
>>> + UINT8 PartitionContentsUse[128];
>>> + UINT32 AccessType;
>>> + UINT32 PartitionStartingLocation;
>>> + UINT32 PartitionLength;
>>> + UDF_ENTITY_ID ImplementationIdentifier;
>>> + UINT8 ImplementationUse[128];
>>> + UINT8 Reserved[156];
>>> +} UDF_PARTITION_DESCRIPTOR;
>>> +
>>> #pragma pack()
>>>
>>> #endif
>>> --
>>> 2.11.0
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-18 0:54 ` Ni, Ruiyu
@ 2017-09-18 16:38 ` Paulo Alcantara
2017-09-20 8:14 ` Ni, Ruiyu
0 siblings, 1 reply; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-18 16:38 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Dong, Eric, Zeng, Star, Laszlo Ersek
Hi,
On 9/17/2017 9:54 PM, Ni, Ruiyu wrote:
> Paulo,
> Several comments:
> 1. Can below logic be removed in PartitionDxe/Udf.c?
> while (!IsDevicePathEnd (DevicePathNode)) {
> //
> // Do not allow checking for UDF file systems in CDROM "El Torito"
> // partitions, and skip duplicate installation of UDF file system child
> // nodes.
> //
> if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
> if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
> return EFI_NOT_FOUND;
> }
> if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
> VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
> OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
> if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
> return EFI_NOT_FOUND;
> }
> }
> }
> //
> // Try next device path node
> //
> DevicePathNode = NextDevicePathNode (DevicePathNode);
> }
I think it's no longer necessary, so I'll remove it.
>
> 2. Can you add function header comments for the newly added functions?
Sure.
>
> 3. Change the consuming code of IS_UDF_XXX macro after patch #1 is changed.
ACK.
>
> 4. Perhaps to add more checks for these numbers read from the UDF CDROM.
> As Jiewen mentioned in another mail, secure code needs to validate all external inputs.
> We shouldn't assume the UDF CDROM is a well-formatted CDROM.
> A hacker may create a UDF CDROM to hack the system by using mechanism like
> buffer overflow, integer overflow, etc.
OK, when we start the Main Volume Descriptor Sequence (MVDS), we rely on
"ExtentAd->ExtentLocation" to know the LBA
where to start the sequence, and on "ExtentAd->ExtentLength" to know
many LBAs the sequence has. We only care about
LVD, PD and TD descriptors. We currently check whether a TD descriptor
exists, or the search exceed ExtentAd->ExtentLength blocks
to determine when stopping the sequence. That doesn't seem to be safe
enough, but here are some thoughts:
a. "ExtentAd->ExtentLength" may point to a bad value. If it's greater
than or equal to BlockIo->Media->LastBlock, and there is no
TD descriptor, we'll have to walk the entire disk and there will too
much I/O overhead and then impacting bootup. Obviously,
when it's greater than LastBlock, DiksIo->ReadDisk() should not allow to
read a non-existent LBA and then return
EFI_INVALID_PARAMETER, as per UEFI spec.
b. "ExtentAd->ExtentLocation" might point to a invalid LBA, although
DiskIo->ReadDisk() would not allow it.
So, it's time to look at UDF 2.60 spec again:
> Volume Descriptor Sequence Extent
>
> Both the main and reserve volume descriptor sequence extents
> shall each have a minimum length of 16 logical sectors. The VDS
> Extent may be terminated by the extent length.
It doesn't tell us either a limit of logical blocks, or a maximum number.
Next:
> 6.9.2.3 Step 3. Volume Descriptor Sequence
> Read logical sectors:
> MVDS_Location through MVDS_Location + (MVDS_Length - 1) / SectorSize
> ...
MVDS_Length == ExtentAd->ExtentLength, so still insufficient.
Next:
> The data located in bytes 0-3 and 5 of the Anchor Volume Descriptor
Pointer may be
> used for format verification if desired. Verifying the Tag Checksum
in byte 4 and
> Descriptor CRC in bytes 8-11 are good additional verifications to
perform.
> MVDS_Location and MVDS_Length are read from this structure.
Seems like a great check prior to reading ExtentAd->ExtentLocation and
ExtentAd->ExtentLength contents. Would that be sufficient to you?
I haven't found a way to limit the number of blocks a Volume Descriptor
Sequence might have.
Thanks,
Paulo
>
>
> Thanks/Ray
>
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
>> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>
>> Subject: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF
>> logical partition
>>
>> Do not use entire block device size for the UDF logical partition, instead
>> reserve the appropriate space (LVD space) for it.
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
>> ++++++++++++++++++--
>> 1 file changed, 299 insertions(+), 24 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> index 7856b5dfc1..3e84882922 100644
>> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
>> @@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
>> return EFI_VOLUME_CORRUPTED;
>> }
>>
>> -/**
>> - Check if block device supports a valid UDF file system as specified by OSTA
>> - Universal Disk Format Specification 2.60.
>> -
>> - @param[in] BlockIo BlockIo interface.
>> - @param[in] DiskIo DiskIo interface.
>> -
>> - @retval EFI_SUCCESS UDF file system found.
>> - @retval EFI_UNSUPPORTED UDF file system not found.
>> - @retval EFI_NO_MEDIA The device has no media.
>> - @retval EFI_DEVICE_ERROR The device reported an error.
>> - @retval EFI_VOLUME_CORRUPTED The file system structures are
>> corrupted.
>> - @retval EFI_OUT_OF_RESOURCES The scan was not successful due to lack
>> of
>> - resources.
>> -
>> -**/
>> EFI_STATUS
>> -SupportUdfFileSystem (
>> +FindUdfVolumeIdentifiers (
>> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>> IN EFI_DISK_IO_PROTOCOL *DiskIo
>> )
>> @@ -111,7 +95,6 @@ SupportUdfFileSystem (
>> UINT64 EndDiskOffset;
>> CDROM_VOLUME_DESCRIPTOR VolDescriptor;
>> CDROM_VOLUME_DESCRIPTOR TerminatingVolDescriptor;
>> - UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
>>
>> ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof
>> (CDROM_VOLUME_DESCRIPTOR));
>>
>> @@ -207,12 +190,302 @@ SupportUdfFileSystem (
>> return EFI_UNSUPPORTED;
>> }
>>
>> + return EFI_SUCCESS;
>> +}
>> +
>> +EFI_STATUS
>> +GetPartitionNumber (
>> + IN UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc,
>> + OUT UINT16 *PartitionNum
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UDF_LONG_ALLOCATION_DESCRIPTOR *LongAd;
>> +
>> + Status = EFI_SUCCESS;
>> +
>> + switch (LV_UDF_REVISION (LogicalVolDesc)) { case 0x0102:
>> + //
>> + // UDF 1.20 only supports Type 1 Partition
>> + //
>> + *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
>>> PartitionMaps[4]);
>> + break;
>> + case 0x0150:
>> + //
>> + // Ensure Type 1 Partition map
>> + //
>> + ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
>> + LogicalVolDesc->PartitionMaps[1] == 6);
>> + *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
>>> PartitionMaps[4]);
>> + break;
>> + case 0x0260:
>> + LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
>> + *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
>> + break;
>> + default:
>> + //
>> + // Unhandled UDF revision
>> + //
>> + Status = EFI_VOLUME_CORRUPTED;
>> + break;
>> + }
>> +
>> + return Status;
>> +}
>> +
>> +EFI_STATUS
>> +FindLogicalVolumeLocation (
>> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
>> + IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
>> + OUT UINT64 *MainVdsStartLsn,
>> + OUT UINT64 *LogicalVolEndLsn
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UINT32 BlockSize;
>> + EFI_LBA LastBlock;
>> + UDF_EXTENT_AD *ExtentAd;
>> + UINT64 StartingLsn;
>> + UINT64 EndingLsn;
>> + VOID *Buffer;
>> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>> + UDF_PARTITION_DESCRIPTOR *PartitionDesc;
>> + UINT64 GuardMainVdsStartLsn;
>> + UINT16 PartitionNum;
>> +
>> + BlockSize = BlockIo->Media->BlockSize;
>> + LastBlock = BlockIo->Media->LastBlock;
>> + ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>> + StartingLsn = (UINT64)ExtentAd->ExtentLocation;
>> + EndingLsn =
>> + StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength,
>> + BlockSize);
>> +
>> + LogicalVolDesc = NULL;
>> + PartitionDesc = NULL;
>> + GuardMainVdsStartLsn = StartingLsn;
>> +
>> + //
>> + // Allocate buffer for reading disk blocks // Buffer =
>> + AllocateZeroPool (BlockSize); if (Buffer == NULL) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>> +
>> + Status = EFI_VOLUME_CORRUPTED;
>> +
>> + //
>> + // As per UDF 2.60 specification:
>> + //
>> + // There shall be exactly one prevailing Logical Volume Descriptor
>> + // recorded per Volume Set.
>> + //
>> + // Start Main Volume Descriptor Sequence and find Logical Volume
>> + Descriptor // while (StartingLsn <= EndingLsn) {
>> + //
>> + // Read disk block
>> + //
>> + Status = DiskIo->ReadDisk (
>> + DiskIo,
>> + BlockIo->Media->MediaId,
>> + MultU64x32 (StartingLsn, BlockSize),
>> + BlockSize,
>> + Buffer
>> + );
>> + if (EFI_ERROR (Status)) {
>> + goto Out_Free;
>> + }
>> +
>> + //
>> + // Check if read block is a Terminating Descriptor
>> + //
>> + if (IS_TD (Buffer)) {
>> + //
>> + // Stop Main Volume Descriptor Sequence
>> + //
>> + break;
>> + }
>> +
>> + //
>> + // Check if read block is a Logical Volume Descriptor
>> + //
>> + if (IS_LVD (Buffer)) {
>> + //
>> + // Ensure only one LVD (Logical Volume Descriptor) is handled
>> + //
>> + if (LogicalVolDesc != NULL) {
>> + Status = EFI_UNSUPPORTED;
>> + goto Out_Free;
>> + }
>> +
>> + //
>> + // As per UDF 2.60 specification:
>> + //
>> + // For the purpose of interchange, Partition Maps shall be limited to
>> + // Partition Map type 1, except type 2 maps.
>> + //
>> + // NOTE: Type 1 Partitions are the only supported in this implementation.
>> + //
>> + LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
>> + if (LogicalVolDesc == NULL) {
>> + Status = EFI_OUT_OF_RESOURCES;
>> + goto Out_Free;
>> + }
>> +
>> + //
>> + // Save Logical Volume Descriptor
>> + //
>> + CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
>> + } else if (IS_PD (Buffer)) {
>> + //
>> + // Ensure only one PD (Partition Descriptor) is handled
>> + //
>> + if (PartitionDesc != NULL) {
>> + Status = EFI_UNSUPPORTED;
>> + goto Out_Free;
>> + }
>> +
>> + //
>> + // Found a Partition Descriptor.
>> + //
>> + // As per UDF 2.60 specification:
>> + //
>> + // A Partition Descriptor Access Type of read-only, rewritable,
>> + // overwritable, write-once and pseudo-overwritable shall be
>> + // supported. There shall be exactly one prevailing Partition
>> + // Descriptor recorded per volume, with one exception. For Volume
>> + // Sets that consist of single volume, the volume may contain 2 non-
>> + // overlapping Partitions with 2 prevailing Partition Descriptors only
>> + // if one has an Access Type of read-only and the other has an
>> + // Access Type of rewritable, overwritable, or write-once. The
>> + // Logical Volume for this volume would consist of the contents of
>> + // both partitions.
>> + //
>> + PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
>> + if (PartitionDesc == NULL) {
>> + Status = EFI_OUT_OF_RESOURCES;
>> + goto Out_Free;
>> + }
>> +
>> + //
>> + // Save Partition Descriptor
>> + //
>> + CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
>> + }
>> + //
>> + // Go to next disk block
>> + //
>> + StartingLsn++;
>> + }
>> +
>> + Status = EFI_VOLUME_CORRUPTED;
>> +
>> + //
>> + // Check if LVD and PD were found
>> + //
>> + if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
>> + //
>> + // Get partition number from Partition map in LVD descriptor
>> + //
>> + Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
>> + if (EFI_ERROR (Status)) {
>> + goto Out_Free;
>> + }
>> +
>> + //
>> + // Make sure we're handling expected Partition Descriptor
>> + //
>> + if (PartitionDesc->PartitionNumber != PartitionNum) {
>> + Status = EFI_VOLUME_CORRUPTED;
>> + goto Out_Free;
>> + }
>> +
>> + //
>> + // Cover the main VDS area so UdfDxe driver will also be able to get LVD
>> and
>> + // PD descriptors out from the file system.
>> + //
>> + *MainVdsStartLsn = GuardMainVdsStartLsn;
>> + *LogicalVolEndLsn = *MainVdsStartLsn +
>> + (UINT64)ExtentAd->ExtentLength;
>> +
>> + //
>> + // Cover UDF partition area
>> + //
>> + *LogicalVolEndLsn +=
>> + ((UINT64)PartitionDesc->PartitionStartingLocation -
>> + *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
>> + //
>> + // Ensure to not attempt reading past end of device
>> + //
>> + if (*LogicalVolEndLsn > LastBlock) {
>> + Status = EFI_VOLUME_CORRUPTED;
>> + } else {
>> + Status = EFI_SUCCESS;
>> + }
>> + }
>> +
>> +Out_Free:
>> + //
>> + // Free block read buffer
>> + //
>> + FreePool (Buffer);
>> + //
>> + // Free Logical Volume Descriptor
>> + //
>> + if (LogicalVolDesc != NULL) {
>> + FreePool (LogicalVolDesc);
>> + }
>> + //
>> + // Free Partition Descriptor
>> + //
>> + if (PartitionDesc != NULL) {
>> + FreePool (PartitionDesc);
>> + }
>> +
>> + return Status;
>> +}
>> +
>> +EFI_STATUS
>> +FindUdfLogicalVolume (
>> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
>> + OUT EFI_LBA *StartingLBA,
>> + OUT EFI_LBA *EndingLBA
>> + )
>> +{
>> + EFI_STATUS Status;
>> + UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
>> +
>> + //
>> + // Find UDF volume identifiers
>> + //
>> + Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo); if (EFI_ERROR
>> + (Status)) {
>> + return Status;
>> + }
>> +
>> + //
>> + // Find Anchor Volume Descriptor Pointer //
>> Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo,
>> &AnchorPoint);
>> if (EFI_ERROR (Status)) {
>> - return EFI_UNSUPPORTED;
>> + return Status;
>> }
>>
>> - return EFI_SUCCESS;
>> + //
>> + // Find Logical Volume location
>> + //
>> + Status = FindLogicalVolumeLocation (
>> + BlockIo,
>> + DiskIo,
>> + &AnchorPoint,
>> + (UINT64 *)StartingLBA,
>> + (UINT64 *)EndingLBA
>> + );
>> +
>> + return Status;
>> }
>>
>> /**
>> @@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
>> EFI_GUID *VendorDefinedGuid;
>> EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
>> EFI_PARTITION_INFO_PROTOCOL PartitionInfo;
>> + EFI_LBA StartingLBA;
>> + EFI_LBA EndingLBA;
>>
>> Media = BlockIo->Media;
>>
>> @@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
>> }
>>
>> //
>> - // Check if block device supports an UDF file system
>> + // Find UDF logical volume on block device
>> //
>> - Status = SupportUdfFileSystem (BlockIo, DiskIo);
>> + Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA,
>> + &EndingLBA);
>> if (EFI_ERROR (Status)) {
>> return EFI_NOT_FOUND;
>> }
>> @@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
>> DevicePath,
>> (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
>> &PartitionInfo,
>> - 0,
>> - Media->LastBlock,
>> + StartingLBA,
>> + EndingLBA,
>> Media->BlockSize
>> );
>> if (!EFI_ERROR (Status)) {
>> --
>> 2.11.0
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
2017-09-18 1:00 ` Ni, Ruiyu
@ 2017-09-18 16:51 ` Paulo Alcantara
2017-09-20 7:52 ` Ni, Ruiyu
0 siblings, 1 reply; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-18 16:51 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org; +Cc: Dong, Eric, Zeng, Star, Laszlo Ersek
On 9/17/2017 10:00 PM, Ni, Ruiyu wrote:
> Paulo,
> With the change in partition driver, I suppose UdfDxe driver
> only needs to take care of area covered by the partition descriptor.
> But why StartMainVolumeDescriptorSequence() still reads
> LVD, TD, and PD?
We still need the LVD again for a few reasons:
1. GetVolumeSize() needs to read LVID (Logical Volume Integrity
Descriptor), which is located in LVD, in order to calculate free and
used volume space.
2. To find FSD (File Set Descriptor), where the root directory is located.
3. To know which Partition Descriptor to use based on partition number
from LVD->PartitionMaps field. I know UDF 2.60 spec says that there
should be only one Partition Descriptor that covers the entire space of
a single LVD, but it's good to make sure we're reading it correctly.
TD descriptor is used to know when stopping the Volume Descriptor
Sequence and then we don't need to read all ExtentAd->ExtentLength
blocks unnecessarily.
Thanks,
Paulo
>
> I thought the UdfDxe driver's logic can be simplified a lot.
> There should be no duplicated logic in Partition driver and udf driver.
>
> Can you explain more?
>
> Thanks/Ray
>
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric <eric.dong@intel.com>;
>> Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star <star.zeng@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>
>> Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support
>> PartitionDxe changes
>>
>> This patch reworks the driver to support Partition driver changes.
>>
>> Cc: Eric Dong <eric.dong@intel.com>
>> Cc: Paulo Alcantara <pcacjr@zytor.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 13 +-
>> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
>> ++++++++------------
>> MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
>> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 88 +---
>> 4 files changed, 204 insertions(+), 419 deletions(-)
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> index 01361141bb..f2c62967e8 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
>> @@ -100,6 +100,7 @@ UdfOpenVolume (
>> &PrivFsData->Volume,
>> &PrivFsData->Root
>> );
>> + ASSERT_EFI_ERROR (Status);
>> if (EFI_ERROR (Status)) {
>> goto Error_Find_Root_Dir;
>> }
>> @@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
>> CleanupFileInformation (&PrivFsData->Root);
>>
>> Error_Find_Root_Dir:
>> - CleanupVolumeInformation (&PrivFsData->Volume);
>>
>> Error_Read_Udf_Volume:
>> Error_Invalid_Params:
>> @@ -528,7 +528,6 @@ UdfClose (
>> EFI_TPL OldTpl;
>> EFI_STATUS Status;
>> PRIVATE_UDF_FILE_DATA *PrivFileData;
>> - PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
>>
>> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
>>
>> @@ -541,8 +540,6 @@ UdfClose (
>>
>> PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
>>
>> - PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS (PrivFileData-
>>> SimpleFs);
>> -
>> if (!PrivFileData->IsRootDirectory) {
>> CleanupFileInformation (&PrivFileData->File);
>>
>> @@ -551,10 +548,6 @@ UdfClose (
>> }
>> }
>>
>> - if (--PrivFsData->OpenFiles == 0) {
>> - CleanupVolumeInformation (&PrivFsData->Volume);
>> - }
>> -
>> FreePool ((VOID *)PrivFileData);
>>
>> Exit:
>> @@ -787,7 +780,7 @@ UdfGetInfo (
>> } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
>> String = VolumeLabel;
>>
>> - FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
>> + FileSetDesc = &PrivFsData->Volume.FileSetDesc;
>>
>> OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
>>
>> @@ -846,7 +839,7 @@ UdfGetInfo (
>> FileSystemInfo->Size = FileSystemInfoLength;
>> FileSystemInfo->ReadOnly = TRUE;
>> FileSystemInfo->BlockSize =
>> - LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
>> + PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
>> FileSystemInfo->VolumeSize = VolumeSize;
>> FileSystemInfo->FreeSpace = FreeSpaceSize;
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> index 4609580b30..63b643e60a 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
>> @@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
>>
>> EFI_STATUS
>> StartMainVolumeDescriptorSequence (
>> - IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>> - IN EFI_DISK_IO_PROTOCOL *DiskIo,
>> - IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
>> - OUT UDF_VOLUME_INFO *Volume
>> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
>> + OUT UDF_VOLUME_INFO *Volume
>> )
>> {
>> - EFI_STATUS Status;
>> - UINT32 BlockSize;
>> - UDF_EXTENT_AD *ExtentAd;
>> - UINT64 StartingLsn;
>> - UINT64 EndingLsn;
>> - VOID *Buffer;
>> - UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
>> - UINTN Index;
>> - UINT32 LogicalBlockSize;
>> + EFI_STATUS Status;
>> + UINT32 BlockSize;
>> + UINT64 BlockOffset;
>> + VOID *Buffer;
>> + UINT32 LogicalBlockSize;
>> +
>> + BlockSize = BlockIo->Media->BlockSize;
>>
>> //
>> - // We've already found an ADVP on the volume. It contains the extent
>> - // (MainVolumeDescriptorSequenceExtent) where the Main Volume
>> Descriptor
>> - // Sequence starts. Therefore, we'll look for Logical Volume Descriptors and
>> - // Partitions Descriptors and save them in memory, accordingly.
>> - //
>> - // Note also that each descriptor will be aligned on a block size (BlockSize)
>> - // boundary, so we need to read one block at a time.
>> + // Allocate buffer for reading disk blocks
>> //
>> - BlockSize = BlockIo->Media->BlockSize;
>> - ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
>> - StartingLsn = (UINT64)ExtentAd->ExtentLocation;
>> - EndingLsn = StartingLsn + DivU64x32 (
>> - (UINT64)ExtentAd->ExtentLength,
>> - BlockSize
>> - );
>> -
>> - Volume->LogicalVolDescs =
>> - (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
>>> ExtentLength);
>> - if (Volume->LogicalVolDescs == NULL) {
>> - return EFI_OUT_OF_RESOURCES;
>> - }
>> -
>> - Volume->PartitionDescs =
>> - (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
>>> ExtentLength);
>> - if (Volume->PartitionDescs == NULL) {
>> - Status = EFI_OUT_OF_RESOURCES;
>> - goto Error_Alloc_Pds;
>> - }
>> -
>> - Buffer = AllocateZeroPool (BlockSize);
>> + Buffer = AllocateZeroPool ((UINTN)BlockSize);
>> if (Buffer == NULL) {
>> - Status = EFI_OUT_OF_RESOURCES;
>> - goto Error_Alloc_Buf;
>> + return EFI_OUT_OF_RESOURCES;
>> }
>>
>> - Volume->LogicalVolDescsNo = 0;
>> - Volume->PartitionDescsNo = 0;
>> -
>> - while (StartingLsn <= EndingLsn) {
>> + //
>> + // As per UDF 2.60 specification:
>> + //
>> + // ---
>> + // For the purpose of interchange, Partition Maps shall be limited to
>> + // Partition Map type 1, except type 2 maps.
>> + // ---
>> + // A Partition Descriptor Access Type of read-only, rewritable, //
>> + overwritable, write-once and pseudo-overwritable shall be //
>> + supported. There shall be exactly one prevailing Partition //
>> + Descriptor recorded per volume, with one exception. For Volume //
>> + Sets that consist of single volume, the volume may contain 2 non- //
>> + overlapping Partitions with 2 prevailing Partition Descriptors only
>> + // if one has an Access Type of read-only and the other has an //
>> + Access Type of rewritable, overwritable, or write-once. The //
>> + Logical Volume for this volume would consist of the contents of //
>> + both partitions.
>> + // ---
>> + //
>> + // The logical partition created by Partition driver is relative to
>> + the main // VDS extent location, so we start the Main Volume
>> + Descriptor Sequence at // LBA 0.
>> + //
>> + // We don't need to make sure we got both LVD and PD -- that is, if
>> + we are // here, that means Partition driver was able to find them both
>> previously.
>> + //
>> + for (BlockOffset = 0;
>> + BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
>> + BlockIo->Media->BlockSize);
>> + BlockOffset += BlockSize) {
>> + // Read disk block
>> + //
>> Status = DiskIo->ReadDisk (
>> DiskIo,
>> BlockIo->Media->MediaId,
>> - MultU64x32 (StartingLsn, BlockSize),
>> + BlockOffset,
>> BlockSize,
>> Buffer
>> );
>> if (EFI_ERROR (Status)) {
>> - goto Error_Read_Disk_Blk;
>> + goto Out_Free;
>> }
>>
>> + //
>> + // Check if read block is a Terminating Descriptor
>> + //
>> if (IS_TD (Buffer)) {
>> //
>> - // Found a Terminating Descriptor. Stop the sequence then.
>> + // Terminate Main Volume Descriptor Sequence
>> //
>> break;
>> }
>>
>> if (IS_LVD (Buffer)) {
>> //
>> - // Found a Logical Volume Descriptor.
>> + // Save Logical Volume Descriptor
>> //
>> - LogicalVolDesc =
>> - (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
>> - AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
>> - if (LogicalVolDesc == NULL) {
>> - Status = EFI_OUT_OF_RESOURCES;
>> - goto Error_Alloc_Lvd;
>> - }
>> -
>> - CopyMem ((VOID *)LogicalVolDesc, Buffer,
>> - sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
>> - Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
>> LogicalVolDesc;
>> + CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof
>> + (Volume->LogicalVolDesc));
>> } else if (IS_PD (Buffer)) {
>> //
>> - // Found a Partition Descriptor.
>> + // Save Partition Descriptor
>> //
>> - PartitionDesc =
>> - (UDF_PARTITION_DESCRIPTOR *)
>> - AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
>> - if (PartitionDesc == NULL) {
>> - Status = EFI_OUT_OF_RESOURCES;
>> - goto Error_Alloc_Pd;
>> - }
>> -
>> - CopyMem ((VOID *)PartitionDesc, Buffer,
>> - sizeof (UDF_PARTITION_DESCRIPTOR));
>> - Volume->PartitionDescs[Volume->PartitionDescsNo++] = PartitionDesc;
>> + CopyMem (&Volume->PartitionDesc, Buffer, sizeof
>> + (Volume->PartitionDesc));
>> }
>> -
>> - StartingLsn++;
>> }
>>
>> //
>> - // When an UDF volume (revision 2.00 or higher) contains a File Entry rather
>> - // than an Extended File Entry (which is not recommended as per spec), we
>> need
>> - // to make sure the size of a FE will be _at least_ 2048
>> - // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward compatibility.
>> + // Determine FE (File Entry) size
>> //
>> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>> if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
>> - Volume->FileEntrySize = LogicalBlockSize;
>> + Volume->FileEntrySize = (UINTN)LogicalBlockSize;
>> } else {
>> Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
>> }
>>
>> - FreePool (Buffer);
>> -
>> - return EFI_SUCCESS;
>> -
>> -Error_Alloc_Pd:
>> -Error_Alloc_Lvd:
>> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
>> - FreePool ((VOID *)Volume->PartitionDescs[Index]);
>> - }
>> -
>> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> - FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
>> - }
>> + Status = EFI_SUCCESS;
>>
>> -Error_Read_Disk_Blk:
>> +Out_Free:
>> + //
>> + // Free block read buffer
>> + //
>> FreePool (Buffer);
>>
>> -Error_Alloc_Buf:
>> - FreePool ((VOID *)Volume->PartitionDescs);
>> - Volume->PartitionDescs = NULL;
>> -
>> -Error_Alloc_Pds:
>> - FreePool ((VOID *)Volume->LogicalVolDescs);
>> - Volume->LogicalVolDescs = NULL;
>> -
>> return Status;
>> }
>>
>> @@ -223,11 +180,9 @@ GetPdFromLongAd (
>> )
>> {
>> UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>> - UINTN Index;
>> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
>> UINT16 PartitionNum;
>>
>> - LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
>> + LogicalVolDesc = &Volume->LogicalVolDesc;
>>
>> switch (LV_UDF_REVISION (LogicalVolDesc)) {
>> case 0x0102:
>> @@ -252,19 +207,21 @@ GetPdFromLongAd (
>> PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc->PartitionMaps[4]);
>> break;
>> case 0x0260:
>> - //
>> - // Fall through.
>> - //
>> - default:
>> PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
>> break;
>> + default:
>> + //
>> + // Unhandled UDF revision
>> + //
>> + return NULL;
>> }
>>
>> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
>> - PartitionDesc = Volume->PartitionDescs[Index];
>> - if (PartitionDesc->PartitionNumber == PartitionNum) {
>> - return PartitionDesc;
>> - }
>> + //
>> + // Check if partition number matches Partition Descriptor found in
>> + Main Volume // Descriptor Sequence.
>> + //
>> + if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
>> + return &Volume->PartitionDesc;
>> }
>>
>> return NULL;
>> @@ -284,8 +241,9 @@ GetLongAdLsn (
>> PartitionDesc = GetPdFromLongAd (Volume, LongAd);
>> ASSERT (PartitionDesc != NULL);
>>
>> - return (UINT64)PartitionDesc->PartitionStartingLocation +
>> - LongAd->ExtentLocation.LogicalBlockNumber;
>> + return (UINT64)PartitionDesc->PartitionStartingLocation -
>> + Volume->MainVdsStartLocation +
>> + LongAd->ExtentLocation.LogicalBlockNumber;
>> }
>>
>> //
>> @@ -293,12 +251,13 @@ GetLongAdLsn (
>> //
>> UINT64
>> GetShortAdLsn (
>> + IN UDF_VOLUME_INFO *Volume,
>> IN UDF_PARTITION_DESCRIPTOR *PartitionDesc,
>> IN UDF_SHORT_ALLOCATION_DESCRIPTOR *ShortAd
>> )
>> {
>> - return (UINT64)PartitionDesc->PartitionStartingLocation +
>> - ShortAd->ExtentPosition;
>> + return (UINT64)PartitionDesc->PartitionStartingLocation -
>> + Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
>> }
>>
>> //
>> @@ -311,36 +270,39 @@ EFI_STATUS
>> FindFileSetDescriptor (
>> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>> IN EFI_DISK_IO_PROTOCOL *DiskIo,
>> - IN UDF_VOLUME_INFO *Volume,
>> - IN UINTN LogicalVolDescNum,
>> - OUT UDF_FILE_SET_DESCRIPTOR *FileSetDesc
>> + IN UDF_VOLUME_INFO *Volume
>> )
>> {
>> EFI_STATUS Status;
>> UINT64 Lsn;
>> UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>>
>> - LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
>> + LogicalVolDesc = &Volume->LogicalVolDesc;
>> Lsn = GetLongAdLsn (Volume, &LogicalVolDesc-
>>> LogicalVolumeContentsUse);
>>
>> //
>> - // Read extent (Long Ad).
>> + // As per UDF 2.60 specification:
>> + //
>> + // There shall be exactly one File Set Descriptor recorded per
>> + Logical // Volume.
>> + //
>> + // Read disk block
>> //
>> Status = DiskIo->ReadDisk (
>> DiskIo,
>> BlockIo->Media->MediaId,
>> MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
>> - sizeof (UDF_FILE_SET_DESCRIPTOR),
>> - (VOID *)FileSetDesc
>> + sizeof (Volume->FileSetDesc),
>> + &Volume->FileSetDesc
>> );
>> if (EFI_ERROR (Status)) {
>> return Status;
>> }
>>
>> //
>> - // Check if the read extent contains a valid FSD's tag identifier.
>> + // Check if read block is a File Set Descriptor
>> //
>> - if (!IS_FSD (FileSetDesc)) {
>> + if (!IS_FSD (&Volume->FileSetDesc)) {
>> return EFI_VOLUME_CORRUPTED;
>> }
>>
>> @@ -348,71 +310,6 @@ FindFileSetDescriptor ( }
>>
>> //
>> -// Get all File Set Descriptors for each Logical Volume Descriptor.
>> -//
>> -EFI_STATUS
>> -GetFileSetDescriptors (
>> - IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
>> - IN EFI_DISK_IO_PROTOCOL *DiskIo,
>> - IN OUT UDF_VOLUME_INFO *Volume
>> - )
>> -{
>> - EFI_STATUS Status;
>> - UINTN Index;
>> - UDF_FILE_SET_DESCRIPTOR *FileSetDesc;
>> - UINTN Count;
>> -
>> - Volume->FileSetDescs =
>> - (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
>> - Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
>> - if (Volume->FileSetDescs == NULL) {
>> - return EFI_OUT_OF_RESOURCES;
>> - }
>> -
>> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> - FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
>> - if (FileSetDesc == NULL) {
>> - Status = EFI_OUT_OF_RESOURCES;
>> - goto Error_Alloc_Fsd;
>> - }
>> -
>> - //
>> - // Find a FSD for this LVD.
>> - //
>> - Status = FindFileSetDescriptor (
>> - BlockIo,
>> - DiskIo,
>> - Volume,
>> - Index,
>> - FileSetDesc
>> - );
>> - if (EFI_ERROR (Status)) {
>> - goto Error_Find_Fsd;
>> - }
>> -
>> - //
>> - // Got one. Save it.
>> - //
>> - Volume->FileSetDescs[Index] = FileSetDesc;
>> - }
>> -
>> - Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
>> - return EFI_SUCCESS;
>> -
>> -Error_Find_Fsd:
>> - Count = Index + 1;
>> - for (Index = 0; Index < Count; Index++) {
>> - FreePool ((VOID *)Volume->FileSetDescs[Index]);
>> - }
>> -
>> - FreePool ((VOID *)Volume->FileSetDescs);
>> - Volume->FileSetDescs = NULL;
>> -
>> -Error_Alloc_Fsd:
>> - return Status;
>> -}
>> -
>> -//
>> // Read Volume and File Structure on an UDF file system.
>> //
>> EFI_STATUS
>> @@ -424,9 +321,10 @@ ReadVolumeFileStructure ( {
>> EFI_STATUS Status;
>> UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
>> + UDF_EXTENT_AD *ExtentAd;
>>
>> //
>> - // Find an AVDP.
>> + // Find Anchor Volume Descriptor Pointer
>> //
>> Status = FindAnchorVolumeDescriptorPointer (
>> BlockIo,
>> @@ -438,12 +336,18 @@ ReadVolumeFileStructure (
>> }
>>
>> //
>> - // AVDP has been found. Start MVDS.
>> + // Save Main VDS start block number
>> + //
>> + ExtentAd = &AnchorPoint.MainVolumeDescriptorSequenceExtent;
>> +
>> + Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
>> +
>> + //
>> + // Start Main Volume Descriptor Sequence.
>> //
>> Status = StartMainVolumeDescriptorSequence (
>> BlockIo,
>> DiskIo,
>> - &AnchorPoint,
>> Volume
>> );
>> if (EFI_ERROR (Status)) {
>> @@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
>> return GetLongAdLsn (Volume, (UDF_LONG_ALLOCATION_DESCRIPTOR
>> *)Ad);
>> } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
>> return GetShortAdLsn (
>> + Volume,
>> GetPdFromLongAd (Volume, ParentIcb),
>> (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
>> );
>> @@ -740,7 +645,7 @@ GetAedAdsOffset (
>> return EFI_OUT_OF_RESOURCES;
>> }
>>
>> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>>
>> //
>> // Read extent.
>> @@ -890,7 +795,7 @@ ReadFile (
>> UINT32 ExtentLength;
>> UDF_FE_RECORDING_FLAGS RecordingFlags;
>>
>> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>> DoFreeAed = FALSE;
>>
>> //
>> @@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation ( {
>> EFI_STATUS Status;
>>
>> + //
>> + // Read all necessary UDF volume information and keep it private to
>> + the driver //
>> Status = ReadVolumeFileStructure (
>> BlockIo,
>> DiskIo,
>> @@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
>> return Status;
>> }
>>
>> - Status = GetFileSetDescriptors (
>> - BlockIo,
>> - DiskIo,
>> - Volume
>> - );
>> + //
>> + // Find File Set Descriptor
>> + //
>> + Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
>> if (EFI_ERROR (Status)) {
>> - CleanupVolumeInformation (Volume);
>> + return Status;
>> }
>>
>> return Status;
>> @@ -1407,7 +1314,7 @@ FindRootDirectory (
>> BlockIo,
>> DiskIo,
>> Volume,
>> - &Volume->FileSetDescs[0]->RootDirectoryIcb,
>> + &Volume->FileSetDesc.RootDirectoryIcb,
>> &File->FileEntry
>> );
>> if (EFI_ERROR (Status)) {
>> @@ -1424,7 +1331,7 @@ FindRootDirectory (
>> L"\\",
>> NULL,
>> &Parent,
>> - &Volume->FileSetDescs[0]->RootDirectoryIcb,
>> + &Volume->FileSetDesc.RootDirectoryIcb,
>> File
>> );
>> if (EFI_ERROR (Status)) {
>> @@ -1465,7 +1372,7 @@ FindFileEntry (
>> UINT32 LogicalBlockSize;
>>
>> Lsn = GetLongAdLsn (Volume, Icb);
>> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
>> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
>>
>> *FileEntry = AllocateZeroPool (Volume->FileEntrySize);
>> if (*FileEntry == NULL) {
>> @@ -1959,43 +1866,6 @@ Error_Find_File:
>> }
>>
>> /**
>> - Clean up in-memory UDF volume information.
>> -
>> - @param[in] Volume Volume information pointer.
>> -
>> -**/
>> -VOID
>> -CleanupVolumeInformation (
>> - IN UDF_VOLUME_INFO *Volume
>> - )
>> -{
>> - UINTN Index;
>> -
>> - if (Volume->LogicalVolDescs != NULL) {
>> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> - FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
>> - }
>> - FreePool ((VOID *)Volume->LogicalVolDescs);
>> - }
>> -
>> - if (Volume->PartitionDescs != NULL) {
>> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
>> - FreePool ((VOID *)Volume->PartitionDescs[Index]);
>> - }
>> - FreePool ((VOID *)Volume->PartitionDescs);
>> - }
>> -
>> - if (Volume->FileSetDescs != NULL) {
>> - for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
>> - FreePool ((VOID *)Volume->FileSetDescs[Index]);
>> - }
>> - FreePool ((VOID *)Volume->FileSetDescs);
>> - }
>> -
>> - ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO)); -}
>> -
>> -/**
>> Clean up in-memory UDF file information.
>>
>> @param[in] File File information pointer.
>> @@ -2249,91 +2119,100 @@ GetVolumeSize (
>> OUT UINT64 *FreeSpaceSize
>> )
>> {
>> - UDF_EXTENT_AD ExtentAd;
>> - UINT32 LogicalBlockSize;
>> - UINT64 Lsn;
>> - EFI_STATUS Status;
>> - UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
>> - UINTN Index;
>> - UINTN Length;
>> - UINT32 LsnsNo;
>> -
>> - *VolumeSize = 0;
>> - *FreeSpaceSize = 0;
>> -
>> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
>> - CopyMem ((VOID *)&ExtentAd,
>> - (VOID *)&Volume->LogicalVolDescs[Index]-
>>> IntegritySequenceExtent,
>> - sizeof (UDF_EXTENT_AD));
>> - if (ExtentAd.ExtentLength == 0) {
>> - continue;
>> - }
>> + EFI_STATUS Status;
>> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
>> + UDF_EXTENT_AD *ExtentAd;
>> + UINT64 Lsn;
>> + UINT32 LogicalBlockSize;
>> + UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
>> + UINTN Index;
>> + UINTN Length;
>> + UINT32 LsnsNo;
>>
>> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
>> + LogicalVolDesc = &Volume->LogicalVolDesc;
>>
>> - Read_Next_Sequence:
>> - LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
>> - AllocatePool (ExtentAd.ExtentLength);
>> - if (LogicalVolInt == NULL) {
>> - return EFI_OUT_OF_RESOURCES;
>> - }
>> + ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
>>
>> - Lsn = (UINT64)ExtentAd.ExtentLocation;
>> + if (ExtentAd->ExtentLength == 0) {
>> + return EFI_VOLUME_CORRUPTED;
>> + }
>>
>> - Status = DiskIo->ReadDisk (
>> - DiskIo,
>> - BlockIo->Media->MediaId,
>> - MultU64x32 (Lsn, LogicalBlockSize),
>> - ExtentAd.ExtentLength,
>> - (VOID *)LogicalVolInt
>> - );
>> - if (EFI_ERROR (Status)) {
>> - FreePool ((VOID *)LogicalVolInt);
>> - return Status;
>> - }
>> + LogicalVolInt = AllocatePool (ExtentAd->ExtentLength); if
>> + (LogicalVolInt == NULL) {
>> + return EFI_OUT_OF_RESOURCES;
>> + }
>>
>> - if (!IS_LVID (LogicalVolInt)) {
>> - FreePool ((VOID *)LogicalVolInt);
>> - return EFI_VOLUME_CORRUPTED;
>> - }
>> + //
>> + // Get location of Logical Volume Integrity Descriptor // Lsn =
>> + (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
>>
>> - Length = LogicalVolInt->NumberOfPartitions;
>> - for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
>> - LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> - if (LsnsNo == 0xFFFFFFFFUL) {
>> - //
>> - // Size not specified.
>> - //
>> - continue;
>> - }
>> + LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
>>
>> - *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
>> - }
>> + //
>> + // Read disk block
>> + //
>> + Status = DiskIo->ReadDisk (
>> + DiskIo,
>> + BlockIo->Media->MediaId,
>> + MultU64x32 (Lsn, LogicalBlockSize),
>> + ExtentAd->ExtentLength,
>> + LogicalVolInt
>> + );
>> + if (EFI_ERROR (Status)) {
>> + goto Out_Free;
>> + }
>>
>> - Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
>> - for (; Index < Length; Index += sizeof (UINT32)) {
>> - LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> - if (LsnsNo == 0xFFFFFFFFUL) {
>> - //
>> - // Size not specified.
>> - //
>> - continue;
>> - }
>> + //
>> + // Check if read block is a Logical Volume Integrity Descriptor //
>> + if (!IS_LVID (LogicalVolInt)) {
>> + Status = EFI_VOLUME_CORRUPTED;
>> + goto Out_Free;
>> + }
>>
>> - *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
>> - }
>> + *VolumeSize = 0;
>> + *FreeSpaceSize = 0;
>>
>> - CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt-
>>> NextIntegrityExtent,
>> - sizeof (UDF_EXTENT_AD));
>> - if (ExtentAd.ExtentLength > 0) {
>> - FreePool ((VOID *)LogicalVolInt);
>> - goto Read_Next_Sequence;
>> + Length = LogicalVolInt->NumberOfPartitions;
>> + for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
>> + LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> + //
>> + // Check if size is not specified
>> + //
>> + if (LsnsNo == 0xFFFFFFFFUL) {
>> + continue;
>> }
>> + //
>> + // Accumulate free space size
>> + //
>> + *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize); }
>>
>> - FreePool ((VOID *)LogicalVolInt);
>> + Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
>> + for (; Index < Length; Index += sizeof (UINT32)) {
>> + LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
>> + //
>> + // Check if size is not specified
>> + //
>> + if (LsnsNo == 0xFFFFFFFFUL) {
>> + continue;
>> + }
>> + //
>> + // Accumulate used volume space
>> + //
>> + *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
>> }
>>
>> - return EFI_SUCCESS;
>> + Status = EFI_SUCCESS;
>> +
>> +Out_Free:
>> + //
>> + // Free Logical Volume Integrity Descriptor
>> + //
>> + FreePool (LogicalVolInt);
>> +
>> + return Status;
>> }
>>
>> /**
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> index 49dc7077b7..d4163b89ca 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
>> @@ -276,13 +276,6 @@ UdfDriverBindingStop (
>> NULL
>> );
>>
>> - //
>> - // Check if there's any open file. If so, clean them up.
>> - //
>> - if (PrivFsData->OpenFiles > 0) {
>> - CleanupVolumeInformation (&PrivFsData->Volume);
>> - }
>> -
>> FreePool ((VOID *)PrivFsData);
>> }
>>
>> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> index 240d420ff5..c5f83914d8 100644
>> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
>> @@ -49,16 +49,8 @@
>> { 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A } \
>> }
>>
>> -#define UDF_DEFAULT_LV_NUM 0
>> -
>> #define IS_PVD(_Pointer) \
>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1)) -#define IS_PD(_Pointer) \
>> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) -#define IS_LVD(_Pointer) \
>> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) -#define IS_TD(_Pointer) \
>> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8)) #define IS_FSD(_Pointer) \
>> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256)) #define IS_FE(_Pointer) \
>> @@ -152,14 +144,8 @@ typedef enum { #define
>> IS_VALID_COMPRESSION_ID(_CompId) \
>> ((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
>>
>> -#define LV_BLOCK_SIZE(_Vol, _LvNum) \
>> - (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
>> -
>> #define UDF_STANDARD_IDENTIFIER_LENGTH 5
>>
>> -#define LV_UDF_REVISION(_Lv) \
>> - *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
>> -
>> #pragma pack(1)
>>
>> typedef struct {
>> @@ -186,17 +172,6 @@ typedef struct {
>> #pragma pack(1)
>>
>> typedef struct {
>> - UINT8 CharacterSetType;
>> - UINT8 CharacterSetInfo[63];
>> -} UDF_CHAR_SPEC;
>> -
>> -typedef struct {
>> - UINT8 Flags;
>> - UINT8 Identifier[23];
>> - UINT8 IdentifierSuffix[8];
>> -} UDF_ENTITY_ID;
>> -
>> -typedef struct {
>> UINT16 TypeAndTimezone;
>> INT16 Year;
>> UINT8 Month;
>> @@ -210,17 +185,6 @@ typedef struct {
>> } UDF_TIMESTAMP;
>>
>> typedef struct {
>> - UINT32 LogicalBlockNumber;
>> - UINT16 PartitionReferenceNumber;
>> -} UDF_LB_ADDR;
>> -
>> -typedef struct {
>> - UINT32 ExtentLength;
>> - UDF_LB_ADDR ExtentLocation;
>> - UINT8 ImplementationUse[6];
>> -} UDF_LONG_ALLOCATION_DESCRIPTOR;
>> -
>> -typedef struct {
>> UDF_DESCRIPTOR_TAG DescriptorTag;
>> UINT32 PrevAllocationExtentDescriptor;
>> UINT32 LengthOfAllocationDescriptors;
>> @@ -235,37 +199,6 @@ typedef struct {
>> } UDF_VOLUME_DESCRIPTOR;
>>
>> typedef struct {
>> - UDF_DESCRIPTOR_TAG DescriptorTag;
>> - UINT32 VolumeDescriptorSequenceNumber;
>> - UINT16 PartitionFlags;
>> - UINT16 PartitionNumber;
>> - UDF_ENTITY_ID PartitionContents;
>> - UINT8 PartitionContentsUse[128];
>> - UINT32 AccessType;
>> - UINT32 PartitionStartingLocation;
>> - UINT32 PartitionLength;
>> - UDF_ENTITY_ID ImplementationIdentifier;
>> - UINT8 ImplementationUse[128];
>> - UINT8 Reserved[156];
>> -} UDF_PARTITION_DESCRIPTOR;
>> -
>> -typedef struct {
>> - UDF_DESCRIPTOR_TAG DescriptorTag;
>> - UINT32 VolumeDescriptorSequenceNumber;
>> - UDF_CHAR_SPEC DescriptorCharacterSet;
>> - UINT8 LogicalVolumeIdentifier[128];
>> - UINT32 LogicalBlockSize;
>> - UDF_ENTITY_ID DomainIdentifier;
>> - UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
>> - UINT32 MapTableLength;
>> - UINT32 NumberOfPartitionMaps;
>> - UDF_ENTITY_ID ImplementationIdentifier;
>> - UINT8 ImplementationUse[128];
>> - UDF_EXTENT_AD IntegritySequenceExtent;
>> - UINT8 PartitionMaps[6];
>> -} UDF_LOGICAL_VOLUME_DESCRIPTOR;
>> -
>> -typedef struct {
>> UDF_DESCRIPTOR_TAG DescriptorTag;
>> UDF_TIMESTAMP RecordingDateTime;
>> UINT32 IntegrityType;
>> @@ -389,12 +322,10 @@ typedef struct {
>> // UDF filesystem driver's private data // typedef struct {
>> - UDF_LOGICAL_VOLUME_DESCRIPTOR **LogicalVolDescs;
>> - UINTN LogicalVolDescsNo;
>> - UDF_PARTITION_DESCRIPTOR **PartitionDescs;
>> - UINTN PartitionDescsNo;
>> - UDF_FILE_SET_DESCRIPTOR **FileSetDescs;
>> - UINTN FileSetDescsNo;
>> + UINT64 MainVdsStartLocation;
>> + UDF_LOGICAL_VOLUME_DESCRIPTOR LogicalVolDesc;
>> + UDF_PARTITION_DESCRIPTOR PartitionDesc;
>> + UDF_FILE_SET_DESCRIPTOR FileSetDesc;
>> UINTN FileEntrySize;
>> } UDF_VOLUME_INFO;
>>
>> @@ -883,17 +814,6 @@ ResolveSymlink (
>> );
>>
>> /**
>> - Clean up in-memory UDF volume information.
>> -
>> - @param[in] Volume Volume information pointer.
>> -
>> -**/
>> -VOID
>> -CleanupVolumeInformation (
>> - IN UDF_VOLUME_INFO *Volume
>> - );
>> -
>> -/**
>> Clean up in-memory UDF file information.
>>
>> @param[in] File File information pointer.
>> --
>> 2.11.0
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 0/3] UDF partition driver fix
2017-09-18 4:52 ` [PATCH v2 0/3] UDF partition driver fix Ni, Ruiyu
@ 2017-09-18 17:49 ` Paulo Alcantara
0 siblings, 0 replies; 20+ messages in thread
From: Paulo Alcantara @ 2017-09-18 17:49 UTC (permalink / raw)
To: Ni, Ruiyu, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Gao, Liming, Laszlo Ersek, Zeng, Star,
Yao, Jiewen
Hi Ruiyu,
On 9/18/2017 1:52 AM, Ni, Ruiyu wrote:
> Paulo,
> Could you please paste a "map -r" output on a CDROM which
> contains Eltorito volume?
> I want to confirm that the result is expected.
With UdfDxe driver disabled in OVMF:
>Mapping table
> FS0: Alias(s):CD0c65535a1:;BLK2:
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x1)
> BLK0: Alias(s):
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)
> BLK1: Alias(s):
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x0)
> BLK3: Alias(s):
>
PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)
With UdfDxe driver enabled in OVMF:
> Mapping table
> FS0: Alias(s):CD0c65535a1:;BLK2:
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x1)
> FS1: Alias(s):F0c65535a:;BLK3:
>
PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/VenMedia(C5BD4D42-1A76-4996-8956-73CDA326CD0A)
> BLK0: Alias(s):
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)
> BLK1: Alias(s):
> PciRoot(0x0)/Pci(0x1F,0x2)/Sata(0x2,0xFFFF,0x0)/CDROM(0x0)
Thanks!
Paulo
>
> Thanks/Ray
>
>> -----Original Message-----
>> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
>> Sent: Sunday, September 17, 2017 9:13 PM
>> To: edk2-devel@lists.01.org
>> Cc: Paulo Alcantara <pcacjr@zytor.com>; Kinney, Michael D
>> <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; Laszlo
>> Ersek <lersek@redhat.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
>> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
>> Subject: [PATCH v2 0/3] UDF partition driver fix
>>
>> REF: https://bugzilla.tianocore.org/show_bug.cgi?id=707
>>
>> Hi,
>>
>> This patchset fixes a bug in Partition driver that UDF partitions occupied the
>> entire disk space instead of using LVD space only.
>>
>> BTW, I've only tested it under OVMF and built it with GCC only. That would
>> be appreciable if someone could build with other toolchains and see if this
>> doesn't break.
>>
>> I used a Windows 10 ISO image with UdfDxe disabled and enabled. The `map
>> -r` output seemed OK. No breakage when booting an OS off an ElTorito
>> partition from an UDF bridge disk.
>>
>> v1->v2:
>> - Followed Laszlo's suggestions to submit a proper patchset. Thanks!
>> - As I'm still waiting for Ruiyu and Star to test this fix, I took
>> advantage of it and did some code cleanups :-)
>>
>> Repo: https://github.com/pcacjr/edk2.git
>> Branch: udf-partition-fix-v2
>>
>> Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> Cc: Liming Gao <liming.gao@intel.com>
>> Cc: Laszlo Ersek <lersek@redhat.com>
>> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
>> Cc: Star Zeng <star.zeng@intel.com>
>> Cc: Jiewen Yao <jiewen.yao@intel.com>
>> Contributed-under: TianoCore Contribution Agreement 1.1
>> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
>> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
>> ---
>>
>> Paulo Alcantara (3):
>> MdePkg: Add UDF volume structure definitions
>> MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
>> MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
>>
>> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
>> +++++++++++-
>> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 13 +-
>> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
>> ++++++++------------
>> MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
>> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 88 +---
>> MdePkg/Include/IndustryStandard/Udf.h | 63 +++
>> 6 files changed, 566 insertions(+), 443 deletions(-)
>>
>> --
>> 2.11.0
>
>
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes
2017-09-18 16:51 ` Paulo Alcantara
@ 2017-09-20 7:52 ` Ni, Ruiyu
0 siblings, 0 replies; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-20 7:52 UTC (permalink / raw)
To: Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star, Laszlo Ersek
OK I understood the reason now.
I tried to understand the relationship between AVDP, LVD, and PD, but failed.
The code below in Partition driver seems to say the LVD and PD are close to each other.
Is it always true? If it is, I agree with the implementation.
//
// Cover the main VDS area so UdfDxe driver will also be able to get LVD and
// PD descriptors out from the file system.
//
*MainVdsStartLsn = GuardMainVdsStartLsn;
*LogicalVolEndLsn = *MainVdsStartLsn + (UINT64)ExtentAd->ExtentLength;
//
// Cover UDF partition area
//
*LogicalVolEndLsn +=
((UINT64)PartitionDesc->PartitionStartingLocation -
*LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
Thanks/Ray
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Tuesday, September 19, 2017 12:51 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to
> support PartitionDxe changes
>
>
>
> On 9/17/2017 10:00 PM, Ni, Ruiyu wrote:
> > Paulo,
> > With the change in partition driver, I suppose UdfDxe driver only
> > needs to take care of area covered by the partition descriptor.
> > But why StartMainVolumeDescriptorSequence() still reads LVD, TD, and
> > PD?
>
> We still need the LVD again for a few reasons:
>
> 1. GetVolumeSize() needs to read LVID (Logical Volume Integrity Descriptor),
> which is located in LVD, in order to calculate free and used volume space.
> 2. To find FSD (File Set Descriptor), where the root directory is located.
> 3. To know which Partition Descriptor to use based on partition number from
> LVD->PartitionMaps field. I know UDF 2.60 spec says that there should be
> only one Partition Descriptor that covers the entire space of a single LVD, but
> it's good to make sure we're reading it correctly.
>
> TD descriptor is used to know when stopping the Volume Descriptor
> Sequence and then we don't need to read all ExtentAd->ExtentLength blocks
> unnecessarily.
>
> Thanks,
> Paulo
>
> >
> > I thought the UdfDxe driver's logic can be simplified a lot.
> > There should be no duplicated logic in Partition driver and udf driver.
> >
> > Can you explain more?
> >
> > Thanks/Ray
> >
> >> -----Original Message-----
> >> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> >> Sent: Sunday, September 17, 2017 9:13 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric
> >> <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> >> <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>
> >> Subject: [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to
> support
> >> PartitionDxe changes
> >>
> >> This patch reworks the driver to support Partition driver changes.
> >>
> >> Cc: Eric Dong <eric.dong@intel.com>
> >> Cc: Paulo Alcantara <pcacjr@zytor.com>
> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Cc: Star Zeng <star.zeng@intel.com>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> >> ---
> >> MdeModulePkg/Universal/Disk/UdfDxe/File.c | 13 +-
> >> MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 515
> >> ++++++++------------
> >> MdeModulePkg/Universal/Disk/UdfDxe/Udf.c | 7 -
> >> MdeModulePkg/Universal/Disk/UdfDxe/Udf.h | 88 +---
> >> 4 files changed, 204 insertions(+), 419 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> index 01361141bb..f2c62967e8 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> >> @@ -100,6 +100,7 @@ UdfOpenVolume (
> >> &PrivFsData->Volume,
> >> &PrivFsData->Root
> >> );
> >> + ASSERT_EFI_ERROR (Status);
> >> if (EFI_ERROR (Status)) {
> >> goto Error_Find_Root_Dir;
> >> }
> >> @@ -131,7 +132,6 @@ Error_Alloc_Priv_File_Data:
> >> CleanupFileInformation (&PrivFsData->Root);
> >>
> >> Error_Find_Root_Dir:
> >> - CleanupVolumeInformation (&PrivFsData->Volume);
> >>
> >> Error_Read_Udf_Volume:
> >> Error_Invalid_Params:
> >> @@ -528,7 +528,6 @@ UdfClose (
> >> EFI_TPL OldTpl;
> >> EFI_STATUS Status;
> >> PRIVATE_UDF_FILE_DATA *PrivFileData;
> >> - PRIVATE_UDF_SIMPLE_FS_DATA *PrivFsData;
> >>
> >> OldTpl = gBS->RaiseTPL (TPL_CALLBACK);
> >>
> >> @@ -541,8 +540,6 @@ UdfClose (
> >>
> >> PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
> >>
> >> - PrivFsData = PRIVATE_UDF_SIMPLE_FS_DATA_FROM_THIS
> (PrivFileData-
> >>> SimpleFs);
> >> -
> >> if (!PrivFileData->IsRootDirectory) {
> >> CleanupFileInformation (&PrivFileData->File);
> >>
> >> @@ -551,10 +548,6 @@ UdfClose (
> >> }
> >> }
> >>
> >> - if (--PrivFsData->OpenFiles == 0) {
> >> - CleanupVolumeInformation (&PrivFsData->Volume);
> >> - }
> >> -
> >> FreePool ((VOID *)PrivFileData);
> >>
> >> Exit:
> >> @@ -787,7 +780,7 @@ UdfGetInfo (
> >> } else if (CompareGuid (InformationType, &gEfiFileSystemInfoGuid)) {
> >> String = VolumeLabel;
> >>
> >> - FileSetDesc = PrivFsData->Volume.FileSetDescs[0];
> >> + FileSetDesc = &PrivFsData->Volume.FileSetDesc;
> >>
> >> OstaCompressed = &FileSetDesc->LogicalVolumeIdentifier[0];
> >>
> >> @@ -846,7 +839,7 @@ UdfGetInfo (
> >> FileSystemInfo->Size = FileSystemInfoLength;
> >> FileSystemInfo->ReadOnly = TRUE;
> >> FileSystemInfo->BlockSize =
> >> - LV_BLOCK_SIZE (&PrivFsData->Volume, UDF_DEFAULT_LV_NUM);
> >> + PrivFsData->Volume.LogicalVolDesc.LogicalBlockSize;
> >> FileSystemInfo->VolumeSize = VolumeSize;
> >> FileSystemInfo->FreeSpace = FreeSpaceSize;
> >>
> >> diff --git
> >> a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> index 4609580b30..63b643e60a 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c
> >> @@ -60,154 +60,111 @@ FindAnchorVolumeDescriptorPointer (
> >>
> >> EFI_STATUS
> >> StartMainVolumeDescriptorSequence (
> >> - IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> >> - IN EFI_DISK_IO_PROTOCOL *DiskIo,
> >> - IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
> >> - OUT UDF_VOLUME_INFO *Volume
> >> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> >> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> >> + OUT UDF_VOLUME_INFO *Volume
> >> )
> >> {
> >> - EFI_STATUS Status;
> >> - UINT32 BlockSize;
> >> - UDF_EXTENT_AD *ExtentAd;
> >> - UINT64 StartingLsn;
> >> - UINT64 EndingLsn;
> >> - VOID *Buffer;
> >> - UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> >> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
> >> - UINTN Index;
> >> - UINT32 LogicalBlockSize;
> >> + EFI_STATUS Status;
> >> + UINT32 BlockSize;
> >> + UINT64 BlockOffset;
> >> + VOID *Buffer;
> >> + UINT32 LogicalBlockSize;
> >> +
> >> + BlockSize = BlockIo->Media->BlockSize;
> >>
> >> //
> >> - // We've already found an ADVP on the volume. It contains the
> >> extent
> >> - // (MainVolumeDescriptorSequenceExtent) where the Main Volume
> >> Descriptor
> >> - // Sequence starts. Therefore, we'll look for Logical Volume
> >> Descriptors and
> >> - // Partitions Descriptors and save them in memory, accordingly.
> >> - //
> >> - // Note also that each descriptor will be aligned on a block size
> >> (BlockSize)
> >> - // boundary, so we need to read one block at a time.
> >> + // Allocate buffer for reading disk blocks
> >> //
> >> - BlockSize = BlockIo->Media->BlockSize;
> >> - ExtentAd = &AnchorPoint->MainVolumeDescriptorSequenceExtent;
> >> - StartingLsn = (UINT64)ExtentAd->ExtentLocation;
> >> - EndingLsn = StartingLsn + DivU64x32 (
> >> - (UINT64)ExtentAd->ExtentLength,
> >> - BlockSize
> >> - );
> >> -
> >> - Volume->LogicalVolDescs =
> >> - (UDF_LOGICAL_VOLUME_DESCRIPTOR **)AllocateZeroPool
> (ExtentAd-
> >>> ExtentLength);
> >> - if (Volume->LogicalVolDescs == NULL) {
> >> - return EFI_OUT_OF_RESOURCES;
> >> - }
> >> -
> >> - Volume->PartitionDescs =
> >> - (UDF_PARTITION_DESCRIPTOR **)AllocateZeroPool (ExtentAd-
> >>> ExtentLength);
> >> - if (Volume->PartitionDescs == NULL) {
> >> - Status = EFI_OUT_OF_RESOURCES;
> >> - goto Error_Alloc_Pds;
> >> - }
> >> -
> >> - Buffer = AllocateZeroPool (BlockSize);
> >> + Buffer = AllocateZeroPool ((UINTN)BlockSize);
> >> if (Buffer == NULL) {
> >> - Status = EFI_OUT_OF_RESOURCES;
> >> - goto Error_Alloc_Buf;
> >> + return EFI_OUT_OF_RESOURCES;
> >> }
> >>
> >> - Volume->LogicalVolDescsNo = 0;
> >> - Volume->PartitionDescsNo = 0;
> >> -
> >> - while (StartingLsn <= EndingLsn) {
> >> + //
> >> + // As per UDF 2.60 specification:
> >> + //
> >> + // ---
> >> + // For the purpose of interchange, Partition Maps shall be limited
> >> + to // Partition Map type 1, except type 2 maps.
> >> + // ---
> >> + // A Partition Descriptor Access Type of read-only, rewritable,
> >> + // overwritable, write-once and pseudo-overwritable shall be //
> >> + supported. There shall be exactly one prevailing Partition //
> >> + Descriptor recorded per volume, with one exception. For Volume //
> >> + Sets that consist of single volume, the volume may contain 2 non-
> >> + // overlapping Partitions with 2 prevailing Partition Descriptors
> >> + only // if one has an Access Type of read-only and the other has an
> >> + // Access Type of rewritable, overwritable, or write-once. The //
> >> + Logical Volume for this volume would consist of the contents of //
> >> + both partitions.
> >> + // ---
> >> + //
> >> + // The logical partition created by Partition driver is relative
> >> + to the main // VDS extent location, so we start the Main Volume
> >> + Descriptor Sequence at // LBA 0.
> >> + //
> >> + // We don't need to make sure we got both LVD and PD -- that is,
> >> + if we are // here, that means Partition driver was able to find
> >> + them both
> >> previously.
> >> + //
> >> + for (BlockOffset = 0;
> >> + BlockOffset < MultU64x32 (BlockIo->Media->LastBlock,
> >> + BlockIo->Media->BlockSize);
> >> + BlockOffset += BlockSize) {
> >> + // Read disk block
> >> + //
> >> Status = DiskIo->ReadDisk (
> >> DiskIo,
> >> BlockIo->Media->MediaId,
> >> - MultU64x32 (StartingLsn, BlockSize),
> >> + BlockOffset,
> >> BlockSize,
> >> Buffer
> >> );
> >> if (EFI_ERROR (Status)) {
> >> - goto Error_Read_Disk_Blk;
> >> + goto Out_Free;
> >> }
> >>
> >> + //
> >> + // Check if read block is a Terminating Descriptor
> >> + //
> >> if (IS_TD (Buffer)) {
> >> //
> >> - // Found a Terminating Descriptor. Stop the sequence then.
> >> + // Terminate Main Volume Descriptor Sequence
> >> //
> >> break;
> >> }
> >>
> >> if (IS_LVD (Buffer)) {
> >> //
> >> - // Found a Logical Volume Descriptor.
> >> + // Save Logical Volume Descriptor
> >> //
> >> - LogicalVolDesc =
> >> - (UDF_LOGICAL_VOLUME_DESCRIPTOR *)
> >> - AllocateZeroPool (sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> >> - if (LogicalVolDesc == NULL) {
> >> - Status = EFI_OUT_OF_RESOURCES;
> >> - goto Error_Alloc_Lvd;
> >> - }
> >> -
> >> - CopyMem ((VOID *)LogicalVolDesc, Buffer,
> >> - sizeof (UDF_LOGICAL_VOLUME_DESCRIPTOR));
> >> - Volume->LogicalVolDescs[Volume->LogicalVolDescsNo++] =
> >> LogicalVolDesc;
> >> + CopyMem (&Volume->LogicalVolDesc, Buffer, sizeof
> >> + (Volume->LogicalVolDesc));
> >> } else if (IS_PD (Buffer)) {
> >> //
> >> - // Found a Partition Descriptor.
> >> + // Save Partition Descriptor
> >> //
> >> - PartitionDesc =
> >> - (UDF_PARTITION_DESCRIPTOR *)
> >> - AllocateZeroPool (sizeof (UDF_PARTITION_DESCRIPTOR));
> >> - if (PartitionDesc == NULL) {
> >> - Status = EFI_OUT_OF_RESOURCES;
> >> - goto Error_Alloc_Pd;
> >> - }
> >> -
> >> - CopyMem ((VOID *)PartitionDesc, Buffer,
> >> - sizeof (UDF_PARTITION_DESCRIPTOR));
> >> - Volume->PartitionDescs[Volume->PartitionDescsNo++] =
> PartitionDesc;
> >> + CopyMem (&Volume->PartitionDesc, Buffer, sizeof
> >> + (Volume->PartitionDesc));
> >> }
> >> -
> >> - StartingLsn++;
> >> }
> >>
> >> //
> >> - // When an UDF volume (revision 2.00 or higher) contains a File
> >> Entry rather
> >> - // than an Extended File Entry (which is not recommended as per
> >> spec), we need
> >> - // to make sure the size of a FE will be _at least_ 2048
> >> - // (UDF_LOGICAL_SECTOR_SIZE) bytes long to keep backward
> compatibility.
> >> + // Determine FE (File Entry) size
> >> //
> >> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> >> if (LogicalBlockSize >= UDF_LOGICAL_SECTOR_SIZE) {
> >> - Volume->FileEntrySize = LogicalBlockSize;
> >> + Volume->FileEntrySize = (UINTN)LogicalBlockSize;
> >> } else {
> >> Volume->FileEntrySize = UDF_LOGICAL_SECTOR_SIZE;
> >> }
> >>
> >> - FreePool (Buffer);
> >> -
> >> - return EFI_SUCCESS;
> >> -
> >> -Error_Alloc_Pd:
> >> -Error_Alloc_Lvd:
> >> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> >> - FreePool ((VOID *)Volume->PartitionDescs[Index]);
> >> - }
> >> -
> >> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> - FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> >> - }
> >> + Status = EFI_SUCCESS;
> >>
> >> -Error_Read_Disk_Blk:
> >> +Out_Free:
> >> + //
> >> + // Free block read buffer
> >> + //
> >> FreePool (Buffer);
> >>
> >> -Error_Alloc_Buf:
> >> - FreePool ((VOID *)Volume->PartitionDescs);
> >> - Volume->PartitionDescs = NULL;
> >> -
> >> -Error_Alloc_Pds:
> >> - FreePool ((VOID *)Volume->LogicalVolDescs);
> >> - Volume->LogicalVolDescs = NULL;
> >> -
> >> return Status;
> >> }
> >>
> >> @@ -223,11 +180,9 @@ GetPdFromLongAd (
> >> )
> >> {
> >> UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> >> - UINTN Index;
> >> - UDF_PARTITION_DESCRIPTOR *PartitionDesc;
> >> UINT16 PartitionNum;
> >>
> >> - LogicalVolDesc = Volume->LogicalVolDescs[UDF_DEFAULT_LV_NUM];
> >> + LogicalVolDesc = &Volume->LogicalVolDesc;
> >>
> >> switch (LV_UDF_REVISION (LogicalVolDesc)) {
> >> case 0x0102:
> >> @@ -252,19 +207,21 @@ GetPdFromLongAd (
> >> PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >PartitionMaps[4]);
> >> break;
> >> case 0x0260:
> >> - //
> >> - // Fall through.
> >> - //
> >> - default:
> >> PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
> >> break;
> >> + default:
> >> + //
> >> + // Unhandled UDF revision
> >> + //
> >> + return NULL;
> >> }
> >>
> >> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> >> - PartitionDesc = Volume->PartitionDescs[Index];
> >> - if (PartitionDesc->PartitionNumber == PartitionNum) {
> >> - return PartitionDesc;
> >> - }
> >> + //
> >> + // Check if partition number matches Partition Descriptor found in
> >> + Main Volume // Descriptor Sequence.
> >> + //
> >> + if (Volume->PartitionDesc.PartitionNumber == PartitionNum) {
> >> + return &Volume->PartitionDesc;
> >> }
> >>
> >> return NULL;
> >> @@ -284,8 +241,9 @@ GetLongAdLsn (
> >> PartitionDesc = GetPdFromLongAd (Volume, LongAd);
> >> ASSERT (PartitionDesc != NULL);
> >>
> >> - return (UINT64)PartitionDesc->PartitionStartingLocation +
> >> - LongAd->ExtentLocation.LogicalBlockNumber;
> >> + return (UINT64)PartitionDesc->PartitionStartingLocation -
> >> + Volume->MainVdsStartLocation +
> >> + LongAd->ExtentLocation.LogicalBlockNumber;
> >> }
> >>
> >> //
> >> @@ -293,12 +251,13 @@ GetLongAdLsn (
> >> //
> >> UINT64
> >> GetShortAdLsn (
> >> + IN UDF_VOLUME_INFO *Volume,
> >> IN UDF_PARTITION_DESCRIPTOR *PartitionDesc,
> >> IN UDF_SHORT_ALLOCATION_DESCRIPTOR *ShortAd
> >> )
> >> {
> >> - return (UINT64)PartitionDesc->PartitionStartingLocation +
> >> - ShortAd->ExtentPosition;
> >> + return (UINT64)PartitionDesc->PartitionStartingLocation -
> >> + Volume->MainVdsStartLocation + ShortAd->ExtentPosition;
> >> }
> >>
> >> //
> >> @@ -311,36 +270,39 @@ EFI_STATUS
> >> FindFileSetDescriptor (
> >> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> >> IN EFI_DISK_IO_PROTOCOL *DiskIo,
> >> - IN UDF_VOLUME_INFO *Volume,
> >> - IN UINTN LogicalVolDescNum,
> >> - OUT UDF_FILE_SET_DESCRIPTOR *FileSetDesc
> >> + IN UDF_VOLUME_INFO *Volume
> >> )
> >> {
> >> EFI_STATUS Status;
> >> UINT64 Lsn;
> >> UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> >>
> >> - LogicalVolDesc = Volume->LogicalVolDescs[LogicalVolDescNum];
> >> + LogicalVolDesc = &Volume->LogicalVolDesc;
> >> Lsn = GetLongAdLsn (Volume, &LogicalVolDesc-
> >>> LogicalVolumeContentsUse);
> >>
> >> //
> >> - // Read extent (Long Ad).
> >> + // As per UDF 2.60 specification:
> >> + //
> >> + // There shall be exactly one File Set Descriptor recorded per
> >> + Logical // Volume.
> >> + //
> >> + // Read disk block
> >> //
> >> Status = DiskIo->ReadDisk (
> >> DiskIo,
> >> BlockIo->Media->MediaId,
> >> MultU64x32 (Lsn, LogicalVolDesc->LogicalBlockSize),
> >> - sizeof (UDF_FILE_SET_DESCRIPTOR),
> >> - (VOID *)FileSetDesc
> >> + sizeof (Volume->FileSetDesc),
> >> + &Volume->FileSetDesc
> >> );
> >> if (EFI_ERROR (Status)) {
> >> return Status;
> >> }
> >>
> >> //
> >> - // Check if the read extent contains a valid FSD's tag identifier.
> >> + // Check if read block is a File Set Descriptor
> >> //
> >> - if (!IS_FSD (FileSetDesc)) {
> >> + if (!IS_FSD (&Volume->FileSetDesc)) {
> >> return EFI_VOLUME_CORRUPTED;
> >> }
> >>
> >> @@ -348,71 +310,6 @@ FindFileSetDescriptor ( }
> >>
> >> //
> >> -// Get all File Set Descriptors for each Logical Volume Descriptor.
> >> -//
> >> -EFI_STATUS
> >> -GetFileSetDescriptors (
> >> - IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> >> - IN EFI_DISK_IO_PROTOCOL *DiskIo,
> >> - IN OUT UDF_VOLUME_INFO *Volume
> >> - )
> >> -{
> >> - EFI_STATUS Status;
> >> - UINTN Index;
> >> - UDF_FILE_SET_DESCRIPTOR *FileSetDesc;
> >> - UINTN Count;
> >> -
> >> - Volume->FileSetDescs =
> >> - (UDF_FILE_SET_DESCRIPTOR **)AllocateZeroPool (
> >> - Volume->LogicalVolDescsNo * sizeof (UDF_FILE_SET_DESCRIPTOR));
> >> - if (Volume->FileSetDescs == NULL) {
> >> - return EFI_OUT_OF_RESOURCES;
> >> - }
> >> -
> >> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> - FileSetDesc = AllocateZeroPool (sizeof (UDF_FILE_SET_DESCRIPTOR));
> >> - if (FileSetDesc == NULL) {
> >> - Status = EFI_OUT_OF_RESOURCES;
> >> - goto Error_Alloc_Fsd;
> >> - }
> >> -
> >> - //
> >> - // Find a FSD for this LVD.
> >> - //
> >> - Status = FindFileSetDescriptor (
> >> - BlockIo,
> >> - DiskIo,
> >> - Volume,
> >> - Index,
> >> - FileSetDesc
> >> - );
> >> - if (EFI_ERROR (Status)) {
> >> - goto Error_Find_Fsd;
> >> - }
> >> -
> >> - //
> >> - // Got one. Save it.
> >> - //
> >> - Volume->FileSetDescs[Index] = FileSetDesc;
> >> - }
> >> -
> >> - Volume->FileSetDescsNo = Volume->LogicalVolDescsNo;
> >> - return EFI_SUCCESS;
> >> -
> >> -Error_Find_Fsd:
> >> - Count = Index + 1;
> >> - for (Index = 0; Index < Count; Index++) {
> >> - FreePool ((VOID *)Volume->FileSetDescs[Index]);
> >> - }
> >> -
> >> - FreePool ((VOID *)Volume->FileSetDescs);
> >> - Volume->FileSetDescs = NULL;
> >> -
> >> -Error_Alloc_Fsd:
> >> - return Status;
> >> -}
> >> -
> >> -//
> >> // Read Volume and File Structure on an UDF file system.
> >> //
> >> EFI_STATUS
> >> @@ -424,9 +321,10 @@ ReadVolumeFileStructure ( {
> >> EFI_STATUS Status;
> >> UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
> >> + UDF_EXTENT_AD *ExtentAd;
> >>
> >> //
> >> - // Find an AVDP.
> >> + // Find Anchor Volume Descriptor Pointer
> >> //
> >> Status = FindAnchorVolumeDescriptorPointer (
> >> BlockIo,
> >> @@ -438,12 +336,18 @@ ReadVolumeFileStructure (
> >> }
> >>
> >> //
> >> - // AVDP has been found. Start MVDS.
> >> + // Save Main VDS start block number // ExtentAd =
> >> + &AnchorPoint.MainVolumeDescriptorSequenceExtent;
> >> +
> >> + Volume->MainVdsStartLocation = (UINT64)ExtentAd->ExtentLocation;
> >> +
> >> + //
> >> + // Start Main Volume Descriptor Sequence.
> >> //
> >> Status = StartMainVolumeDescriptorSequence (
> >> BlockIo,
> >> DiskIo,
> >> - &AnchorPoint,
> >> Volume
> >> );
> >> if (EFI_ERROR (Status)) {
> >> @@ -699,6 +603,7 @@ GetAllocationDescriptorLsn (
> >> return GetLongAdLsn (Volume,
> (UDF_LONG_ALLOCATION_DESCRIPTOR
> >> *)Ad);
> >> } else if (RecordingFlags == SHORT_ADS_SEQUENCE) {
> >> return GetShortAdLsn (
> >> + Volume,
> >> GetPdFromLongAd (Volume, ParentIcb),
> >> (UDF_SHORT_ALLOCATION_DESCRIPTOR *)Ad
> >> );
> >> @@ -740,7 +645,7 @@ GetAedAdsOffset (
> >> return EFI_OUT_OF_RESOURCES;
> >> }
> >>
> >> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> >>
> >> //
> >> // Read extent.
> >> @@ -890,7 +795,7 @@ ReadFile (
> >> UINT32 ExtentLength;
> >> UDF_FE_RECORDING_FLAGS RecordingFlags;
> >>
> >> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> >> DoFreeAed = FALSE;
> >>
> >> //
> >> @@ -1355,6 +1260,9 @@ ReadUdfVolumeInformation ( {
> >> EFI_STATUS Status;
> >>
> >> + //
> >> + // Read all necessary UDF volume information and keep it private
> >> + to the driver //
> >> Status = ReadVolumeFileStructure (
> >> BlockIo,
> >> DiskIo,
> >> @@ -1364,13 +1272,12 @@ ReadUdfVolumeInformation (
> >> return Status;
> >> }
> >>
> >> - Status = GetFileSetDescriptors (
> >> - BlockIo,
> >> - DiskIo,
> >> - Volume
> >> - );
> >> + //
> >> + // Find File Set Descriptor
> >> + //
> >> + Status = FindFileSetDescriptor (BlockIo, DiskIo, Volume);
> >> if (EFI_ERROR (Status)) {
> >> - CleanupVolumeInformation (Volume);
> >> + return Status;
> >> }
> >>
> >> return Status;
> >> @@ -1407,7 +1314,7 @@ FindRootDirectory (
> >> BlockIo,
> >> DiskIo,
> >> Volume,
> >> - &Volume->FileSetDescs[0]->RootDirectoryIcb,
> >> + &Volume->FileSetDesc.RootDirectoryIcb,
> >> &File->FileEntry
> >> );
> >> if (EFI_ERROR (Status)) {
> >> @@ -1424,7 +1331,7 @@ FindRootDirectory (
> >> L"\\",
> >> NULL,
> >> &Parent,
> >> - &Volume->FileSetDescs[0]->RootDirectoryIcb,
> >> + &Volume->FileSetDesc.RootDirectoryIcb,
> >> File
> >> );
> >> if (EFI_ERROR (Status)) {
> >> @@ -1465,7 +1372,7 @@ FindFileEntry (
> >> UINT32 LogicalBlockSize;
> >>
> >> Lsn = GetLongAdLsn (Volume, Icb);
> >> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, UDF_DEFAULT_LV_NUM);
> >> + LogicalBlockSize = Volume->LogicalVolDesc.LogicalBlockSize;
> >>
> >> *FileEntry = AllocateZeroPool (Volume->FileEntrySize);
> >> if (*FileEntry == NULL) {
> >> @@ -1959,43 +1866,6 @@ Error_Find_File:
> >> }
> >>
> >> /**
> >> - Clean up in-memory UDF volume information.
> >> -
> >> - @param[in] Volume Volume information pointer.
> >> -
> >> -**/
> >> -VOID
> >> -CleanupVolumeInformation (
> >> - IN UDF_VOLUME_INFO *Volume
> >> - )
> >> -{
> >> - UINTN Index;
> >> -
> >> - if (Volume->LogicalVolDescs != NULL) {
> >> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> - FreePool ((VOID *)Volume->LogicalVolDescs[Index]);
> >> - }
> >> - FreePool ((VOID *)Volume->LogicalVolDescs);
> >> - }
> >> -
> >> - if (Volume->PartitionDescs != NULL) {
> >> - for (Index = 0; Index < Volume->PartitionDescsNo; Index++) {
> >> - FreePool ((VOID *)Volume->PartitionDescs[Index]);
> >> - }
> >> - FreePool ((VOID *)Volume->PartitionDescs);
> >> - }
> >> -
> >> - if (Volume->FileSetDescs != NULL) {
> >> - for (Index = 0; Index < Volume->FileSetDescsNo; Index++) {
> >> - FreePool ((VOID *)Volume->FileSetDescs[Index]);
> >> - }
> >> - FreePool ((VOID *)Volume->FileSetDescs);
> >> - }
> >> -
> >> - ZeroMem ((VOID *)Volume, sizeof (UDF_VOLUME_INFO)); -}
> >> -
> >> -/**
> >> Clean up in-memory UDF file information.
> >>
> >> @param[in] File File information pointer.
> >> @@ -2249,91 +2119,100 @@ GetVolumeSize (
> >> OUT UINT64 *FreeSpaceSize
> >> )
> >> {
> >> - UDF_EXTENT_AD ExtentAd;
> >> - UINT32 LogicalBlockSize;
> >> - UINT64 Lsn;
> >> - EFI_STATUS Status;
> >> - UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
> >> - UINTN Index;
> >> - UINTN Length;
> >> - UINT32 LsnsNo;
> >> -
> >> - *VolumeSize = 0;
> >> - *FreeSpaceSize = 0;
> >> -
> >> - for (Index = 0; Index < Volume->LogicalVolDescsNo; Index++) {
> >> - CopyMem ((VOID *)&ExtentAd,
> >> - (VOID *)&Volume->LogicalVolDescs[Index]-
> >>> IntegritySequenceExtent,
> >> - sizeof (UDF_EXTENT_AD));
> >> - if (ExtentAd.ExtentLength == 0) {
> >> - continue;
> >> - }
> >> + EFI_STATUS Status;
> >> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> >> + UDF_EXTENT_AD *ExtentAd;
> >> + UINT64 Lsn;
> >> + UINT32 LogicalBlockSize;
> >> + UDF_LOGICAL_VOLUME_INTEGRITY *LogicalVolInt;
> >> + UINTN Index;
> >> + UINTN Length;
> >> + UINT32 LsnsNo;
> >>
> >> - LogicalBlockSize = LV_BLOCK_SIZE (Volume, Index);
> >> + LogicalVolDesc = &Volume->LogicalVolDesc;
> >>
> >> - Read_Next_Sequence:
> >> - LogicalVolInt = (UDF_LOGICAL_VOLUME_INTEGRITY *)
> >> - AllocatePool (ExtentAd.ExtentLength);
> >> - if (LogicalVolInt == NULL) {
> >> - return EFI_OUT_OF_RESOURCES;
> >> - }
> >> + ExtentAd = &LogicalVolDesc->IntegritySequenceExtent;
> >>
> >> - Lsn = (UINT64)ExtentAd.ExtentLocation;
> >> + if (ExtentAd->ExtentLength == 0) {
> >> + return EFI_VOLUME_CORRUPTED;
> >> + }
> >>
> >> - Status = DiskIo->ReadDisk (
> >> - DiskIo,
> >> - BlockIo->Media->MediaId,
> >> - MultU64x32 (Lsn, LogicalBlockSize),
> >> - ExtentAd.ExtentLength,
> >> - (VOID *)LogicalVolInt
> >> - );
> >> - if (EFI_ERROR (Status)) {
> >> - FreePool ((VOID *)LogicalVolInt);
> >> - return Status;
> >> - }
> >> + LogicalVolInt = AllocatePool (ExtentAd->ExtentLength); if
> >> + (LogicalVolInt == NULL) {
> >> + return EFI_OUT_OF_RESOURCES;
> >> + }
> >>
> >> - if (!IS_LVID (LogicalVolInt)) {
> >> - FreePool ((VOID *)LogicalVolInt);
> >> - return EFI_VOLUME_CORRUPTED;
> >> - }
> >> + //
> >> + // Get location of Logical Volume Integrity Descriptor // Lsn =
> >> + (UINT64)ExtentAd->ExtentLocation - Volume->MainVdsStartLocation;
> >>
> >> - Length = LogicalVolInt->NumberOfPartitions;
> >> - for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> >> - LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> - if (LsnsNo == 0xFFFFFFFFUL) {
> >> - //
> >> - // Size not specified.
> >> - //
> >> - continue;
> >> - }
> >> + LogicalBlockSize = LogicalVolDesc->LogicalBlockSize;
> >>
> >> - *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >> - }
> >> + //
> >> + // Read disk block
> >> + //
> >> + Status = DiskIo->ReadDisk (
> >> + DiskIo,
> >> + BlockIo->Media->MediaId,
> >> + MultU64x32 (Lsn, LogicalBlockSize),
> >> + ExtentAd->ExtentLength,
> >> + LogicalVolInt
> >> + );
> >> + if (EFI_ERROR (Status)) {
> >> + goto Out_Free;
> >> + }
> >>
> >> - Length = (LogicalVolInt->NumberOfPartitions * sizeof (UINT32)) << 1;
> >> - for (; Index < Length; Index += sizeof (UINT32)) {
> >> - LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> - if (LsnsNo == 0xFFFFFFFFUL) {
> >> - //
> >> - // Size not specified.
> >> - //
> >> - continue;
> >> - }
> >> + //
> >> + // Check if read block is a Logical Volume Integrity Descriptor
> >> + // if (!IS_LVID (LogicalVolInt)) {
> >> + Status = EFI_VOLUME_CORRUPTED;
> >> + goto Out_Free;
> >> + }
> >>
> >> - *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >> - }
> >> + *VolumeSize = 0;
> >> + *FreeSpaceSize = 0;
> >>
> >> - CopyMem ((VOID *)&ExtentAd,(VOID *)&LogicalVolInt-
> >>> NextIntegrityExtent,
> >> - sizeof (UDF_EXTENT_AD));
> >> - if (ExtentAd.ExtentLength > 0) {
> >> - FreePool ((VOID *)LogicalVolInt);
> >> - goto Read_Next_Sequence;
> >> + Length = LogicalVolInt->NumberOfPartitions;
> >> + for (Index = 0; Index < Length; Index += sizeof (UINT32)) {
> >> + LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> + //
> >> + // Check if size is not specified
> >> + //
> >> + if (LsnsNo == 0xFFFFFFFFUL) {
> >> + continue;
> >> }
> >> + //
> >> + // Accumulate free space size
> >> + //
> >> + *FreeSpaceSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >> + }
> >>
> >> - FreePool ((VOID *)LogicalVolInt);
> >> + Length = LogicalVolInt->NumberOfPartitions * sizeof (UINT32) * 2;
> >> + for (; Index < Length; Index += sizeof (UINT32)) {
> >> + LsnsNo = *(UINT32 *)((UINT8 *)LogicalVolInt->Data + Index);
> >> + //
> >> + // Check if size is not specified
> >> + //
> >> + if (LsnsNo == 0xFFFFFFFFUL) {
> >> + continue;
> >> + }
> >> + //
> >> + // Accumulate used volume space
> >> + //
> >> + *VolumeSize += MultU64x32 ((UINT64)LsnsNo, LogicalBlockSize);
> >> }
> >>
> >> - return EFI_SUCCESS;
> >> + Status = EFI_SUCCESS;
> >> +
> >> +Out_Free:
> >> + //
> >> + // Free Logical Volume Integrity Descriptor
> >> + //
> >> + FreePool (LogicalVolInt);
> >> +
> >> + return Status;
> >> }
> >>
> >> /**
> >> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> index 49dc7077b7..d4163b89ca 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.c
> >> @@ -276,13 +276,6 @@ UdfDriverBindingStop (
> >> NULL
> >> );
> >>
> >> - //
> >> - // Check if there's any open file. If so, clean them up.
> >> - //
> >> - if (PrivFsData->OpenFiles > 0) {
> >> - CleanupVolumeInformation (&PrivFsData->Volume);
> >> - }
> >> -
> >> FreePool ((VOID *)PrivFsData);
> >> }
> >>
> >> diff --git a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> index 240d420ff5..c5f83914d8 100644
> >> --- a/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/Udf.h
> >> @@ -49,16 +49,8 @@
> >> { 0x89, 0x56, 0x73, 0xCD, 0xA3, 0x26, 0xCD, 0x0A } \
> >> }
> >>
> >> -#define UDF_DEFAULT_LV_NUM 0
> >> -
> >> #define IS_PVD(_Pointer) \
> >> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 1)) -#define IS_PD(_Pointer)
> >> \
> >> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 5)) -#define IS_LVD(_Pointer)
> >> \
> >> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 6)) -#define IS_TD(_Pointer)
> >> \
> >> - ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 8)) #define IS_FSD(_Pointer)
> \
> >> ((BOOLEAN)(_GET_TAG_ID (_Pointer) == 256)) #define
> >> IS_FE(_Pointer) \ @@ -152,14 +144,8 @@ typedef enum { #define
> >> IS_VALID_COMPRESSION_ID(_CompId) \
> >> ((BOOLEAN)((_CompId) == 8 || (_CompId) == 16))
> >>
> >> -#define LV_BLOCK_SIZE(_Vol, _LvNum) \
> >> - (_Vol)->LogicalVolDescs[(_LvNum)]->LogicalBlockSize
> >> -
> >> #define UDF_STANDARD_IDENTIFIER_LENGTH 5
> >>
> >> -#define LV_UDF_REVISION(_Lv) \
> >> - *(UINT16 *)(UINTN)(_Lv)->DomainIdentifier.IdentifierSuffix
> >> -
> >> #pragma pack(1)
> >>
> >> typedef struct {
> >> @@ -186,17 +172,6 @@ typedef struct {
> >> #pragma pack(1)
> >>
> >> typedef struct {
> >> - UINT8 CharacterSetType;
> >> - UINT8 CharacterSetInfo[63];
> >> -} UDF_CHAR_SPEC;
> >> -
> >> -typedef struct {
> >> - UINT8 Flags;
> >> - UINT8 Identifier[23];
> >> - UINT8 IdentifierSuffix[8];
> >> -} UDF_ENTITY_ID;
> >> -
> >> -typedef struct {
> >> UINT16 TypeAndTimezone;
> >> INT16 Year;
> >> UINT8 Month;
> >> @@ -210,17 +185,6 @@ typedef struct {
> >> } UDF_TIMESTAMP;
> >>
> >> typedef struct {
> >> - UINT32 LogicalBlockNumber;
> >> - UINT16 PartitionReferenceNumber;
> >> -} UDF_LB_ADDR;
> >> -
> >> -typedef struct {
> >> - UINT32 ExtentLength;
> >> - UDF_LB_ADDR ExtentLocation;
> >> - UINT8 ImplementationUse[6];
> >> -} UDF_LONG_ALLOCATION_DESCRIPTOR;
> >> -
> >> -typedef struct {
> >> UDF_DESCRIPTOR_TAG DescriptorTag;
> >> UINT32 PrevAllocationExtentDescriptor;
> >> UINT32 LengthOfAllocationDescriptors;
> >> @@ -235,37 +199,6 @@ typedef struct {
> >> } UDF_VOLUME_DESCRIPTOR;
> >>
> >> typedef struct {
> >> - UDF_DESCRIPTOR_TAG DescriptorTag;
> >> - UINT32 VolumeDescriptorSequenceNumber;
> >> - UINT16 PartitionFlags;
> >> - UINT16 PartitionNumber;
> >> - UDF_ENTITY_ID PartitionContents;
> >> - UINT8 PartitionContentsUse[128];
> >> - UINT32 AccessType;
> >> - UINT32 PartitionStartingLocation;
> >> - UINT32 PartitionLength;
> >> - UDF_ENTITY_ID ImplementationIdentifier;
> >> - UINT8 ImplementationUse[128];
> >> - UINT8 Reserved[156];
> >> -} UDF_PARTITION_DESCRIPTOR;
> >> -
> >> -typedef struct {
> >> - UDF_DESCRIPTOR_TAG DescriptorTag;
> >> - UINT32 VolumeDescriptorSequenceNumber;
> >> - UDF_CHAR_SPEC DescriptorCharacterSet;
> >> - UINT8 LogicalVolumeIdentifier[128];
> >> - UINT32 LogicalBlockSize;
> >> - UDF_ENTITY_ID DomainIdentifier;
> >> - UDF_LONG_ALLOCATION_DESCRIPTOR LogicalVolumeContentsUse;
> >> - UINT32 MapTableLength;
> >> - UINT32 NumberOfPartitionMaps;
> >> - UDF_ENTITY_ID ImplementationIdentifier;
> >> - UINT8 ImplementationUse[128];
> >> - UDF_EXTENT_AD IntegritySequenceExtent;
> >> - UINT8 PartitionMaps[6];
> >> -} UDF_LOGICAL_VOLUME_DESCRIPTOR;
> >> -
> >> -typedef struct {
> >> UDF_DESCRIPTOR_TAG DescriptorTag;
> >> UDF_TIMESTAMP RecordingDateTime;
> >> UINT32 IntegrityType;
> >> @@ -389,12 +322,10 @@ typedef struct {
> >> // UDF filesystem driver's private data // typedef struct {
> >> - UDF_LOGICAL_VOLUME_DESCRIPTOR **LogicalVolDescs;
> >> - UINTN LogicalVolDescsNo;
> >> - UDF_PARTITION_DESCRIPTOR **PartitionDescs;
> >> - UINTN PartitionDescsNo;
> >> - UDF_FILE_SET_DESCRIPTOR **FileSetDescs;
> >> - UINTN FileSetDescsNo;
> >> + UINT64 MainVdsStartLocation;
> >> + UDF_LOGICAL_VOLUME_DESCRIPTOR LogicalVolDesc;
> >> + UDF_PARTITION_DESCRIPTOR PartitionDesc;
> >> + UDF_FILE_SET_DESCRIPTOR FileSetDesc;
> >> UINTN FileEntrySize;
> >> } UDF_VOLUME_INFO;
> >>
> >> @@ -883,17 +814,6 @@ ResolveSymlink (
> >> );
> >>
> >> /**
> >> - Clean up in-memory UDF volume information.
> >> -
> >> - @param[in] Volume Volume information pointer.
> >> -
> >> -**/
> >> -VOID
> >> -CleanupVolumeInformation (
> >> - IN UDF_VOLUME_INFO *Volume
> >> - );
> >> -
> >> -/**
> >> Clean up in-memory UDF file information.
> >>
> >> @param[in] File File information pointer.
> >> --
> >> 2.11.0
> >
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-18 16:38 ` Paulo Alcantara
@ 2017-09-20 8:14 ` Ni, Ruiyu
2017-09-20 9:59 ` Laszlo Ersek
0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-20 8:14 UTC (permalink / raw)
To: Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star, Laszlo Ersek
Thanks/Ray
> -----Original Message-----
> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> Sent: Tuesday, September 19, 2017 12:38 AM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>;
> Laszlo Ersek <lersek@redhat.com>
> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> UDF logical partition
>
> Hi,
>
> On 9/17/2017 9:54 PM, Ni, Ruiyu wrote:
> > Paulo,
> > Several comments:
> > 1. Can below logic be removed in PartitionDxe/Udf.c?
> > while (!IsDevicePathEnd (DevicePathNode)) {
> > //
> > // Do not allow checking for UDF file systems in CDROM "El Torito"
> > // partitions, and skip duplicate installation of UDF file system child
> > // nodes.
> > //
> > if (DevicePathType (DevicePathNode) == MEDIA_DEVICE_PATH) {
> > if (DevicePathSubType (DevicePathNode) == MEDIA_CDROM_DP) {
> > return EFI_NOT_FOUND;
> > }
> > if (DevicePathSubType (DevicePathNode) == MEDIA_VENDOR_DP) {
> > VendorDefinedGuid = (EFI_GUID *)((UINTN)DevicePathNode +
> > OFFSET_OF (VENDOR_DEVICE_PATH, Guid));
> > if (CompareGuid (VendorDefinedGuid, &UdfDevPathGuid)) {
> > return EFI_NOT_FOUND;
> > }
> > }
> > }
> > //
> > // Try next device path node
> > //
> > DevicePathNode = NextDevicePathNode (DevicePathNode);
> > }
>
> I think it's no longer necessary, so I'll remove it.
>
> >
> > 2. Can you add function header comments for the newly added functions?
>
> Sure.
When you add the header comments, please run
Python BaseTools/Source/Python/Ecc/Ecc.py -t MdeModulePkg/Universal/Disk/PartitionDxe -s
To make sure there is no other coding style issue.
>
> >
> > 3. Change the consuming code of IS_UDF_XXX macro after patch #1 is
> changed.
>
> ACK.
I think you may also need to declare the Buffer as UDF_DESCRIPTOR_TAG.
>
> >
> > 4. Perhaps to add more checks for these numbers read from the UDF
> CDROM.
> > As Jiewen mentioned in another mail, secure code needs to validate all
> external inputs.
> > We shouldn't assume the UDF CDROM is a well-formatted CDROM.
> > A hacker may create a UDF CDROM to hack the system by using mechanism
> > like buffer overflow, integer overflow, etc.
>
> OK, when we start the Main Volume Descriptor Sequence (MVDS), we rely
> on "ExtentAd->ExtentLocation" to know the LBA where to start the
> sequence, and on "ExtentAd->ExtentLength" to know many LBAs the
> sequence has. We only care about LVD, PD and TD descriptors. We currently
> check whether a TD descriptor exists, or the search exceed ExtentAd-
> >ExtentLength blocks to determine when stopping the sequence. That
> doesn't seem to be safe enough, but here are some thoughts:
>
> a. "ExtentAd->ExtentLength" may point to a bad value. If it's greater than or
> equal to BlockIo->Media->LastBlock, and there is no TD descriptor, we'll have
> to walk the entire disk and there will too much I/O overhead and then
> impacting bootup. Obviously, when it's greater than LastBlock, DiksIo-
> >ReadDisk() should not allow to read a non-existent LBA and then return
> EFI_INVALID_PARAMETER, as per UEFI spec.
>
> b. "ExtentAd->ExtentLocation" might point to a invalid LBA, although
> DiskIo->ReadDisk() would not allow it.
>
> So, it's time to look at UDF 2.60 spec again:
>
> > Volume Descriptor Sequence Extent
> >
> > Both the main and reserve volume descriptor sequence extents > shall
> each have a minimum length of 16 logical sectors. The VDS > Extent may be
> terminated by the extent length.
>
> It doesn't tell us either a limit of logical blocks, or a maximum number.
>
> Next:
>
> > 6.9.2.3 Step 3. Volume Descriptor Sequence > Read logical sectors:
> > MVDS_Location through MVDS_Location + (MVDS_Length - 1) /
> SectorSize > ...
>
> MVDS_Length == ExtentAd->ExtentLength, so still insufficient.
>
> Next:
>
> > The data located in bytes 0-3 and 5 of the Anchor Volume Descriptor
> Pointer may be > used for format verification if desired. Verifying the Tag
> Checksum in byte 4 and > Descriptor CRC in bytes 8-11 are good additional
> verifications to perform.
> > MVDS_Location and MVDS_Length are read from this structure.
>
> Seems like a great check prior to reading ExtentAd->ExtentLocation and
> ExtentAd->ExtentLength contents. Would that be sufficient to you?
OK to me.
>
> I haven't found a way to limit the number of blocks a Volume Descriptor
> Sequence might have.
>
> Thanks,
> Paulo
>
> >
> >
> > Thanks/Ray
> >
> >> -----Original Message-----
> >> From: Paulo Alcantara [mailto:pcacjr@zytor.com]
> >> Sent: Sunday, September 17, 2017 9:13 PM
> >> To: edk2-devel@lists.01.org
> >> Cc: Paulo Alcantara <pcacjr@zytor.com>; Dong, Eric
> >> <eric.dong@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Zeng, Star
> >> <star.zeng@intel.com>; Laszlo Ersek <lersek@redhat.com>
> >> Subject: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> >> UDF logical partition
> >>
> >> Do not use entire block device size for the UDF logical partition,
> >> instead reserve the appropriate space (LVD space) for it.
> >>
> >> Cc: Eric Dong <eric.dong@intel.com>
> >> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Cc: Star Zeng <star.zeng@intel.com>
> >> Cc: Laszlo Ersek <lersek@redhat.com>
> >> Reported-by: Ruiyu Ni <ruiyu.ni@intel.com>
> >> Contributed-under: TianoCore Contribution Agreement 1.1
> >> Signed-off-by: Paulo Alcantara <pcacjr@zytor.com>
> >> ---
> >> MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c | 323
> >> ++++++++++++++++++--
> >> 1 file changed, 299 insertions(+), 24 deletions(-)
> >>
> >> diff --git a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> index 7856b5dfc1..3e84882922 100644
> >> --- a/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> +++ b/MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c
> >> @@ -84,24 +84,8 @@ FindAnchorVolumeDescriptorPointer (
> >> return EFI_VOLUME_CORRUPTED;
> >> }
> >>
> >> -/**
> >> - Check if block device supports a valid UDF file system as
> >> specified by OSTA
> >> - Universal Disk Format Specification 2.60.
> >> -
> >> - @param[in] BlockIo BlockIo interface.
> >> - @param[in] DiskIo DiskIo interface.
> >> -
> >> - @retval EFI_SUCCESS UDF file system found.
> >> - @retval EFI_UNSUPPORTED UDF file system not found.
> >> - @retval EFI_NO_MEDIA The device has no media.
> >> - @retval EFI_DEVICE_ERROR The device reported an error.
> >> - @retval EFI_VOLUME_CORRUPTED The file system structures are
> >> corrupted.
> >> - @retval EFI_OUT_OF_RESOURCES The scan was not successful due to
> >> lack of
> >> - resources.
> >> -
> >> -**/
> >> EFI_STATUS
> >> -SupportUdfFileSystem (
> >> +FindUdfVolumeIdentifiers (
> >> IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> >> IN EFI_DISK_IO_PROTOCOL *DiskIo
> >> )
> >> @@ -111,7 +95,6 @@ SupportUdfFileSystem (
> >> UINT64 EndDiskOffset;
> >> CDROM_VOLUME_DESCRIPTOR VolDescriptor;
> >> CDROM_VOLUME_DESCRIPTOR TerminatingVolDescriptor;
> >> - UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
> >>
> >> ZeroMem ((VOID *)&TerminatingVolDescriptor, sizeof
> >> (CDROM_VOLUME_DESCRIPTOR));
> >>
> >> @@ -207,12 +190,302 @@ SupportUdfFileSystem (
> >> return EFI_UNSUPPORTED;
> >> }
> >>
> >> + return EFI_SUCCESS;
> >> +}
> >> +
> >> +EFI_STATUS
> >> +GetPartitionNumber (
> >> + IN UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc,
> >> + OUT UINT16 *PartitionNum
> >> + )
> >> +{
> >> + EFI_STATUS Status;
> >> + UDF_LONG_ALLOCATION_DESCRIPTOR *LongAd;
> >> +
> >> + Status = EFI_SUCCESS;
> >> +
> >> + switch (LV_UDF_REVISION (LogicalVolDesc)) { case 0x0102:
> >> + //
> >> + // UDF 1.20 only supports Type 1 Partition
> >> + //
> >> + *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >>> PartitionMaps[4]);
> >> + break;
> >> + case 0x0150:
> >> + //
> >> + // Ensure Type 1 Partition map
> >> + //
> >> + ASSERT (LogicalVolDesc->PartitionMaps[0] == 1 &&
> >> + LogicalVolDesc->PartitionMaps[1] == 6);
> >> + *PartitionNum = *(UINT16 *)((UINTN)&LogicalVolDesc-
> >>> PartitionMaps[4]);
> >> + break;
> >> + case 0x0260:
> >> + LongAd = &LogicalVolDesc->LogicalVolumeContentsUse;
> >> + *PartitionNum = LongAd->ExtentLocation.PartitionReferenceNumber;
> >> + break;
> >> + default:
> >> + //
> >> + // Unhandled UDF revision
> >> + //
> >> + Status = EFI_VOLUME_CORRUPTED;
> >> + break;
> >> + }
> >> +
> >> + return Status;
> >> +}
> >> +
> >> +EFI_STATUS
> >> +FindLogicalVolumeLocation (
> >> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> >> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> >> + IN UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER *AnchorPoint,
> >> + OUT UINT64 *MainVdsStartLsn,
> >> + OUT UINT64 *LogicalVolEndLsn
> >> + )
> >> +{
> >> + EFI_STATUS Status;
> >> + UINT32 BlockSize;
> >> + EFI_LBA LastBlock;
> >> + UDF_EXTENT_AD *ExtentAd;
> >> + UINT64 StartingLsn;
> >> + UINT64 EndingLsn;
> >> + VOID *Buffer;
> >> + UDF_LOGICAL_VOLUME_DESCRIPTOR *LogicalVolDesc;
> >> + UDF_PARTITION_DESCRIPTOR *PartitionDesc;
> >> + UINT64 GuardMainVdsStartLsn;
> >> + UINT16 PartitionNum;
> >> +
> >> + BlockSize = BlockIo->Media->BlockSize;
> >> + LastBlock = BlockIo->Media->LastBlock;
> >> + ExtentAd = &AnchorPoint-
> >MainVolumeDescriptorSequenceExtent;
> >> + StartingLsn = (UINT64)ExtentAd->ExtentLocation;
> >> + EndingLsn =
> >> + StartingLsn + DivU64x32 ((UINT64)ExtentAd->ExtentLength,
> >> + BlockSize);
> >> +
> >> + LogicalVolDesc = NULL;
> >> + PartitionDesc = NULL;
> >> + GuardMainVdsStartLsn = StartingLsn;
> >> +
> >> + //
> >> + // Allocate buffer for reading disk blocks // Buffer =
> >> + AllocateZeroPool (BlockSize); if (Buffer == NULL) {
> >> + return EFI_OUT_OF_RESOURCES;
> >> + }
> >> +
> >> + Status = EFI_VOLUME_CORRUPTED;
> >> +
> >> + //
> >> + // As per UDF 2.60 specification:
> >> + //
> >> + // There shall be exactly one prevailing Logical Volume Descriptor
> >> + // recorded per Volume Set.
> >> + //
> >> + // Start Main Volume Descriptor Sequence and find Logical Volume
> >> + Descriptor // while (StartingLsn <= EndingLsn) {
> >> + //
> >> + // Read disk block
> >> + //
> >> + Status = DiskIo->ReadDisk (
> >> + DiskIo,
> >> + BlockIo->Media->MediaId,
> >> + MultU64x32 (StartingLsn, BlockSize),
> >> + BlockSize,
> >> + Buffer
> >> + );
> >> + if (EFI_ERROR (Status)) {
> >> + goto Out_Free;
> >> + }
> >> +
> >> + //
> >> + // Check if read block is a Terminating Descriptor
> >> + //
> >> + if (IS_TD (Buffer)) {
> >> + //
> >> + // Stop Main Volume Descriptor Sequence
> >> + //
> >> + break;
> >> + }
> >> +
> >> + //
> >> + // Check if read block is a Logical Volume Descriptor
> >> + //
> >> + if (IS_LVD (Buffer)) {
> >> + //
> >> + // Ensure only one LVD (Logical Volume Descriptor) is handled
> >> + //
> >> + if (LogicalVolDesc != NULL) {
> >> + Status = EFI_UNSUPPORTED;
> >> + goto Out_Free;
> >> + }
> >> +
> >> + //
> >> + // As per UDF 2.60 specification:
> >> + //
> >> + // For the purpose of interchange, Partition Maps shall be limited to
> >> + // Partition Map type 1, except type 2 maps.
> >> + //
> >> + // NOTE: Type 1 Partitions are the only supported in this
> implementation.
> >> + //
> >> + LogicalVolDesc = AllocateZeroPool (sizeof (*LogicalVolDesc));
> >> + if (LogicalVolDesc == NULL) {
> >> + Status = EFI_OUT_OF_RESOURCES;
> >> + goto Out_Free;
> >> + }
> >> +
> >> + //
> >> + // Save Logical Volume Descriptor
> >> + //
> >> + CopyMem (LogicalVolDesc, Buffer, sizeof (*LogicalVolDesc));
> >> + } else if (IS_PD (Buffer)) {
> >> + //
> >> + // Ensure only one PD (Partition Descriptor) is handled
> >> + //
> >> + if (PartitionDesc != NULL) {
> >> + Status = EFI_UNSUPPORTED;
> >> + goto Out_Free;
> >> + }
> >> +
> >> + //
> >> + // Found a Partition Descriptor.
> >> + //
> >> + // As per UDF 2.60 specification:
> >> + //
> >> + // A Partition Descriptor Access Type of read-only, rewritable,
> >> + // overwritable, write-once and pseudo-overwritable shall be
> >> + // supported. There shall be exactly one prevailing Partition
> >> + // Descriptor recorded per volume, with one exception. For Volume
> >> + // Sets that consist of single volume, the volume may contain 2 non-
> >> + // overlapping Partitions with 2 prevailing Partition Descriptors only
> >> + // if one has an Access Type of read-only and the other has an
> >> + // Access Type of rewritable, overwritable, or write-once. The
> >> + // Logical Volume for this volume would consist of the contents of
> >> + // both partitions.
> >> + //
> >> + PartitionDesc = AllocateZeroPool (sizeof (*PartitionDesc));
> >> + if (PartitionDesc == NULL) {
> >> + Status = EFI_OUT_OF_RESOURCES;
> >> + goto Out_Free;
> >> + }
> >> +
> >> + //
> >> + // Save Partition Descriptor
> >> + //
> >> + CopyMem (PartitionDesc, Buffer, sizeof (*PartitionDesc));
> >> + }
> >> + //
> >> + // Go to next disk block
> >> + //
> >> + StartingLsn++;
> >> + }
> >> +
> >> + Status = EFI_VOLUME_CORRUPTED;
> >> +
> >> + //
> >> + // Check if LVD and PD were found
> >> + //
> >> + if (LogicalVolDesc != NULL && PartitionDesc != NULL) {
> >> + //
> >> + // Get partition number from Partition map in LVD descriptor
> >> + //
> >> + Status = GetPartitionNumber (LogicalVolDesc, &PartitionNum);
> >> + if (EFI_ERROR (Status)) {
> >> + goto Out_Free;
> >> + }
> >> +
> >> + //
> >> + // Make sure we're handling expected Partition Descriptor
> >> + //
> >> + if (PartitionDesc->PartitionNumber != PartitionNum) {
> >> + Status = EFI_VOLUME_CORRUPTED;
> >> + goto Out_Free;
> >> + }
> >> +
> >> + //
> >> + // Cover the main VDS area so UdfDxe driver will also be able to
> >> + get LVD
> >> and
> >> + // PD descriptors out from the file system.
> >> + //
> >> + *MainVdsStartLsn = GuardMainVdsStartLsn;
> >> + *LogicalVolEndLsn = *MainVdsStartLsn +
> >> + (UINT64)ExtentAd->ExtentLength;
> >> +
> >> + //
> >> + // Cover UDF partition area
> >> + //
> >> + *LogicalVolEndLsn +=
> >> + ((UINT64)PartitionDesc->PartitionStartingLocation -
> >> + *LogicalVolEndLsn) + PartitionDesc->PartitionLength - 1;
> >> + //
> >> + // Ensure to not attempt reading past end of device
> >> + //
> >> + if (*LogicalVolEndLsn > LastBlock) {
> >> + Status = EFI_VOLUME_CORRUPTED;
> >> + } else {
> >> + Status = EFI_SUCCESS;
> >> + }
> >> + }
> >> +
> >> +Out_Free:
> >> + //
> >> + // Free block read buffer
> >> + //
> >> + FreePool (Buffer);
> >> + //
> >> + // Free Logical Volume Descriptor
> >> + //
> >> + if (LogicalVolDesc != NULL) {
> >> + FreePool (LogicalVolDesc);
> >> + }
> >> + //
> >> + // Free Partition Descriptor
> >> + //
> >> + if (PartitionDesc != NULL) {
> >> + FreePool (PartitionDesc);
> >> + }
> >> +
> >> + return Status;
> >> +}
> >> +
> >> +EFI_STATUS
> >> +FindUdfLogicalVolume (
> >> + IN EFI_BLOCK_IO_PROTOCOL *BlockIo,
> >> + IN EFI_DISK_IO_PROTOCOL *DiskIo,
> >> + OUT EFI_LBA *StartingLBA,
> >> + OUT EFI_LBA *EndingLBA
> >> + )
> >> +{
> >> + EFI_STATUS Status;
> >> + UDF_ANCHOR_VOLUME_DESCRIPTOR_POINTER AnchorPoint;
> >> +
> >> + //
> >> + // Find UDF volume identifiers
> >> + //
> >> + Status = FindUdfVolumeIdentifiers (BlockIo, DiskIo); if
> >> + (EFI_ERROR
> >> + (Status)) {
> >> + return Status;
> >> + }
> >> +
> >> + //
> >> + // Find Anchor Volume Descriptor Pointer //
> >> Status = FindAnchorVolumeDescriptorPointer (BlockIo, DiskIo,
> >> &AnchorPoint);
> >> if (EFI_ERROR (Status)) {
> >> - return EFI_UNSUPPORTED;
> >> + return Status;
> >> }
> >>
> >> - return EFI_SUCCESS;
> >> + //
> >> + // Find Logical Volume location
> >> + //
> >> + Status = FindLogicalVolumeLocation (
> >> + BlockIo,
> >> + DiskIo,
> >> + &AnchorPoint,
> >> + (UINT64 *)StartingLBA,
> >> + (UINT64 *)EndingLBA
> >> + );
> >> +
> >> + return Status;
> >> }
> >>
> >> /**
> >> @@ -250,6 +523,8 @@ PartitionInstallUdfChildHandles (
> >> EFI_GUID *VendorDefinedGuid;
> >> EFI_GUID UdfDevPathGuid = EFI_UDF_DEVICE_PATH_GUID;
> >> EFI_PARTITION_INFO_PROTOCOL PartitionInfo;
> >> + EFI_LBA StartingLBA;
> >> + EFI_LBA EndingLBA;
> >>
> >> Media = BlockIo->Media;
> >>
> >> @@ -291,9 +566,9 @@ PartitionInstallUdfChildHandles (
> >> }
> >>
> >> //
> >> - // Check if block device supports an UDF file system
> >> + // Find UDF logical volume on block device
> >> //
> >> - Status = SupportUdfFileSystem (BlockIo, DiskIo);
> >> + Status = FindUdfLogicalVolume (BlockIo, DiskIo, &StartingLBA,
> >> + &EndingLBA);
> >> if (EFI_ERROR (Status)) {
> >> return EFI_NOT_FOUND;
> >> }
> >> @@ -318,8 +593,8 @@ PartitionInstallUdfChildHandles (
> >> DevicePath,
> >> (EFI_DEVICE_PATH_PROTOCOL *)&gUdfDevicePath,
> >> &PartitionInfo,
> >> - 0,
> >> - Media->LastBlock,
> >> + StartingLBA,
> >> + EndingLBA,
> >> Media->BlockSize
> >> );
> >> if (!EFI_ERROR (Status)) {
> >> --
> >> 2.11.0
> >
> >
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-20 8:14 ` Ni, Ruiyu
@ 2017-09-20 9:59 ` Laszlo Ersek
2017-09-20 10:11 ` Ni, Ruiyu
0 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2017-09-20 9:59 UTC (permalink / raw)
To: Ni, Ruiyu, Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star
Ray,
On 09/20/17 10:14, Ni, Ruiyu wrote:
> When you add the header comments, please run
> Python BaseTools/Source/Python/Ecc/Ecc.py -t MdeModulePkg/Universal/Disk/PartitionDxe -s
> To make sure there is no other coding style issue.
side question: do you mean "PatchCheck.py"?
I've never heard of "Ecc.py" before, what does it do? (The top comment
only says "This file is used to be the main entrance of ECC tool", and
the "BaseTools/UserManuals" directory doesn't seem to contain anything
related.)
Thanks,
Laslzo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-20 9:59 ` Laszlo Ersek
@ 2017-09-20 10:11 ` Ni, Ruiyu
2017-09-20 11:09 ` Laszlo Ersek
0 siblings, 1 reply; 20+ messages in thread
From: Ni, Ruiyu @ 2017-09-20 10:11 UTC (permalink / raw)
To: Laszlo Ersek, Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star
I am surprised that you don't know ECC tool. 😊
It's a much more complex tool than PatchCheck.py.
It's to report all coding style issues that doesn't follow EDKII C coding style.
Thanks/Ray
> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Wednesday, September 20, 2017 5:59 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>;
> edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> UDF logical partition
>
> Ray,
>
> On 09/20/17 10:14, Ni, Ruiyu wrote:
>
> > When you add the header comments, please run Python
> > BaseTools/Source/Python/Ecc/Ecc.py -t
> > MdeModulePkg/Universal/Disk/PartitionDxe -s To make sure there is no
> other coding style issue.
>
> side question: do you mean "PatchCheck.py"?
>
> I've never heard of "Ecc.py" before, what does it do? (The top comment only
> says "This file is used to be the main entrance of ECC tool", and the
> "BaseTools/UserManuals" directory doesn't seem to contain anything
> related.)
>
> Thanks,
> Laslzo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-20 10:11 ` Ni, Ruiyu
@ 2017-09-20 11:09 ` Laszlo Ersek
2017-09-20 12:36 ` Gao, Liming
0 siblings, 1 reply; 20+ messages in thread
From: Laszlo Ersek @ 2017-09-20 11:09 UTC (permalink / raw)
To: Ni, Ruiyu, Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star
On 09/20/17 12:11, Ni, Ruiyu wrote:
> I am surprised that you don't know ECC tool. 😊
> It's a much more complex tool than PatchCheck.py.
> It's to report all coding style issues that doesn't follow EDKII C coding style.
Well, before my previous email, I tried to execute the command line you named, but it didn't work:
$ source edksetup.sh
$ Ecc -t MdeModulePkg/Universal/Disk/PartitionDxe -s
Traceback (most recent call last):
File "BaseTools/BinWrappers/PosixLike/../../Source/Python/Ecc/Ecc.py", line 24, in <module>
from Check import Check
File "BaseTools/Source/Python/Ecc/Check.py", line 20, in <module>
import c
File "BaseTools/Source/Python/Ecc/c.py", line 18, in <module>
import CodeFragmentCollector
File "BaseTools/Source/Python/Ecc/CodeFragmentCollector.py", line 23, in <module>
import antlr3
ImportError: No module named antlr3
Now I'v checked both Fedora and RHEL7 packages for an "antlr3" python module, but it doesn't exist. The "antlr3" package itself exists in both distros, but the changelogs say,
> * Thu Feb 23 2012 Miloš Jakubíček <xjakub@fi.muni.cz>
> - 3.4-5 - Disable python runtime (incompatible with current antlr version)
The following RHBZ looks relevant: <https://bugzilla.redhat.com/show_bug.cgi?id=1313024>
Either way, I don't think I can use the ECC tool.
Thanks,
Laszlo
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Wednesday, September 20, 2017 5:59 PM
>> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>;
>> edk2-devel@lists.01.org
>> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
>> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
>> UDF logical partition
>>
>> Ray,
>>
>> On 09/20/17 10:14, Ni, Ruiyu wrote:
>>
>>> When you add the header comments, please run Python
>>> BaseTools/Source/Python/Ecc/Ecc.py -t
>>> MdeModulePkg/Universal/Disk/PartitionDxe -s To make sure there is no
>> other coding style issue.
>>
>> side question: do you mean "PatchCheck.py"?
>>
>> I've never heard of "Ecc.py" before, what does it do? (The top comment only
>> says "This file is used to be the main entrance of ECC tool", and the
>> "BaseTools/UserManuals" directory doesn't seem to contain anything
>> related.)
>>
>> Thanks,
>> Laslzo
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
2017-09-20 11:09 ` Laszlo Ersek
@ 2017-09-20 12:36 ` Gao, Liming
0 siblings, 0 replies; 20+ messages in thread
From: Gao, Liming @ 2017-09-20 12:36 UTC (permalink / raw)
To: Laszlo Ersek, Ni, Ruiyu, Paulo Alcantara, edk2-devel@lists.01.org
Cc: Dong, Eric, Zeng, Star
Laszlo:
ECC depends on antlr V3.0.1. It can be downloaded from http://www.antlr3.org/download/Python/
After download it, use python setup.py install to install it. I try this way on Windows OS, it can work.
Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Wednesday, September 20, 2017 7:10 PM
> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>; edk2-devel@lists.01.org
> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition
>
> On 09/20/17 12:11, Ni, Ruiyu wrote:
> > I am surprised that you don't know ECC tool. 😊
> > It's a much more complex tool than PatchCheck.py.
> > It's to report all coding style issues that doesn't follow EDKII C coding style.
>
> Well, before my previous email, I tried to execute the command line you named, but it didn't work:
>
> $ source edksetup.sh
>
> $ Ecc -t MdeModulePkg/Universal/Disk/PartitionDxe -s
> Traceback (most recent call last):
> File "BaseTools/BinWrappers/PosixLike/../../Source/Python/Ecc/Ecc.py", line 24, in <module>
> from Check import Check
> File "BaseTools/Source/Python/Ecc/Check.py", line 20, in <module>
> import c
> File "BaseTools/Source/Python/Ecc/c.py", line 18, in <module>
> import CodeFragmentCollector
> File "BaseTools/Source/Python/Ecc/CodeFragmentCollector.py", line 23, in <module>
> import antlr3
> ImportError: No module named antlr3
>
> Now I'v checked both Fedora and RHEL7 packages for an "antlr3" python module, but it doesn't exist. The "antlr3" package itself exists
> in both distros, but the changelogs say,
>
> > * Thu Feb 23 2012 Miloš Jakubíček <xjakub@fi.muni.cz>
> > - 3.4-5 - Disable python runtime (incompatible with current antlr version)
>
> The following RHBZ looks relevant: <https://bugzilla.redhat.com/show_bug.cgi?id=1313024>
>
> Either way, I don't think I can use the ECC tool.
>
> Thanks,
> Laszlo
>
>
> >> -----Original Message-----
> >> From: Laszlo Ersek [mailto:lersek@redhat.com]
> >> Sent: Wednesday, September 20, 2017 5:59 PM
> >> To: Ni, Ruiyu <ruiyu.ni@intel.com>; Paulo Alcantara <pcacjr@zytor.com>;
> >> edk2-devel@lists.01.org
> >> Cc: Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> >> Subject: Re: [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of
> >> UDF logical partition
> >>
> >> Ray,
> >>
> >> On 09/20/17 10:14, Ni, Ruiyu wrote:
> >>
> >>> When you add the header comments, please run Python
> >>> BaseTools/Source/Python/Ecc/Ecc.py -t
> >>> MdeModulePkg/Universal/Disk/PartitionDxe -s To make sure there is no
> >> other coding style issue.
> >>
> >> side question: do you mean "PatchCheck.py"?
> >>
> >> I've never heard of "Ecc.py" before, what does it do? (The top comment only
> >> says "This file is used to be the main entrance of ECC tool", and the
> >> "BaseTools/UserManuals" directory doesn't seem to contain anything
> >> related.)
> >>
> >> Thanks,
> >> Laslzo
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-09-20 12:33 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-17 13:13 [PATCH v2 0/3] UDF partition driver fix Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 1/3] MdePkg: Add UDF volume structure definitions Paulo Alcantara
2017-09-18 0:28 ` Ni, Ruiyu
2017-09-18 0:42 ` Ni, Ruiyu
2017-09-18 13:52 ` Paulo Alcantara
2017-09-18 13:50 ` Paulo Alcantara
2017-09-17 13:13 ` [PATCH v2 2/3] MdeModulePkg/PartitionDxe: Fix creation of UDF logical partition Paulo Alcantara
2017-09-18 0:54 ` Ni, Ruiyu
2017-09-18 16:38 ` Paulo Alcantara
2017-09-20 8:14 ` Ni, Ruiyu
2017-09-20 9:59 ` Laszlo Ersek
2017-09-20 10:11 ` Ni, Ruiyu
2017-09-20 11:09 ` Laszlo Ersek
2017-09-20 12:36 ` Gao, Liming
2017-09-17 13:13 ` [PATCH v2 3/3] MdeModulePkg/UdfDxe: Rework driver to support PartitionDxe changes Paulo Alcantara
2017-09-18 1:00 ` Ni, Ruiyu
2017-09-18 16:51 ` Paulo Alcantara
2017-09-20 7:52 ` Ni, Ruiyu
2017-09-18 4:52 ` [PATCH v2 0/3] UDF partition driver fix Ni, Ruiyu
2017-09-18 17:49 ` Paulo Alcantara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox