Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-12 Thread Laszlo Ersek 
On 09/12/17 17:38, Ard Biesheuvel wrote:
> On 12 September 2017 at 03:14, Laszlo Ersek  wrote:
>> On 09/10/17 02:12, Laszlo Ersek wrote:
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: udf_fixes_cleanups
>>>
>>> Patches #2, #3 and #4 are needed (and enough) for me to build OVMF for
>>> IA32 and X64 with clang-3.8, after the UDF introduction.
>>>
>>> Patches #1 and #5 are cleanups that I felt fit before patch #2 and after
>>> patch #4, respectively.
>>>
>>> Cc: Ard Biesheuvel 
>>> Cc: Eric Dong 
>>> Cc: Paulo Alcantara 
>>> Cc: Ruiyu Ni 
>>> Cc: Star Zeng 
>>>
>>> Thanks
>>> Laszlo
>>>
>>> Laszlo Ersek (5):
>>>   MdeModulePkg/UdfDxe: ASSERT() valid ReadFileInfo Flags for INLINE_DATA
>>> req
>>>   MdeModulePkg/UdfDxe: don't return unset Status if INLINE_DATA req
>>> succeeds
>>>   MdeModulePkg/UdfDxe: replace zero-init of local variables with
>>> ZeroMem()
>>>   MdeModulePkg/PartitionDxe: don't divide 64-bit values with C operators
>>>   MdeModulePkg/PartitionDxe: remove always false comparison
>>>
>>>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 9 +++--
>>>  MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 --
>>>  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 5 +
>>>  3 files changed, 16 insertions(+), 4 deletions(-)
>>>
>>
>> Thanks all for the feedback, pushed as commit range
>> c05cae55ebd8..b4e5807d2492.
>>
>> (I didn't change the sizeof / sizeof() stuff -- for one, I didn't want
>> to touch the code on such an urgent push, relative to the posted and
>> tested version.)
>>
> 
> Rather unexpectedly, the build is still broken on my CI system
> 
> This time, it is GCC 4.8 that complains about uninitialized variables,
> most likely false positives again (apologies for the letter soup)
> 
> :
> In function 'ReadFile':
> :876:27:
> error: 'Status' may be used uninitialized in this function
> [-Werror=maybe-uninitialized]
>EFI_STATUS  Status;
>^

This is actually a bug in the code.

In order to explain that, I have to elaborate.

The UDF standard from the OSTA is introduced like this:

The OSTA Universal Disk Format (UDF ® ) specification defines a
subset of the standard ECMA 167 3 rd edition. The primary goal of
the OSTA UDF is to maximize data interchange and minimize the cost
and complexity of implementing ECMA 167.

Which (in retrospect...) means that we have to download ECMA 167 at once:

https://www.ecma-international.org/publications/standards/Ecma-167.htm

OK, so let's look at the code.

First, the ReadFile() function is very hard to read. It is long (~300
lines), but what is actually problematic about it is that it uses "goto"
statements for control flow that is *not* related to error handling.
This makes it painful to track, and it happens to violate the edk2
coding style spec as well:

https://edk2-docs.gitbooks.io/edk-ii-c-coding-standards-specification/content/5_source_files/57_c_programming.html#57-c-programming

5.7.3.8 Goto Statements should not be used (in general)

In almost all cases, it is possible to write the code so that a goto
is not needed. If a goto is used, be ready to defend it during
review.

It is common to use goto for error handling and thus, exiting a
routine in an error case is the only legal use of a goto. A goto
allows the error exit code to be contained in one place in the
routine. This one place reduces software life cycle maintenance
issues, as there can be one copy of error cleanup code per routine.

   [...]

Second, in this case it actually pays off to read the function top-down.

- The first switch statement (on "ReadFileInfo->Flags") does not set
Status, but it also doesn't read it or jump to a read site.

- The second switch statement has four case labels (RecordingFlags):

  - INLINE_DATA: it sets Status to EFI_SUCCESS
(thanks to my recent patch)

  - LONG_ADS_SEQUENCE, SHORT_ADS_SEQUENCE: Status is set very
soon, when we unconditionally call GetAllocationDescriptor().

  - EXTENDED_ADS_SEQUENCE: we ASSERT(FALSE), but also set Status to
EFI_UNSUPPORTED

Then, assuming we didn't jump to any of the error labels
(Error_Read_Disk_Blk, Error_Alloc_Buffer_To_Next_Ad, Error_Get_Aed), we
proceed to the Done label, and return Status.

(BTW, I checked all the paths to those error labels, and those *are* fine.)

So how is it possible that we reach Done, and return Status, without
having set Status? It is possible by taking *none* of the branches in
the second switch statement (RecordingFlags). The switch statement does

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-12 Thread Ard Biesheuvel 
On 12 September 2017 at 03:14, Laszlo Ersek  wrote:
> On 09/10/17 02:12, Laszlo Ersek wrote:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: udf_fixes_cleanups
>>
>> Patches #2, #3 and #4 are needed (and enough) for me to build OVMF for
>> IA32 and X64 with clang-3.8, after the UDF introduction.
>>
>> Patches #1 and #5 are cleanups that I felt fit before patch #2 and after
>> patch #4, respectively.
>>
>> Cc: Ard Biesheuvel 
>> Cc: Eric Dong 
>> Cc: Paulo Alcantara 
>> Cc: Ruiyu Ni 
>> Cc: Star Zeng 
>>
>> Thanks
>> Laszlo
>>
>> Laszlo Ersek (5):
>>   MdeModulePkg/UdfDxe: ASSERT() valid ReadFileInfo Flags for INLINE_DATA
>> req
>>   MdeModulePkg/UdfDxe: don't return unset Status if INLINE_DATA req
>> succeeds
>>   MdeModulePkg/UdfDxe: replace zero-init of local variables with
>> ZeroMem()
>>   MdeModulePkg/PartitionDxe: don't divide 64-bit values with C operators
>>   MdeModulePkg/PartitionDxe: remove always false comparison
>>
>>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 9 +++--
>>  MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 --
>>  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 5 +
>>  3 files changed, 16 insertions(+), 4 deletions(-)
>>
>
> Thanks all for the feedback, pushed as commit range
> c05cae55ebd8..b4e5807d2492.
>
> (I didn't change the sizeof / sizeof() stuff -- for one, I didn't want
> to touch the code on such an urgent push, relative to the posted and
> tested version.)
>

Rather unexpectedly, the build is still broken on my CI system

This time, it is GCC 4.8 that complains about uninitialized variables,
most likely false positives again (apologies for the letter soup)

:
In function 'ReadFile':
:876:27:
error: 'Status' may be used uninitialized in this function
[-Werror=maybe-uninitialized]
   EFI_STATUS  Status;
   ^
"/home/buildslave/srv/toolchain/arm-tc-14.04/bin/arm-linux-gnueabihf-gcc"
-mthumb -mcpu=cortex-a15
-I
-g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror
-Wno-array-bounds -include AutoGen.h -fno-common -mlittle-endian
-mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections
-fdata-sections -fomit-frame-pointer -Wno-address -mthumb
-mfloat-abi=soft -fno-pic -fno-pie -fstack-protector
-mword-relocations -Wno-unused-but-set-variable -DMDEPKG_NDEBUG
-DDISABLE_NEW_DEPRECATED_INTERFACES -c -o

-I
-I
-I
-I
-I
-I
-I

:887:27:
error: 'BytesLeft' may be used uninitialized in this function
[-Werror=maybe-uninitialized]
   UINT64  BytesLeft;
   ^
cc1: all warnings being treated as errors
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Shi, Steven 
Hi Paulo,
Your fix works for me. The "Null Pointer Use" issue disappears after apply your 
new assignment patch. Thanks!


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522


> -Original Message-
> From: Paulo Alcantara [mailto:pca...@zytor.com]
> Sent: Monday, September 11, 2017 9:52 PM
> To: Shi, Steven <steven@intel.com>; Laszlo Ersek <ler...@redhat.com>;
> edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng,
> Star <star.z...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>
> Subject: Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups
> 
> Hi Steven,
> 
> On 11/09/2017 10:07, Shi, Steven wrote:
> > Hi Laszlo,
> > Your code is good. But there is a "Null Pointer Use" issue, which is a
> undefined behavior in C spec, in
> edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, line:696, column: 7 and
> line:707, column: 14.
> >
> > Line695:
> >FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
> >if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
> > ...
> >
> > The above FileIdentifierDesc can be NULL if you run a .efi in a Udf 
> > partition.
> And the later IS_FID_DIRECTORY_FILE (FileIdentifierDesc) is unsafe if
> FileIdentifierDesc == NULL. You can test it with below extra code which add
> debug output when FileIdentifierDesc == NULL, and check the log after load
> and run something in a Udf partition.
> >
> > +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> > @@ -693,6 +693,11 @@ UdfSetPosition (
> > PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
> >
> > FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
> > +  if (FileIdentifierDesc == NULL) {
> > +DEBUG ((EFI_D_ERROR, "FileIdentifierDesc == NULL!\n"));
> > +return Status;
> > +  }
> > +
> > if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
> >   //
> >   // If the file handle is a directory, the _only_ position that may be 
> > set is
> >
> > I followed the Paulo's suggestion, and found this issue when using the Udf
> partition of Windows 10 Enterprise ISO image on qemu. Below is my qemu
> command:
> > /usr/local/bin/qemu-system-x86_64 -m 5120 -enable-kvm  -machine pc-
> q35-2.9 -bios OVMF.fd -serial file:serial.log  -cdrom /media/jshi19/My\
> Passport/OSImage/ISO/winserver/en_windows_server_2016_x64_dvd_9327
> 751.iso
> >
> > I've submitted a bug for this issue:
> https://bugzilla.tianocore.org/show_bug.cgi?id=704.
> >
> > I found this issue by edk2 llvm undefined behavior sanitizer, and below is
> the sanitizer output. FYI.
> >
> /home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c,
> line:0x02B8, column:0x0007 ErrorType = NullPointerUse: member access
> within null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
> > ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is
> called:
> > UdfSetPosition
> >
> /home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c:
> 696:7
> > EfiShellFindFilesInDir
> >
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:213
> 6:18
> > ShellSearchHandle
> >
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:241
> 3:14
> > EfiShellFindFiles
> >
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:258
> 6:18
> > EfiShellOpenFileList
> >
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:266
> 6:12
> > ShellOpenFileMetaArg
> >
> /home/jshi19/wksp_efi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c:15
> 70:14
> >
> >
> /home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c,
> line:0x02C3, column:0x000E ErrorType = NullPointerUse: member access
> within null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
> > ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is
> called:
> > ...
> 
> Thank you for catching this bug up. I think this issue is related to
> accessing a File Identifier Descriptor from a root directory file --
> e.g., this should be "FileIdentifierDesc =
> PrivFileData->Root->FileIdentifierDesc;"
> 
> Could you please replace the broken assignment with:
> 
> FileIdentifierDesc = _FILE(PrivFileData)->FileIdentifierDesc;
> ASSERT (FileIdentifierDesc != NULL);
> ...
> 
> Tell me if that worked for you. If so, I could send a formal patch
> fixing this issue later.
> 
> (BTW, I do apologize the late replies but I'm only

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Paulo Alcantara 



On 11/09/2017 03:58, Laszlo Ersek wrote:

On 09/10/17 17:51, Paulo Alcantara wrote:



On 10/09/2017 11:27, Shi, Steven wrote:

OK. Does the UDF image you created correctly show up as CD-ROM content
in Linux, e.g Fedora?


The Fedora image I have (Fedora-Workstation-Live-x86_64-26-1.5.iso) does
not contain a valid UDF file system, so you cannot use it for testing.

To make sure it really doesn't, I used a "Philips UDF Conformance Tool"
that you can grab in [1], and the tool didn't find any valid UDF file
system in it.

I've been using an unmodified Windows 10 Enterprise image that does
contain a valid UDF bridge disk image (ISO9660+ElTorito+UDF) to perform
my tests.

Additionally, I use an USB stick that I format it with 'sudo mkudffs -b
512 --media-type=hd /dev/sdX' and copy some files to it for testing

BTW, when testing with OVMF AARCH64, I was using my USB stick that
showed up correctly as 'fsX' in UEFI shell, but with the Windows 10
Enterprise ISO image, it didn't. I looked at the logs to see what was
going on and I found out that the emulated CD-ROM drive is reporting a
block size of 512 instead of 2048 -- which seems wrong to me. With
'qemu-system-x86_64 -cdrom', it reports a block size of 2048.

The Partition driver is still able to find a valid ElTorito partition
because, regardless the device block size, it always use a logical block
size of 2048 starting at 32K. In UDF, when searching for AVDPs (Anchor
Volume Descriptor Pointers), we search for them at fixed locations: 256,
N - 256, N and 512 -- but, with a block size of 512, the locations
change to 1024, N - 1024, N and 2048 -- thus breaking the volume
recognition sequence.

For testing the Windows image with a block size of 512, I used the
comformance tool with './udf_test -verbose 60 -blocksize 512
~/img/win_ent10.iso' and it failed to find a valid UDF file system. But
with a block size of 2048, it worked.

Is there any reason for reporting a block size of 512 when using
'-cdrom' option in qemu-system-aarch64? Is that a bug? Or am I missing
something here?


"-cdrom" is a legacy QEMU option, a shorthand that expands to a -drive
option, and at least one -device option (it could expand to multiple
-device options). And, this expansion can be different on different
machine types. On i440fx machine types, -cdrom can mean an IDE CD, on
q35, an AHCI CD, an aarch64/virt, a virtio-scsi-pci controller and a
scsi-cd. For this reason, it is always best to use complete -drive and
-device specifications (which is equivalent to saying it is best to
always use libvirt).

Now, the above does not imlpy that no bug can exist in this space. Can
you run the "info qtree" monitor command, in both cases, and compare the
output with regard to the CD-ROM?


OK. Thanks for clarifying that to me. I'll do it when I have a chance 
and tell you.


Paulo



Thanks
Laszlo


Thanks!
Paulo

[1] - https://www.lscdweb.com/registered/udf_verifier.html

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Paulo Alcantara 

Hi Steven,

On 11/09/2017 10:07, Shi, Steven wrote:

Hi Laszlo,
Your code is good. But there is a "Null Pointer Use" issue, which is a 
undefined behavior in C spec, in edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
line:696, column: 7 and line:707, column: 14.

Line695:
   FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
   if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
...

The above FileIdentifierDesc can be NULL if you run a .efi in a Udf partition. 
And the later IS_FID_DIRECTORY_FILE (FileIdentifierDesc) is unsafe if 
FileIdentifierDesc == NULL. You can test it with below extra code which add 
debug output when FileIdentifierDesc == NULL, and check the log after load and 
run something in a Udf partition.

+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -693,6 +693,11 @@ UdfSetPosition (
PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);

FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
+  if (FileIdentifierDesc == NULL) {
+DEBUG ((EFI_D_ERROR, "FileIdentifierDesc == NULL!\n"));
+return Status;
+  }
+
if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
  //
  // If the file handle is a directory, the _only_ position that may be set 
is

I followed the Paulo's suggestion, and found this issue when using the Udf 
partition of Windows 10 Enterprise ISO image on qemu. Below is my qemu command:
/usr/local/bin/qemu-system-x86_64 -m 5120 -enable-kvm  -machine pc-q35-2.9 
-bios OVMF.fd -serial file:serial.log  -cdrom /media/jshi19/My\ 
Passport/OSImage/ISO/winserver/en_windows_server_2016_x64_dvd_9327751.iso

I've submitted a bug for this issue: 
https://bugzilla.tianocore.org/show_bug.cgi?id=704.

I found this issue by edk2 llvm undefined behavior sanitizer, and below is the 
sanitizer output. FYI.
/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
line:0x02B8, column:0x0007 ErrorType = NullPointerUse: member access within 
null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called:
UdfSetPosition
/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c:696:7
EfiShellFindFilesInDir
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2136:18
ShellSearchHandle
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2413:14
EfiShellFindFiles
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2586:18
EfiShellOpenFileList
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2666:12
ShellOpenFileMetaArg
/home/jshi19/wksp_efi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c:1570:14

/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
line:0x02C3, column:0x000E ErrorType = NullPointerUse: member access within 
null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called:
...


Thank you for catching this bug up. I think this issue is related to 
accessing a File Identifier Descriptor from a root directory file -- 
e.g., this should be "FileIdentifierDesc = 
PrivFileData->Root->FileIdentifierDesc;"


Could you please replace the broken assignment with:

FileIdentifierDesc = _FILE(PrivFileData)->FileIdentifierDesc;
ASSERT (FileIdentifierDesc != NULL);
...

Tell me if that worked for you. If so, I could send a formal patch 
fixing this issue later.


(BTW, I do apologize the late replies but I'm only able to work on this 
in my free time)


Thanks,
Paulo



Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522


-Original Message-
From: Paulo Alcantara [mailto:pca...@zytor.com]
Sent: Sunday, September 10, 2017 11:52 PM
To: Shi, Steven <steven@intel.com>; Laszlo Ersek <ler...@redhat.com>;
edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng,
Star <star.z...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>
Subject: Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups



On 10/09/2017 11:27, Shi, Steven wrote:

OK. Does the UDF image you created correctly show up as CD-ROM

content in Linux, e.g Fedora?

The Fedora image I have (Fedora-Workstation-Live-x86_64-26-1.5.iso) does
not contain a valid UDF file system, so you cannot use it for testing.

To make sure it really doesn't, I used a "Philips UDF Conformance Tool"
that you can grab in [1], and the tool didn't find any valid UDF file
system in it.

I've been using an unmodified Windows 10 Enterprise image that does
contain a valid UDF bridge disk image (ISO9660+ElTorito+UDF) to perform
my tests.

Additionally, I use an USB stick that I format it with 'sudo mkudffs -b
512 --media-type=hd /dev/sdX' and copy some files to it for testing

BTW, when testing with OVMF AARCH64, I was using my USB stick that
showed up correctly as 'fsX' in UEFI shell, bu

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Laszlo Ersek 
On 09/11/17 15:07, Shi, Steven wrote:
> Hi Laszlo,
> Your code is good. But there is a "Null Pointer Use" issue, which is a 
> undefined behavior in C spec, in 
> edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, line:696, column: 7 and 
> line:707, column: 14.
> 
> Line695:
>   FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
>   if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
>...
> 
> The above FileIdentifierDesc can be NULL if you run a .efi in a Udf 
> partition. And the later IS_FID_DIRECTORY_FILE (FileIdentifierDesc) is unsafe 
> if FileIdentifierDesc == NULL. You can test it with below extra code which 
> add debug output when FileIdentifierDesc == NULL, and check the log after 
> load and run something in a Udf partition.
> 
> +++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
> @@ -693,6 +693,11 @@ UdfSetPosition (
>PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);
> 
>FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
> +  if (FileIdentifierDesc == NULL) {
> +DEBUG ((EFI_D_ERROR, "FileIdentifierDesc == NULL!\n"));
> +return Status;
> +  }
> +
>if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
>  //
>  // If the file handle is a directory, the _only_ position that may be 
> set is
> 
> I followed the Paulo's suggestion, and found this issue when using the Udf 
> partition of Windows 10 Enterprise ISO image on qemu. Below is my qemu 
> command:
> /usr/local/bin/qemu-system-x86_64 -m 5120 -enable-kvm  -machine pc-q35-2.9 
> -bios OVMF.fd -serial file:serial.log  -cdrom /media/jshi19/My\ 
> Passport/OSImage/ISO/winserver/en_windows_server_2016_x64_dvd_9327751.iso
> 
> I've submitted a bug for this issue: 
> https://bugzilla.tianocore.org/show_bug.cgi?id=704.

Right, I saw the report (via
<https://lists.01.org/mailman/listinfo/edk2-bugs>), thank you for filing it.

I asked Paulo a few minutes ago to register in the TianoCore Bugzilla,
and to take the BZ.

I don't intend to co-maintain UdfDxe permanently, aside from one-off
patches and reviews, exactly like with any other MdeModulePkg driver.
I'm totally unfamiliar with UdfDxe code, I'm just interested in using
the feature. The only reason I sent these five patches over the weekend
is that the initial code drop, technically committed by me, broke the
build with CLANG38, and Paulo wasn't near his computer. I wanted to fix
the build breakage ASAP.

If you find other issues, please continue filing BZs, and Paulo should
please continue helping out with them.

Thanks!
Laszlo

> 
> I found this issue by edk2 llvm undefined behavior sanitizer, and below is 
> the sanitizer output. FYI.
> /home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
> line:0x02B8, column:0x0007 ErrorType = NullPointerUse: member access within 
> null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
> ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called: 
> UdfSetPosition
> /home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c:696:7
> EfiShellFindFilesInDir
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2136:18
> ShellSearchHandle
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2413:14
> EfiShellFindFiles
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2586:18
> EfiShellOpenFileList
> /home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2666:12
> ShellOpenFileMetaArg
> /home/jshi19/wksp_efi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c:1570:14
> 
> /home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
> line:0x02C3, column:0x000E ErrorType = NullPointerUse: member access within 
> null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
> ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called:
> ...
> 
> Steven Shi
> Intel\SSG\STO\UEFI Firmware
> 
> Tel: +86 021-61166522
> iNet: 821-6522
> 
>> -Original Message-
>> From: Paulo Alcantara [mailto:pca...@zytor.com]
>> Sent: Sunday, September 10, 2017 11:52 PM
>> To: Shi, Steven <steven....@intel.com>; Laszlo Ersek <ler...@redhat.com>;
>> edk2-devel-01 <edk2-devel@lists.01.org>
>> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng,
>> Star <star.z...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>
>> Subject: Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups
>>
>>
>>
>> On 10/09/2017 11:27, Shi, Steven wrote:
>>> OK. Does the UDF image you created correctly show up as CD-ROM
>> content in Linux, e.g Fedora?
>>
>> The Fedora image I have (Fedora-Workstation-Live-x86_64-26-

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Ni, Ruiyu 
Steven,
Thanks for capturing the bug using ASAN!

-Original Message-
From: Shi, Steven 
Sent: Monday, September 11, 2017 9:08 PM
To: Paulo Alcantara <pca...@zytor.com>; Laszlo Ersek <ler...@redhat.com>; 
edk2-devel-01 <edk2-devel@lists.01.org>
Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng, 
Star <star.z...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>
Subject: RE: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

Hi Laszlo,
Your code is good. But there is a "Null Pointer Use" issue, which is a 
undefined behavior in C spec, in 
edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, line:696, column: 7 and 
line:707, column: 14.

Line695:
  FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
  if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
   ...

The above FileIdentifierDesc can be NULL if you run a .efi in a Udf partition. 
And the later IS_FID_DIRECTORY_FILE (FileIdentifierDesc) is unsafe if 
FileIdentifierDesc == NULL. You can test it with below extra code which add 
debug output when FileIdentifierDesc == NULL, and check the log after load and 
run something in a Udf partition.

+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -693,6 +693,11 @@ UdfSetPosition (
   PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);

   FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
+  if (FileIdentifierDesc == NULL) {
+DEBUG ((EFI_D_ERROR, "FileIdentifierDesc == NULL!\n"));
+return Status;
+  }
+
   if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
 //
 // If the file handle is a directory, the _only_ position that may be set 
is

I followed the Paulo's suggestion, and found this issue when using the Udf 
partition of Windows 10 Enterprise ISO image on qemu. Below is my qemu command:
/usr/local/bin/qemu-system-x86_64 -m 5120 -enable-kvm  -machine pc-q35-2.9 
-bios OVMF.fd -serial file:serial.log  -cdrom /media/jshi19/My\ 
Passport/OSImage/ISO/winserver/en_windows_server_2016_x64_dvd_9327751.iso

I've submitted a bug for this issue: 
https://bugzilla.tianocore.org/show_bug.cgi?id=704. 

I found this issue by edk2 llvm undefined behavior sanitizer, and below is the 
sanitizer output. FYI.
/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
line:0x02B8, column:0x0007 ErrorType = NullPointerUse: member access within 
null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called: 
UdfSetPosition
/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c:696:7
EfiShellFindFilesInDir
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2136:18
ShellSearchHandle
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2413:14
EfiShellFindFiles
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2586:18
EfiShellOpenFileList
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2666:12
ShellOpenFileMetaArg
/home/jshi19/wksp_efi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c:1570:14

/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
line:0x02C3, column:0x000E ErrorType = NullPointerUse: member access within 
null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called:
...

Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

> -Original Message-
> From: Paulo Alcantara [mailto:pca...@zytor.com]
> Sent: Sunday, September 10, 2017 11:52 PM
> To: Shi, Steven <steven@intel.com>; Laszlo Ersek 
> <ler...@redhat.com>;
> edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; 
> Zeng, Star <star.z...@intel.com>; Ard Biesheuvel 
> <ard.biesheu...@linaro.org>
> Subject: Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups
> 
> 
> 
> On 10/09/2017 11:27, Shi, Steven wrote:
> > OK. Does the UDF image you created correctly show up as CD-ROM
> content in Linux, e.g Fedora?
> 
> The Fedora image I have (Fedora-Workstation-Live-x86_64-26-1.5.iso) 
> does not contain a valid UDF file system, so you cannot use it for testing.
> 
> To make sure it really doesn't, I used a "Philips UDF Conformance Tool"
> that you can grab in [1], and the tool didn't find any valid UDF file 
> system in it.
> 
> I've been using an unmodified Windows 10 Enterprise image that does 
> contain a valid UDF bridge disk image (ISO9660+ElTorito+UDF) to 
> perform my tests.
> 
> Additionally, I use an USB stick that I format it with 'sudo mkudffs 
> -b
> 512 --media-type=hd /dev/sdX' and copy some files to it for testing
> 
> BTW, when testing with OVMF AARCH64, I was using my USB stick that 
> showed up correctly as 'fs

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Shi, Steven 
Hi Laszlo,
Your code is good. But there is a "Null Pointer Use" issue, which is a 
undefined behavior in C spec, in 
edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, line:696, column: 7 and 
line:707, column: 14.

Line695:
  FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
  if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
   ...

The above FileIdentifierDesc can be NULL if you run a .efi in a Udf partition. 
And the later IS_FID_DIRECTORY_FILE (FileIdentifierDesc) is unsafe if 
FileIdentifierDesc == NULL. You can test it with below extra code which add 
debug output when FileIdentifierDesc == NULL, and check the log after load and 
run something in a Udf partition.

+++ b/MdeModulePkg/Universal/Disk/UdfDxe/File.c
@@ -693,6 +693,11 @@ UdfSetPosition (
   PrivFileData = PRIVATE_UDF_FILE_DATA_FROM_THIS (This);

   FileIdentifierDesc = PrivFileData->File.FileIdentifierDesc;
+  if (FileIdentifierDesc == NULL) {
+DEBUG ((EFI_D_ERROR, "FileIdentifierDesc == NULL!\n"));
+return Status;
+  }
+
   if (IS_FID_DIRECTORY_FILE (FileIdentifierDesc)) {
 //
 // If the file handle is a directory, the _only_ position that may be set 
is

I followed the Paulo's suggestion, and found this issue when using the Udf 
partition of Windows 10 Enterprise ISO image on qemu. Below is my qemu command:
/usr/local/bin/qemu-system-x86_64 -m 5120 -enable-kvm  -machine pc-q35-2.9 
-bios OVMF.fd -serial file:serial.log  -cdrom /media/jshi19/My\ 
Passport/OSImage/ISO/winserver/en_windows_server_2016_x64_dvd_9327751.iso

I've submitted a bug for this issue: 
https://bugzilla.tianocore.org/show_bug.cgi?id=704. 

I found this issue by edk2 llvm undefined behavior sanitizer, and below is the 
sanitizer output. FYI.
/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
line:0x02B8, column:0x0007 ErrorType = NullPointerUse: member access within 
null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called: 
UdfSetPosition
/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c:696:7
EfiShellFindFilesInDir
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2136:18
ShellSearchHandle
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2413:14
EfiShellFindFiles
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2586:18
EfiShellOpenFileList
/home/jshi19/wksp_efi/edk2/ShellPkg/Application/Shell/ShellProtocol.c:2666:12
ShellOpenFileMetaArg
/home/jshi19/wksp_efi/edk2/ShellPkg/Library/UefiShellLib/UefiShellLib.c:1570:14

/home/jshi19/wksp_efi/edk2/MdeModulePkg/Universal/Disk/UdfDxe/File.c, 
line:0x02C3, column:0x000E ErrorType = NullPointerUse: member access within 
null pointer of type 'UDF_FILE_IDENTIFIER_DESCRIPTOR'
ASAN MEMORY ACCESS check fail! __ubsan_handle_type_mismatch_v1 is called:
...

Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

> -Original Message-
> From: Paulo Alcantara [mailto:pca...@zytor.com]
> Sent: Sunday, September 10, 2017 11:52 PM
> To: Shi, Steven <steven@intel.com>; Laszlo Ersek <ler...@redhat.com>;
> edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng,
> Star <star.z...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>
> Subject: Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups
> 
> 
> 
> On 10/09/2017 11:27, Shi, Steven wrote:
> > OK. Does the UDF image you created correctly show up as CD-ROM
> content in Linux, e.g Fedora?
> 
> The Fedora image I have (Fedora-Workstation-Live-x86_64-26-1.5.iso) does
> not contain a valid UDF file system, so you cannot use it for testing.
> 
> To make sure it really doesn't, I used a "Philips UDF Conformance Tool"
> that you can grab in [1], and the tool didn't find any valid UDF file
> system in it.
> 
> I've been using an unmodified Windows 10 Enterprise image that does
> contain a valid UDF bridge disk image (ISO9660+ElTorito+UDF) to perform
> my tests.
> 
> Additionally, I use an USB stick that I format it with 'sudo mkudffs -b
> 512 --media-type=hd /dev/sdX' and copy some files to it for testing
> 
> BTW, when testing with OVMF AARCH64, I was using my USB stick that
> showed up correctly as 'fsX' in UEFI shell, but with the Windows 10
> Enterprise ISO image, it didn't. I looked at the logs to see what was
> going on and I found out that the emulated CD-ROM drive is reporting a
> block size of 512 instead of 2048 -- which seems wrong to me. With
> 'qemu-system-x86_64 -cdrom', it reports a block size of 2048.
> 
> The Partition driver is still able to find a valid ElTorito partition
> because, regardless the device block size, it always use a logical block
> size of 2048 starting at 32K. In UDF

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Ard Biesheuvel 
On 10 September 2017 at 01:12, Laszlo Ersek  wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: udf_fixes_cleanups
>
> Patches #2, #3 and #4 are needed (and enough) for me to build OVMF for
> IA32 and X64 with clang-3.8, after the UDF introduction.
>
> Patches #1 and #5 are cleanups that I felt fit before patch #2 and after
> patch #4, respectively.
>
> Cc: Ard Biesheuvel 
> Cc: Eric Dong 
> Cc: Paulo Alcantara 
> Cc: Ruiyu Ni 
> Cc: Star Zeng 
>

Series Reviewed-by: Ard Biesheuvel 
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-11 Thread Laszlo Ersek 
On 09/10/17 17:51, Paulo Alcantara wrote:
> 
> 
> On 10/09/2017 11:27, Shi, Steven wrote:
>> OK. Does the UDF image you created correctly show up as CD-ROM content
>> in Linux, e.g Fedora?
> 
> The Fedora image I have (Fedora-Workstation-Live-x86_64-26-1.5.iso) does
> not contain a valid UDF file system, so you cannot use it for testing.
> 
> To make sure it really doesn't, I used a "Philips UDF Conformance Tool"
> that you can grab in [1], and the tool didn't find any valid UDF file
> system in it.
> 
> I've been using an unmodified Windows 10 Enterprise image that does
> contain a valid UDF bridge disk image (ISO9660+ElTorito+UDF) to perform
> my tests.
> 
> Additionally, I use an USB stick that I format it with 'sudo mkudffs -b
> 512 --media-type=hd /dev/sdX' and copy some files to it for testing
> 
> BTW, when testing with OVMF AARCH64, I was using my USB stick that
> showed up correctly as 'fsX' in UEFI shell, but with the Windows 10
> Enterprise ISO image, it didn't. I looked at the logs to see what was
> going on and I found out that the emulated CD-ROM drive is reporting a
> block size of 512 instead of 2048 -- which seems wrong to me. With
> 'qemu-system-x86_64 -cdrom', it reports a block size of 2048.
> 
> The Partition driver is still able to find a valid ElTorito partition
> because, regardless the device block size, it always use a logical block
> size of 2048 starting at 32K. In UDF, when searching for AVDPs (Anchor
> Volume Descriptor Pointers), we search for them at fixed locations: 256,
> N - 256, N and 512 -- but, with a block size of 512, the locations
> change to 1024, N - 1024, N and 2048 -- thus breaking the volume
> recognition sequence.
> 
> For testing the Windows image with a block size of 512, I used the
> comformance tool with './udf_test -verbose 60 -blocksize 512
> ~/img/win_ent10.iso' and it failed to find a valid UDF file system. But
> with a block size of 2048, it worked.
> 
> Is there any reason for reporting a block size of 512 when using
> '-cdrom' option in qemu-system-aarch64? Is that a bug? Or am I missing
> something here?

"-cdrom" is a legacy QEMU option, a shorthand that expands to a -drive
option, and at least one -device option (it could expand to multiple
-device options). And, this expansion can be different on different
machine types. On i440fx machine types, -cdrom can mean an IDE CD, on
q35, an AHCI CD, an aarch64/virt, a virtio-scsi-pci controller and a
scsi-cd. For this reason, it is always best to use complete -drive and
-device specifications (which is equivalent to saying it is best to
always use libvirt).

Now, the above does not imlpy that no bug can exist in this space. Can
you run the "info qtree" monitor command, in both cases, and compare the
output with regard to the CD-ROM?

Thanks
Laszlo

> Thanks!
> Paulo
> 
> [1] - https://www.lscdweb.com/registered/udf_verifier.html
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-10 Thread Paulo Alcantara 



On 10/09/2017 11:27, Shi, Steven wrote:

OK. Does the UDF image you created correctly show up as CD-ROM content in 
Linux, e.g Fedora?


The Fedora image I have (Fedora-Workstation-Live-x86_64-26-1.5.iso) does 
not contain a valid UDF file system, so you cannot use it for testing.


To make sure it really doesn't, I used a "Philips UDF Conformance Tool" 
that you can grab in [1], and the tool didn't find any valid UDF file 
system in it.


I've been using an unmodified Windows 10 Enterprise image that does 
contain a valid UDF bridge disk image (ISO9660+ElTorito+UDF) to perform 
my tests.


Additionally, I use an USB stick that I format it with 'sudo mkudffs -b 
512 --media-type=hd /dev/sdX' and copy some files to it for testing


BTW, when testing with OVMF AARCH64, I was using my USB stick that 
showed up correctly as 'fsX' in UEFI shell, but with the Windows 10 
Enterprise ISO image, it didn't. I looked at the logs to see what was 
going on and I found out that the emulated CD-ROM drive is reporting a 
block size of 512 instead of 2048 -- which seems wrong to me. With 
'qemu-system-x86_64 -cdrom', it reports a block size of 2048.


The Partition driver is still able to find a valid ElTorito partition 
because, regardless the device block size, it always use a logical block 
size of 2048 starting at 32K. In UDF, when searching for AVDPs (Anchor 
Volume Descriptor Pointers), we search for them at fixed locations: 256, 
N - 256, N and 512 -- but, with a block size of 512, the locations 
change to 1024, N - 1024, N and 2048 -- thus breaking the volume 
recognition sequence.


For testing the Windows image with a block size of 512, I used the 
comformance tool with './udf_test -verbose 60 -blocksize 512 
~/img/win_ent10.iso' and it failed to find a valid UDF file system. But 
with a block size of 2048, it worked.


Is there any reason for reporting a block size of 512 when using 
'-cdrom' option in qemu-system-aarch64? Is that a bug? Or am I missing 
something here?


Thanks!
Paulo

[1] - https://www.lscdweb.com/registered/udf_verifier.html




Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522


-Original Message-
From: Laszlo Ersek [mailto:ler...@redhat.com]
Sent: Sunday, September 10, 2017 9:52 PM
To: Shi, Steven <steven@intel.com>; edk2-devel-01 
Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng,
Star <star.z...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>
Subject: Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

Hi Steven,

On 09/10/17 10:38, Laszlo Ersek wrote:

On 09/10/17 06:24, Shi, Steven wrote:

Hi Laszlo,
How could we configure the Qemu and test the UDF driver on OVMF?


I guess you would format e.g. a DVD image with UDF, and attach it to
QEMU like any other CD-ROM.


I tried to look into this -- I tried several things, but nothing
produced an UDF image file that, when attached to the VM, would show up
in the UEFI shell as FSn:

Google returned a bunch of pages, but all I found was:
- tips that didn't work (see above),
- confused users (like me) looking for solutions.

So, at the moment, I have no idea how authoring UDF DVD images is
possible on Linux, so that they'd be recognized in edk2.

(I'm interested in the edk2 UDF driver not because I want to "author"
UDF DVD images (ISO9660+ElTorito works just fine), but because some
optical media images that were given to me are formatted UDF-only. They
can be translated into ISO9660+ElTorito off-line, but that's a chore.)

Thanks,
Laszlo

___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-10 Thread Paulo Alcantara 



On 09/09/2017 21:12, Laszlo Ersek wrote:

Repo:   https://github.com/lersek/edk2.git
Branch: udf_fixes_cleanups

Patches #2, #3 and #4 are needed (and enough) for me to build OVMF for
IA32 and X64 with clang-3.8, after the UDF introduction.

Patches #1 and #5 are cleanups that I felt fit before patch #2 and after
patch #4, respectively.

Cc: Ard Biesheuvel 
Cc: Eric Dong 
Cc: Paulo Alcantara 
Cc: Ruiyu Ni 
Cc: Star Zeng 

Thanks
Laszlo

Laszlo Ersek (5):
   MdeModulePkg/UdfDxe: ASSERT() valid ReadFileInfo Flags for INLINE_DATA
 req
   MdeModulePkg/UdfDxe: don't return unset Status if INLINE_DATA req
 succeeds
   MdeModulePkg/UdfDxe: replace zero-init of local variables with
 ZeroMem()
   MdeModulePkg/PartitionDxe: don't divide 64-bit values with C operators
   MdeModulePkg/PartitionDxe: remove always false comparison

  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 9 +++--
  MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 --
  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 5 +
  3 files changed, 16 insertions(+), 4 deletions(-)



Looks good to me. Tested it with OVMF X64 and AARCH64.

Reviewed-by: Paulo Alcantara 

Thanks!
Paulo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-10 Thread Shi, Steven 
OK. Does the UDF image you created correctly show up as CD-ROM content in 
Linux, e.g Fedora?


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

> -Original Message-
> From: Laszlo Ersek [mailto:ler...@redhat.com]
> Sent: Sunday, September 10, 2017 9:52 PM
> To: Shi, Steven <steven@intel.com>; edk2-devel-01  de...@lists.01.org>
> Cc: Ni, Ruiyu <ruiyu...@intel.com>; Dong, Eric <eric.d...@intel.com>; Zeng,
> Star <star.z...@intel.com>; Ard Biesheuvel <ard.biesheu...@linaro.org>
> Subject: Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups
> 
> Hi Steven,
> 
> On 09/10/17 10:38, Laszlo Ersek wrote:
> > On 09/10/17 06:24, Shi, Steven wrote:
> >> Hi Laszlo,
> >> How could we configure the Qemu and test the UDF driver on OVMF?
> >
> > I guess you would format e.g. a DVD image with UDF, and attach it to
> > QEMU like any other CD-ROM.
> 
> I tried to look into this -- I tried several things, but nothing
> produced an UDF image file that, when attached to the VM, would show up
> in the UEFI shell as FSn:
> 
> Google returned a bunch of pages, but all I found was:
> - tips that didn't work (see above),
> - confused users (like me) looking for solutions.
> 
> So, at the moment, I have no idea how authoring UDF DVD images is
> possible on Linux, so that they'd be recognized in edk2.
> 
> (I'm interested in the edk2 UDF driver not because I want to "author"
> UDF DVD images (ISO9660+ElTorito works just fine), but because some
> optical media images that were given to me are formatted UDF-only. They
> can be translated into ISO9660+ElTorito off-line, but that's a chore.)
> 
> Thanks,
> Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-10 Thread Laszlo Ersek 
Hi Steven,

On 09/10/17 10:38, Laszlo Ersek wrote:
> On 09/10/17 06:24, Shi, Steven wrote:
>> Hi Laszlo,
>> How could we configure the Qemu and test the UDF driver on OVMF?  
> 
> I guess you would format e.g. a DVD image with UDF, and attach it to
> QEMU like any other CD-ROM.

I tried to look into this -- I tried several things, but nothing
produced an UDF image file that, when attached to the VM, would show up
in the UEFI shell as FSn:

Google returned a bunch of pages, but all I found was:
- tips that didn't work (see above),
- confused users (like me) looking for solutions.

So, at the moment, I have no idea how authoring UDF DVD images is
possible on Linux, so that they'd be recognized in edk2.

(I'm interested in the edk2 UDF driver not because I want to "author"
UDF DVD images (ISO9660+ElTorito works just fine), but because some
optical media images that were given to me are formatted UDF-only. They
can be translated into ISO9660+ElTorito off-line, but that's a chore.)

Thanks,
Laszlo
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups

2017-09-09 Thread Shi, Steven 
Hi Laszlo,
How could we configure the Qemu and test the UDF driver on OVMF?  
BTW, how could we configure the Qemu to create a full feature scope machine to 
include all possible devices in it, e.g. USB, ISA, SD/MMC, network etc. 


Steven Shi
Intel\SSG\STO\UEFI Firmware

Tel: +86 021-61166522
iNet: 821-6522

> -Original Message-
> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of
> Laszlo Ersek
> Sent: Sunday, September 10, 2017 8:13 AM
> To: edk2-devel-01 
> Cc: Ni, Ruiyu ; Dong, Eric ; Zeng,
> Star ; Ard Biesheuvel 
> Subject: [edk2] [PATCH 0/5] MdeModulePkg: UDF fixes and cleanups
> 
> Repo:   https://github.com/lersek/edk2.git
> Branch: udf_fixes_cleanups
> 
> Patches #2, #3 and #4 are needed (and enough) for me to build OVMF for
> IA32 and X64 with clang-3.8, after the UDF introduction.
> 
> Patches #1 and #5 are cleanups that I felt fit before patch #2 and after
> patch #4, respectively.
> 
> Cc: Ard Biesheuvel 
> Cc: Eric Dong 
> Cc: Paulo Alcantara 
> Cc: Ruiyu Ni 
> Cc: Star Zeng 
> 
> Thanks
> Laszlo
> 
> Laszlo Ersek (5):
>   MdeModulePkg/UdfDxe: ASSERT() valid ReadFileInfo Flags for INLINE_DATA
> req
>   MdeModulePkg/UdfDxe: don't return unset Status if INLINE_DATA req
> succeeds
>   MdeModulePkg/UdfDxe: replace zero-init of local variables with
> ZeroMem()
>   MdeModulePkg/PartitionDxe: don't divide 64-bit values with C operators
>   MdeModulePkg/PartitionDxe: remove always false comparison
> 
>  MdeModulePkg/Universal/Disk/PartitionDxe/Udf.c| 9 +++--
>  MdeModulePkg/Universal/Disk/UdfDxe/File.c | 6 --
>  MdeModulePkg/Universal/Disk/UdfDxe/FileSystemOperations.c | 5 +
>  3 files changed, 16 insertions(+), 4 deletions(-)
> 
> --
> 2.14.1.3.gb7cf6e02401b
> 
> ___
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel