[PATCH] efi: Restrict the simple file system protocol to support only FAT

[Masami Hiramatsu]

Hi Akashi-san,

2021年6月3日(木) 11:50 AKASHI Takahiro <takahiro.akashi at linaro.org>:
>
> On Thu, Jun 03, 2021 at 11:17:56AM +0900, Masami Hiramatsu wrote:
> > Because UEFI specification v2.9, 13.3 File System Format said "The
> > file system supported by the Extensible Firmware Interface is based
> > on the FAT file system.", the simple file system protocol might be
> > better to support only FAT filesystem.
>
> If I remember correctly, Heinrich rejected the idea a long time ago.
>
> When I posted the commit 867400677cda ("efi_loader: disk: install
> FILE_SYSTEM_PROTOCOL only if available"), he insisted that the UEFI
> specification does require FAT support but that it doesn't deny
> any support for other file systems.
> So I had to change the code, allowing FS_TYPE_ANY.

Thanks for the good information.
Indeed, the specification doesn't deny any other filesystems.

I think the problem is that the simple file system protocol doesn't
provide any filesystem format information.
EFI_FILE_PROTOCOL.GetInfo() will get the EFI_FILE_SYSTEM_INFO, but it
doesn't include the filesystem format information. Thus, UEFI
application can not know why it can write the file on this volume but
cannot on another volume.

Maybe we can set the ReadOnly field of EFI_FILE_SYSTEM_INFO correctly
if the filesystem doesn't support write operation, and make
EFI_FILE_PROTOCOL.Write() returns EFI_WRITE_PROTECTED. In that case it
may help UEFI application understands what happen.

Thank you,

>
> -Takahiro Akashi
>
> > There must be no problem from UEFI application to access only FAT
> > because ESP must be formatted by FAT32 and the removable media is
> > FAT12 or FAT16, according to the UEFI spec.
> >
> > Reported-by: Kazuhiko Sakamoto <sakamoto.kazuhiko at socionext.com>
> > Signed-off-by: Masami Hiramatsu <masami.hiramatsu at linaro.org>
> > ---
> >  lib/efi_loader/efi_disk.c |   18 ++++++++++++------
> >  1 file changed, 12 insertions(+), 6 deletions(-)
> >
> > diff --git a/lib/efi_loader/efi_disk.c b/lib/efi_loader/efi_disk.c
> > index 307d5d759b..f69ae6587f 100644
> > --- a/lib/efi_loader/efi_disk.c
> > +++ b/lib/efi_loader/efi_disk.c
> > @@ -318,19 +318,25 @@ efi_fs_from_path(struct efi_device_path *full_path)
> >  }
> >
> >  /**
> > - * efi_fs_exists() - check if a partition bears a file system
> > + * efi_supported_fs_exists() - check if a partition bears a supported file system
> >   *
> >   * @desc:    block device descriptor
> >   * @part:    partition number
> > - * Return:   1 if a file system exists on the partition
> > + * Return:   1 if a supported file system exists on the partition
> >   *           0 otherwise
> >   */
> > -static int efi_fs_exists(struct blk_desc *desc, int part)
> > +static int efi_supported_fs_exists(struct blk_desc *desc, int part)
> >  {
> >       if (fs_set_blk_dev_with_part(desc, part))
> >               return 0;
> >
> > -     if (fs_get_type() == FS_TYPE_ANY)
> > +     /*
> > +      * Because UEFI specification v2.9, 13.3 File System Format said
> > +      * "The file system supported by the Extensible Firmware Interface
> > +      * is based on the FAT file system.", the simple file system protocol
> > +      * should support only FAT filesystem.
> > +      */
> > +     if (fs_get_type() != FS_TYPE_FAT)
> >               return 0;
> >
> >       fs_close();
> > @@ -428,10 +434,10 @@ static efi_status_t efi_disk_add_dev(
> >
> >       /*
> >        * On partitions or whole disks without partitions install the
> > -      * simple file system protocol if a file system is available.
> > +      * simple file system protocol if a supported file system exists.
> >        */
> >       if ((part || desc->part_type == PART_TYPE_UNKNOWN) &&
> > -         efi_fs_exists(desc, part)) {
> > +         efi_supported_fs_exists(desc, part)) {
> >               diskobj->volume = efi_simple_file_system(desc, part,
> >                                                        diskobj->dp);
> >               ret = efi_add_protocol(&diskobj->header,
> >



-- 
Masami Hiramatsu