Re: [PATCH v8 0/2] Introduce EROFS support

[Gao Xiang]

Hi Glenn, Daniel,

On 2024/4/24 17:30, Yifan Zhao wrote:
> EROFS [1] is a lightweight read-only filesystem designed for performance
> which has already been shipped in most Linux distributions as well as widely
> used in several scenarios, such as Android system partitions, container
> images, and rootfs for embedded devices.
>
> This patch brings EROFS uncompressed support together with related tests.
> Now, it's possible to boot directly through GRUB with an EROFS rootfs.
>
> EROFS compressed files will be supported later since it has more work to
> polish.
>
> [1] https://erofs.docs.kernel.org
>
> changelog since v7:
> - check if log2_blksz is too small in erofs_mount()
> - handle bondary case and memleak in erofs_iterate_dir()
> - fix incorrect grub_strnlen() implementation

Could you help have another look at this latest version?
Is it ready for merging (Yifan has been working on for almost one year
in his leisure time) so that kernel/initrd can be loaded then..
Any comment would be much helpful.

Thanks,
Gao Xiang

> Yifan Zhao (2):
>    fs/erofs: Add support for EROFS
>    fs/erofs: Add tests for EROFS in grub-fs-tester
>
>   .gitignore                   |   1 +
>   INSTALL                      |   8 +-
>   Makefile.util.def            |   7 +
>   docs/grub.texi               |   3 +-
>   grub-core/Makefile.core.def  |   5 +
>   grub-core/fs/erofs.c         | 984 +++++++++++++++++++++++++++++++++++
>   grub-core/kern/misc.c        |  14 +
>   include/grub/misc.h          |   1 +
>   tests/erofs_test.in          |  20 +
>   tests/util/grub-fs-tester.in |  32 +-
>   10 files changed, 1063 insertions(+), 12 deletions(-)
>   create mode 100644 grub-core/fs/erofs.c
>   create mode 100644 tests/erofs_test.in
>
> Interdiff against v7:
> diff --git a/grub-core/fs/erofs.c b/grub-core/fs/erofs.c
> index 5b89b7924..13f92e71a 100644
> --- a/grub-core/fs/erofs.c
> +++ b/grub-core/fs/erofs.c
> @@ -101,6 +101,7 @@ struct grub_erofs_inode_chunk_info
>   
>   #define EROFS_NULL_ADDR			1
>   #define EROFS_NAME_LEN                        255
> +#define EROFS_MIN_LOG2_BLOCK_SIZE      9
>   #define EROFS_MAX_LOG2_BLOCK_SIZE     16
>   
>   struct grub_erofs_inode_chunk_index
> @@ -558,7 +559,7 @@ erofs_iterate_dir (grub_fshelp_node_t dir, 
> grub_fshelp_iterate_dir_hook_t hook,
>         goto not_found;
>   
>         nameoff = grub_le_to_cpu16 (de->nameoff);
> -      if (nameoff < sizeof (struct grub_erofs_dirent) || nameoff > maxsize)
> +      if (nameoff < sizeof (struct grub_erofs_dirent) || nameoff >= maxsize)
>         {
>           grub_error (GRUB_ERR_BAD_FS,
>                       "invalid nameoff %u @ inode %" PRIuGRUB_UINT64_T,
> @@ -587,11 +588,12 @@ erofs_iterate_dir (grub_fshelp_node_t dir, 
> grub_fshelp_iterate_dir_hook_t hook,
>           fdiro->inode_loaded = false;
>   
>   	  nameoff = grub_le_to_cpu16 (de->nameoff);
> -         if (nameoff < sizeof (struct grub_erofs_dirent) || nameoff > maxsize)
> +         if (nameoff < sizeof (struct grub_erofs_dirent) || nameoff >= maxsize)
>             {
>               grub_error (GRUB_ERR_BAD_FS,
>                           "invalid nameoff %u @ inode %" PRIuGRUB_UINT64_T,
>                           nameoff, dir->ino);
> +             grub_free (fdiro);
>               goto not_found;
>             }
>   
> @@ -607,6 +609,7 @@ erofs_iterate_dir (grub_fshelp_node_t dir, grub_fshelp_iterate_dir_hook_t hook,
>                           "invalid de_namelen %" PRIuGRUB_SIZE
>                           " @ inode %" PRIuGRUB_UINT64_T,
>                           de_namelen, dir->ino);
> +             grub_free (fdiro);
>               goto not_found;
>             }
>   
> @@ -700,6 +703,7 @@ erofs_mount (grub_disk_t disk, bool read_root)
>     if (err != GRUB_ERR_NONE)
>       return NULL;
>     if (sb.magic != grub_cpu_to_le32_compile_time (EROFS_MAGIC) ||
> +      grub_le_to_cpu32 (sb.log2_blksz) < EROFS_MIN_LOG2_BLOCK_SIZE ||
>         grub_le_to_cpu32 (sb.log2_blksz) > EROFS_MAX_LOG2_BLOCK_SIZE)
>       {
>         grub_error (GRUB_ERR_BAD_FS, "not a valid erofs filesystem");
> diff --git a/grub-core/kern/misc.c b/grub-core/kern/misc.c
> index ecb27fa5a..67240d64d 100644
> --- a/grub-core/kern/misc.c
> +++ b/grub-core/kern/misc.c
> @@ -602,7 +602,7 @@ grub_strnlen (const char *s, grub_size_t n)
>     if (n == 0)
>       return 0;
>   
> -  while (*p && n--)
> +  while (n-- && *p)
>       p++;
>   
>     return p - s;