On Fri, May 10, 2024 at 08:52:55AM +0800, Gao Xiang wrote: > From: Yifan Zhao <zhaoyi...@sjtu.edu.cn> > > 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. 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 > > Signed-off-by: Yifan Zhao <zhaoyi...@sjtu.edu.cn> > Tested-by: Daniel Axtens <d...@axtens.net> # fuzz testing only > Signed-off-by: Gao Xiang <hsiang...@linux.alibaba.com>
In general patch LGTM except some nits... > --- > Tested-by Link: > https://lists.gnu.org/archive/html/grub-devel/2024-05/msg00001.html > > INSTALL | 8 +- > Makefile.util.def | 1 + > docs/grub.texi | 3 +- > grub-core/Makefile.core.def | 5 + > grub-core/fs/erofs.c | 1008 +++++++++++++++++++++++++++++++++++ > 5 files changed, 1020 insertions(+), 5 deletions(-) > create mode 100644 grub-core/fs/erofs.c > > diff --git a/INSTALL b/INSTALL > index 8d9207c84..84030c9f4 100644 > --- a/INSTALL > +++ b/INSTALL > @@ -77,15 +77,15 @@ Prerequisites for make-check: > > * If running a Linux kernel the following modules must be loaded: > - fuse, loop > - - btrfs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, minix, nilfs2, > + - btrfs, erofs, ext4, f2fs, fat, hfs, hfsplus, jfs, mac-roman, minix, > nilfs2, > reiserfs, udf, xfs > - On newer kernels, the exfat kernel modules may be used instead of the > exfat FUSE filesystem > * The following are Debian named packages required mostly for the full > suite of filesystem testing (but some are needed by other tests as well): > - - btrfs-progs, dosfstools, e2fsprogs, exfat-utils, f2fs-tools, genromfs, > - hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs, squashfs-tools, > - reiserfsprogs, udftools, xfsprogs, zfs-fuse > + - btrfs-progs, dosfstools, e2fsprogs, erofs-utils, exfat-utils, f2fs-tools, > + genromfs, hfsprogs, jfsutils, nilfs-tools, ntfs-3g, reiserfsprogs, > + squashfs-tools, reiserfsprogs, udftools, xfsprogs, zfs-fuse > - exfat-fuse, if not using the exfat kernel module > - gzip, lzop, xz-utils > - attr, cpio, g++, gawk, parted, recode, tar, util-linux > diff --git a/Makefile.util.def b/Makefile.util.def > index 9432365a9..8d3bc107f 100644 > --- a/Makefile.util.def > +++ b/Makefile.util.def > @@ -98,6 +98,7 @@ library = { > common = grub-core/fs/cpio_be.c; > common = grub-core/fs/odc.c; > common = grub-core/fs/newc.c; > + common = grub-core/fs/erofs.c; > common = grub-core/fs/ext2.c; > common = grub-core/fs/fat.c; > common = grub-core/fs/exfat.c; > diff --git a/docs/grub.texi b/docs/grub.texi > index d32266f69..b198d963d 100644 > --- a/docs/grub.texi > +++ b/docs/grub.texi > @@ -353,6 +353,7 @@ blocklist notation. The currently supported filesystem > types are @dfn{Amiga > Fast FileSystem (AFFS)}, @dfn{AtheOS fs}, @dfn{BeFS}, > @dfn{BtrFS} (including raid0, raid1, raid10, gzip and lzo), > @dfn{cpio} (little- and big-endian bin, odc and newc variants), > +@dfn{EROFS} (only uncompressed support for now), > @dfn{Linux ext2/ext3/ext4}, @dfn{DOS FAT12/FAT16/FAT32}, > @dfn{exFAT}, @dfn{F2FS}, @dfn{HFS}, @dfn{HFS+}, > @dfn{ISO9660} (including Joliet, Rock-ridge and multi-chunk files), > @@ -6276,7 +6277,7 @@ assumed to be encoded in UTF-8. > NTFS, JFS, UDF, HFS+, exFAT, long filenames in FAT, Joliet part of > ISO9660 are treated as UTF-16 as per specification. AFS and BFS are read > as UTF-8, again according to specification. BtrFS, cpio, tar, squash4, minix, > -minix2, minix3, ROMFS, ReiserFS, XFS, ext2, ext3, ext4, FAT (short names), > +minix2, minix3, ROMFS, ReiserFS, XFS, EROFS, ext2, ext3, ext4, FAT (short > names), > F2FS, RockRidge part of ISO9660, nilfs2, UFS1, UFS2 and ZFS are assumed > to be UTF-8. This might be false on systems configured with legacy charset > but as long as the charset used is superset of ASCII you should be able to > diff --git a/grub-core/Makefile.core.def b/grub-core/Makefile.core.def > index 8e1b1d9f3..7fa9446bd 100644 > --- a/grub-core/Makefile.core.def > +++ b/grub-core/Makefile.core.def > @@ -1442,6 +1442,11 @@ module = { > common = fs/odc.c; > }; > > +module = { > + name = erofs; > + common = fs/erofs.c; > +}; > + > module = { > name = ext2; > common = fs/ext2.c; > diff --git a/grub-core/fs/erofs.c b/grub-core/fs/erofs.c > new file mode 100644 > index 000000000..14c86f435 > --- /dev/null > +++ b/grub-core/fs/erofs.c > @@ -0,0 +1,1008 @@ > +/* erofs.c - Enhanced Read-Only File System */ > +/* > + * GRUB -- GRand Unified Bootloader > + * Copyright (C) 2024 Free Software Foundation, Inc. > + * > + * GRUB is free software: you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation, either version 3 of the License, or > + * (at your option) any later version. > + * > + * GRUB is distributed in the hope that it will be useful, > + * but WITHOUT ANY WARRANTY; without even the implied warranty of > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > + * GNU General Public License for more details. > + * > + * You should have received a copy of the GNU General Public License > + * along with GRUB. If not, see <http://www.gnu.org/licenses/>. > + */ > + > +#include <grub/disk.h> > +#include <grub/dl.h> > +#include <grub/err.h> > +#include <grub/file.h> > +#include <grub/fs.h> > +#include <grub/fshelp.h> > +#include <grub/misc.h> > +#include <grub/mm.h> > +#include <grub/safemath.h> > +#include <grub/types.h> > + > +GRUB_MOD_LICENSE ("GPLv3+"); > + > +#define EROFS_SUPER_OFFSET 1024 > +#define EROFS_MAGIC 0xE0F5E1E2 > +#define EROFS_ISLOTBITS 5 > + > +#define EROFS_FEATURE_INCOMPAT_CHUNKED_FILE 0x00000004 > +#define EROFS_ALL_FEATURE_INCOMPAT > EROFS_FEATURE_INCOMPAT_CHUNKED_FILE > + > +struct grub_erofs_super > +{ > + grub_uint32_t magic; > + grub_uint32_t checksum; > + grub_uint32_t feature_compat; > + grub_uint8_t log2_blksz; > + grub_uint8_t sb_extslots; May I ask you to align struct/union members in the following way? grub_uint32_t magic; grub_uint32_t checksum; grub_uint32_t feature_compat; grub_uint8_t log2_blksz; grub_uint8_t sb_extslots; grub_uint16_t root_nid; grub_uint64_t inos; grub_uint64_t build_time; grub_uint32_t build_time_nsec; ... Please do this for all/most structs and unions definitions below. > + > + grub_uint16_t root_nid; > + grub_uint64_t inos; > + > + grub_uint64_t build_time; > + grub_uint32_t build_time_nsec; > + grub_uint32_t blocks; > + grub_uint32_t meta_blkaddr; > + grub_uint32_t xattr_blkaddr; > + grub_packed_guid_t uuid; > + grub_uint8_t volume_name[16]; > + grub_uint32_t feature_incompat; > + > + union > + { > + grub_uint16_t available_compr_algs; > + grub_uint16_t lz4_max_distance; > + } GRUB_PACKED u1; > + > + grub_uint16_t extra_devices; > + grub_uint16_t devt_slotoff; > + grub_uint8_t log2_dirblksz; > + grub_uint8_t xattr_prefix_count; > + grub_uint32_t xattr_prefix_start; > + grub_uint64_t packed_nid; > + grub_uint8_t reserved2[24]; > +} GRUB_PACKED; > + > +#define EROFS_INODE_LAYOUT_COMPACT 0 > +#define EROFS_INODE_LAYOUT_EXTENDED 1 > + > +#define EROFS_INODE_FLAT_PLAIN 0 > +#define EROFS_INODE_COMPRESSED_FULL 1 > +#define EROFS_INODE_FLAT_INLINE 2 > +#define EROFS_INODE_COMPRESSED_COMPACT 3 > +#define EROFS_INODE_CHUNK_BASED 4 > + > +#define EROFS_I_VERSION_MASKS 0x01 > +#define EROFS_I_DATALAYOUT_MASKS 0x07 > + > +#define EROFS_I_VERSION_BIT 0 > +#define EROFS_I_DATALAYOUT_BIT 1 > + > +struct grub_erofs_inode_chunk_info > +{ > + grub_uint16_t format; > + grub_uint16_t reserved; > +} GRUB_PACKED; > + > +#define EROFS_CHUNK_FORMAT_BLKBITS_MASK 0x001F > +#define EROFS_CHUNK_FORMAT_INDEXES 0x0020 > + > +#define EROFS_BLOCK_MAP_ENTRY_SIZE 4 > +#define EROFS_MAP_MAPPED 0x02 > + > +#define EROFS_NULL_ADDR 1 > +#define EROFS_NAME_LEN 255 > +#define EROFS_PATH_LEN 4096 > +#define EROFS_MIN_LOG2_BLOCK_SIZE 9 > +#define EROFS_MAX_LOG2_BLOCK_SIZE 16 > + > +struct grub_erofs_inode_chunk_index > +{ > + grub_uint16_t advise; > + grub_uint16_t device_id; > + grub_uint32_t blkaddr; > +}; > + > +union grub_erofs_inode_i_u > +{ > + grub_uint32_t compressed_blocks; > + grub_uint32_t raw_blkaddr; > + > + grub_uint32_t rdev; > + > + struct grub_erofs_inode_chunk_info c; Things like this one can be an exception... > +}; > + > +struct grub_erofs_inode_compact > +{ > + grub_uint16_t i_format; > + > + grub_uint16_t i_xattr_icount; > + grub_uint16_t i_mode; > + grub_uint16_t i_nlink; > + grub_uint32_t i_size; > + grub_uint32_t i_reserved; > + > + union grub_erofs_inode_i_u i_u; > + > + grub_uint32_t i_ino; > + grub_uint16_t i_uid; > + grub_uint16_t i_gid; > + grub_uint32_t i_reserved2; > +} GRUB_PACKED; > + > +struct grub_erofs_inode_extended > +{ > + grub_uint16_t i_format; > + > + grub_uint16_t i_xattr_icount; > + grub_uint16_t i_mode; > + grub_uint16_t i_reserved; > + grub_uint64_t i_size; > + > + union grub_erofs_inode_i_u i_u; > + > + grub_uint32_t i_ino; > + > + grub_uint32_t i_uid; > + grub_uint32_t i_gid; > + grub_uint64_t i_mtime; > + grub_uint32_t i_mtime_nsec; > + grub_uint32_t i_nlink; > + grub_uint8_t i_reserved2[16]; > +} GRUB_PACKED; > + > +union grub_erofs_inode > +{ > + struct grub_erofs_inode_compact c; > + struct grub_erofs_inode_extended e; > +} GRUB_PACKED; > + > +#define EROFS_FT_UNKNOWN 0 > +#define EROFS_FT_REG_FILE 1 > +#define EROFS_FT_DIR 2 > +#define EROFS_FT_CHRDEV 3 > +#define EROFS_FT_BLKDEV 4 > +#define EROFS_FT_FIFO 5 > +#define EROFS_FT_SOCK 6 > +#define EROFS_FT_SYMLINK 7 > + > +struct grub_erofs_dirent > +{ > + grub_uint64_t nid; > + grub_uint16_t nameoff; > + grub_uint8_t file_type; > + grub_uint8_t reserved; > +} GRUB_PACKED; > + > +struct grub_erofs_map_blocks > +{ > + grub_uint64_t m_pa; /* physical address */ > + grub_uint64_t m_la; /* logical address */ > + grub_uint64_t m_plen; /* physical length */ > + grub_uint64_t m_llen; /* logical length */ > + grub_uint32_t m_flags; > +}; > + > +struct grub_erofs_xattr_ibody_header > +{ > + grub_uint32_t h_reserved; > + grub_uint8_t h_shared_count; > + grub_uint8_t h_reserved2[7]; > + grub_uint32_t h_shared_xattrs[0]; > +}; > + > +struct grub_fshelp_node > +{ > + struct grub_erofs_data *data; > + union grub_erofs_inode inode; > + > + grub_uint64_t ino; > + grub_uint8_t inode_type; > + grub_uint8_t inode_datalayout; > + > + /* if the inode has been read into memory? */ > + bool inode_loaded; > +}; > + > +struct grub_erofs_data > +{ > + grub_disk_t disk; > + struct grub_erofs_super sb; > + > + struct grub_fshelp_node inode; > +}; > + > +#define erofs_blocksz(data) (((grub_uint32_t) 1) << data->sb.log2_blksz) > + > +static grub_size_t > +grub_erofs_strnlen (const char *s, grub_size_t n) > +{ > + const char *p = s; > + > + if (n == 0) > + return 0; > + > + while (n-- && *p) > + p++; > + > + return p - s; > +} > + > +static grub_uint64_t > +erofs_iloc (grub_fshelp_node_t node) > +{ > + struct grub_erofs_super *sb = &node->data->sb; > + > + return ((grub_uint64_t) grub_le_to_cpu32 (sb->meta_blkaddr) << > sb->log2_blksz) + > + (node->ino << EROFS_ISLOTBITS); > +} > + > +static grub_err_t > +erofs_read_inode (struct grub_erofs_data *data, grub_fshelp_node_t node) > +{ > + union grub_erofs_inode *di; > + grub_err_t err; > + grub_uint16_t i_format; > + grub_uint64_t addr = erofs_iloc (node); > + > + di = (union grub_erofs_inode *) &node->inode; > + > + err = grub_disk_read (data->disk, addr >> GRUB_DISK_SECTOR_BITS, > + addr & (GRUB_DISK_SECTOR_SIZE - 1), > + sizeof (struct grub_erofs_inode_compact), &di->c); > + if (err != GRUB_ERR_NONE) > + return err; > + > + i_format = grub_le_to_cpu16 (di->c.i_format); > + node->inode_type = (i_format >> EROFS_I_VERSION_BIT) & > EROFS_I_VERSION_MASKS; > + node->inode_datalayout = (i_format >> EROFS_I_DATALAYOUT_BIT) & > EROFS_I_DATALAYOUT_MASKS; > + > + switch (node->inode_type) > + { > + case EROFS_INODE_LAYOUT_EXTENDED: > + addr += sizeof (struct grub_erofs_inode_compact); > + err = grub_disk_read ( > + data->disk, addr >> GRUB_DISK_SECTOR_BITS, > + addr & (GRUB_DISK_SECTOR_SIZE - 1), > + sizeof (struct grub_erofs_inode_extended) - sizeof (struct > grub_erofs_inode_compact), > + (grub_uint8_t *) di + sizeof (struct grub_erofs_inode_compact)); This function call is unreadable. Please fix it. I am OK with lines longer than 80 chars. So, first argument for the grub_disk_read() should be immediately behind "(". Then other arguments may follow below starting from the same column with the first argument. grub_disk_read (data->disk, addr >> GRUB_DISK_SECTOR_BITS, addr & (GRUB_DISK_SECTOR_SIZE - 1), ... > + if (err != GRUB_ERR_NONE) > + return err; > + break; > + case EROFS_INODE_LAYOUT_COMPACT: > + break; > + default: > + return grub_error (GRUB_ERR_BAD_FS, "invalid type %u @ inode %" > PRIuGRUB_UINT64_T, > + node->inode_type, node->ino); > + } > + > + node->inode_loaded = true; > + > + return 0; > +} > + > +static grub_uint64_t > +erofs_inode_size (grub_fshelp_node_t node) > +{ > + return node->inode_type == EROFS_INODE_LAYOUT_COMPACT > + ? sizeof (struct grub_erofs_inode_compact) > + : sizeof (struct grub_erofs_inode_extended); > +} > + > +static grub_uint64_t > +erofs_inode_file_size (grub_fshelp_node_t node) > +{ > + union grub_erofs_inode *di = (union grub_erofs_inode *) &node->inode; > + > + return node->inode_type == EROFS_INODE_LAYOUT_COMPACT > + ? grub_le_to_cpu32 (di->c.i_size) > + : grub_le_to_cpu64 (di->e.i_size); > +} > + > +static grub_uint32_t > +erofs_inode_xattr_ibody_size (grub_fshelp_node_t node) > +{ > + grub_uint16_t cnt = grub_le_to_cpu16 (node->inode.e.i_xattr_icount); > + > + if (cnt == 0) > + return 0; > + > + return sizeof (struct grub_erofs_xattr_ibody_header) + ((cnt - 1) * sizeof > (grub_uint32_t)); > +} > + > +static grub_uint64_t > +erofs_inode_mtime (grub_fshelp_node_t node) > +{ > + return node->inode_type == EROFS_INODE_LAYOUT_COMPACT > + ? grub_le_to_cpu64 (node->data->sb.build_time) > + : grub_le_to_cpu64 (node->inode.e.i_mtime); > +} > + > +static grub_err_t > +erofs_map_blocks_flatmode (grub_fshelp_node_t node, > + struct grub_erofs_map_blocks *map) > +{ > + grub_uint64_t nblocks, lastblk, file_size; > + bool tailendpacking = (node->inode_datalayout == EROFS_INODE_FLAT_INLINE); > + grub_uint64_t blocksz = erofs_blocksz (node->data); > + > + /* file_size is checked by caller and cannot be zero, hence nblocks > 0 */ > + file_size = erofs_inode_file_size (node); > + if (grub_add (file_size, blocksz - 1, &nblocks)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); You use "overflow is detected" message everywhere. It is not helpful. I think (almost) every message should be different, e.g. "nblocks overflow" or "invalid argument: %d..." or ... > + nblocks >>= node->data->sb.log2_blksz; > + lastblk = nblocks - tailendpacking; > + > + map->m_flags = EROFS_MAP_MAPPED; > + > + /* no overflow as (lastblk <= nblocks) && (nblocks * blocksz <= UINT64_MAX > - blocksz + 1) */ > + if (map->m_la < (lastblk * blocksz)) > + { > + if (grub_mul ((grub_uint64_t) grub_le_to_cpu32 > (node->inode.e.i_u.raw_blkaddr), blocksz, > + &map->m_pa) || Please move this to the line with grub_mul()... > + grub_add (map->m_pa, map->m_la, &map->m_pa)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); Again, this message does not help... > + if (grub_sub (lastblk * blocksz, map->m_la, &map->m_plen)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); To be precise, this is an underflow... > + } > + else if (tailendpacking) > + { > + if (grub_add (erofs_iloc (node), erofs_inode_size (node), &map->m_pa) > || > + grub_add (map->m_pa, erofs_inode_xattr_ibody_size (node), &map->m_pa) > || > + grub_add (map->m_pa, map->m_la % blocksz, &map->m_pa)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); Please fix all these messages... > + if (grub_sub (file_size, map->m_la, &map->m_plen)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); > + > + /* no overflow as map->m_plen <= UINT64_MAX - blocksz + 1 */ > + if (((map->m_pa % blocksz) + map->m_plen) > blocksz) > + return grub_error ( > + GRUB_ERR_BAD_FS, > + "inline data cross block boundary @ inode %" PRIuGRUB_UINT64_T, > + node->ino); Wrong wrapping as above. Please fix it. > + } > + else > + return grub_error (GRUB_ERR_BAD_FS, > + "invalid map->m_la=%" PRIuGRUB_UINT64_T > + " @ inode %" PRIuGRUB_UINT64_T, > + map->m_la, node->ino); Ditto... Please fix similar wrapping everywhere. > + map->m_llen = map->m_plen; > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +erofs_map_blocks_chunkmode (grub_fshelp_node_t node, > + struct grub_erofs_map_blocks *map) > +{ > + grub_uint16_t chunk_format = grub_le_to_cpu16 (node->inode.e.i_u.c.format); > + grub_uint64_t unit, pos, chunknr, blkaddr; > + grub_uint8_t chunkbits; > + grub_err_t err; > + > + if (chunk_format & EROFS_CHUNK_FORMAT_INDEXES) > + unit = sizeof (struct grub_erofs_inode_chunk_index); > + else > + unit = EROFS_BLOCK_MAP_ENTRY_SIZE; > + > + chunkbits = node->data->sb.log2_blksz + (chunk_format & > EROFS_CHUNK_FORMAT_BLKBITS_MASK); > + if (chunkbits > 63) > + return grub_error (GRUB_ERR_BAD_FS, "invalid chunkbits %u @ inode %" > PRIuGRUB_UINT64_T, > + chunkbits, node->ino); > + > + chunknr = map->m_la >> chunkbits; > + > + if (grub_add (erofs_iloc (node), erofs_inode_size (node), &pos) || > + grub_add (pos, erofs_inode_xattr_ibody_size (node), &pos)) Maybe in some cases it is worth checking overflow for every operation separately and then print relevant messages. > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); > + > + /* pos = ALIGN_UP(pos, unit) */ > + if (grub_add (pos, unit - 1, &pos)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); > + pos &= ~(unit - 1); May I ask you to add a macro which allows overflow detection, e.g. ALIGN_UP_OVF(). This should go to a separate patch. > + /* no overflow for multiplication as chunkbits >= 9 and sizeof(unit) <= 8 > */ Please start all comments with upper case and end with full stop. > + if (grub_add (pos, chunknr * unit, &pos)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); > + > + map->m_la = chunknr << chunkbits; > + > + if (grub_sub (erofs_inode_file_size (node), map->m_la, &map->m_plen)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); > + map->m_plen = grub_min (((grub_uint64_t) 1) << chunkbits, > + ALIGN_UP (map->m_plen, erofs_blocksz (node->data))); > + > + if (chunk_format & EROFS_CHUNK_FORMAT_INDEXES) > + { > + struct grub_erofs_inode_chunk_index idx; > + > + err = grub_disk_read (node->data->disk, pos >> GRUB_DISK_SECTOR_BITS, > + pos & (GRUB_DISK_SECTOR_SIZE - 1), unit, &idx); > + if (err != GRUB_ERR_NONE) > + return err; > + > + blkaddr = grub_le_to_cpu32 (idx.blkaddr); > + } > + else > + { > + grub_uint32_t blkaddr_le; > + > + err = grub_disk_read (node->data->disk, pos >> GRUB_DISK_SECTOR_BITS, > + pos & (GRUB_DISK_SECTOR_SIZE - 1), unit, > &blkaddr_le); > + if (err != GRUB_ERR_NONE) > + return err; > + > + blkaddr = grub_le_to_cpu32 (blkaddr_le); > + } > + > + if (blkaddr == EROFS_NULL_ADDR) > + { > + map->m_pa = 0; > + map->m_flags = 0; > + } > + else > + { > + map->m_pa = blkaddr << node->data->sb.log2_blksz; > + map->m_flags = EROFS_MAP_MAPPED; > + } > + > + map->m_llen = map->m_plen; > + return GRUB_ERR_NONE; > +} > + > +static grub_err_t > +erofs_map_blocks (grub_fshelp_node_t node, struct grub_erofs_map_blocks *map) > +{ > + if (map->m_la >= erofs_inode_file_size (node)) > + { > + map->m_llen = map->m_plen = 0; > + map->m_pa = 0; > + map->m_flags = 0; > + return GRUB_ERR_NONE; > + } > + > + if (node->inode_datalayout != EROFS_INODE_CHUNK_BASED) > + return erofs_map_blocks_flatmode (node, map); > + else > + return erofs_map_blocks_chunkmode (node, map); > +} > + > +static grub_err_t > +erofs_read_raw_data (grub_fshelp_node_t node, grub_uint8_t *buf, > grub_uint64_t size, > + grub_uint64_t offset, grub_uint64_t *bytes) > +{ > + struct grub_erofs_map_blocks map = {0}; > + grub_uint64_t cur; > + grub_err_t err; > + > + if (bytes) > + *bytes = 0; > + > + if (!node->inode_loaded) if (node->inode_loaded == false) ... and please fix similar things everywhere... > + { > + err = erofs_read_inode (node->data, node); > + if (err != GRUB_ERR_NONE) > + return err; > + } > + > + cur = offset; > + while (cur < offset + size) > + { > + grub_uint8_t *const estart = buf + cur - offset; > + grub_uint64_t eend, moff = 0; > + > + map.m_la = cur; > + err = erofs_map_blocks (node, &map); > + if (err != GRUB_ERR_NONE) > + return err; > + > + if (grub_add(map.m_la, map.m_llen, &eend)) > + return grub_error (GRUB_ERR_OUT_OF_RANGE, "overflow is detected"); > + > + eend = grub_min (eend, offset + size); > + if (!(map.m_flags & EROFS_MAP_MAPPED)) > + { > + if (!map.m_llen) > + { > + /* reached EOF */ > + grub_memset (estart, 0, offset + size - cur); > + cur = offset + size; > + continue; > + } > + > + /* Hole */ > + grub_memset (estart, 0, eend - cur); > + if (bytes) > + *bytes += eend - cur; > + cur = eend; > + continue; > + } > + > + if (cur > map.m_la) > + { > + moff = cur - map.m_la; > + map.m_la = cur; > + } > + > + err = grub_disk_read (node->data->disk, > + (map.m_pa + moff) >> GRUB_DISK_SECTOR_BITS, > + (map.m_pa + moff) & (GRUB_DISK_SECTOR_SIZE - 1), > + eend - map.m_la, estart); > + if (err != GRUB_ERR_NONE) > + return err; > + > + if (bytes) > + *bytes += eend - map.m_la; > + > + cur = eend; > + } > + > + return GRUB_ERR_NONE; > +} > + > +static int > +erofs_iterate_dir (grub_fshelp_node_t dir, grub_fshelp_iterate_dir_hook_t > hook, > + void *hook_data) > +{ > + grub_uint64_t offset = 0, file_size; > + grub_uint32_t blocksz = erofs_blocksz (dir->data); > + grub_uint8_t *buf; > + grub_err_t err; > + > + if (!dir->inode_loaded) Ditto. > + { > + err = erofs_read_inode (dir->data, dir); > + if (err != GRUB_ERR_NONE) > + return 0; > + } > + > + file_size = erofs_inode_file_size (dir); > + buf = grub_malloc (blocksz); > + if (!buf) if (buf == NULL) I know your version works but it is less readable. Please fix similar checks everywhere... > + return 0; > + > + while (offset < file_size) > + { > + grub_uint64_t maxsize = grub_min (blocksz, file_size - offset); > + struct grub_erofs_dirent *de = (void *) buf, *end; > + grub_uint16_t nameoff; > + > + err = erofs_read_raw_data (dir, buf, maxsize, offset, NULL); > + if (err != GRUB_ERR_NONE) > + goto not_found; > + > + nameoff = grub_le_to_cpu16 (de->nameoff); > + 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); > + goto not_found; > + } > + > + end = (struct grub_erofs_dirent *) ((grub_uint8_t *) de + nameoff); > + while (de < end) > + { > + struct grub_fshelp_node *fdiro; > + enum grub_fshelp_filetype type; > + char filename[EROFS_NAME_LEN + 1]; > + grub_size_t de_namelen; > + const char *de_name; > + > + fdiro = grub_malloc (sizeof (struct grub_fshelp_node)); > + if (!fdiro) Ditto... Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel