Generally looks good. Few comments. I have missed part of discussion, so point me to relevant parts of it if I miss something that's already been discussed
+ struct grub_erofs_super *sb = &node->data->sb;
+
+ return (grub_le_to_cpu32 (sb->meta_blkaddr) << sb->log2_blksz) + (node->ino << EROFS_ISLOTBITS);
Here you have an overflow. You shift 32-bit value and so it's shifted as 32-bit and only then it's implicitly cast to 64-bit. You need an explicit cast before shift but after bytesesp for meta_blkaddr.
+}
+
+static grub_err_t
+erofs_read_inode (struct grub_erofs_data *data, grub_fshelp_node_t node)
+{
+ struct grub_erofs_inode_compact *dic;
+ grub_err_t err;
+ grub_uint16_t i_format;
+ grub_uint64_t addr = erofs_iloc (node);
+
+ dic = (struct grub_erofs_inode_compact *) &node->inode;
Why not use union member here?
+static grub_uint64_t
+erofs_inode_file_size (grub_fshelp_node_t node)
+{
+ struct grub_erofs_inode_compact *dic = (struct grub_erofs_inode_compact *) &node->inode;
Ditto
+ grub_uint32_t blocksz = erofs_blocksz (node->data);
+
+ file_size = erofs_inode_file_size (node);
+ nblocks = (file_size + blocksz - 1) >> node->data->sb.log2_blksz;
+ lastblk = nblocks - tailendpacking;
+
+ map->m_flags = EROFS_MAP_MAPPED;
+
+ if (map->m_la < (lastblk * blocksz))
Is this multiplication checked somewhere?
+ {
+ if (grub_mul (grub_le_to_cpu32 (node->inode.i_u.raw_blkaddr), blocksz, &map->m_pa) ||
Missing cast to 64-bit.
+ grub_add (map->m_pa, map->m_la, &map->m_pa))
+ return GRUB_ERR_OUT_OF_RANGE;
It looks like you miss a grub_error
+ if (grub_sub (lastblk * blocksz, map->m_la, &map->m_plen))
Ditto
+ return GRUB_ERR_OUT_OF_RANGE;
Ditto. I stop reviewing this particular construct
+ }
+ 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_ERR_OUT_OF_RANGE;
+ if (grub_sub (file_size, map->m_la, &map->m_plen))
+ return GRUB_ERR_OUT_OF_RANGE;
+
+ if (((map->m_pa % blocksz) + map->m_plen) > blocksz)
Addition not checked. If there is a reason it can't overflow even if FS is maliciously corrupted, please add it in the comment
+ pos = ALIGN_UP (pos, unit);
Potential overflow if pos is only slightly under it's limit.
+ if (grub_add (pos, chunknr * unit, &pos))
Does this multiplication need verification?
+
+ if (chunk_format & EROFS_CHUNK_FORMAT_INDEXES)
+ {
+ struct grub_erofs_inode_chunk_index idx;
+ grub_uint32_t blkaddr;
I recommend making it 64-bit to avoid casts
+ {
+ map->m_pa = blkaddr << node->data->sb.log2_blksz;
Overflow again.
+ {
+ map->m_pa = blkaddr << node->data->sb.log2_blksz;
Overflow
+
+ eend = grub_min (offset + size, map.m_la + map.m_llen);
Where is it checked that m_la+m_llen don't overflow? It can be checked when you read in the map it should be trimmed or error-out
+ file_size = erofs_inode_file_size (dir);
+ buf = grub_malloc (blocksz);
+ if (!buf)
+ {
+ grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
No need for grub_error: malloc already does it.
+ else
+ de_namelen = grub_le_to_cpu16 (de[1].nameoff) - nameoff;
This needs a check that it's not negative
+
+ err = grub_disk_read (disk, EROFS_SUPER_OFFSET >> GRUB_DISK_SECTOR_BITS, 0,
+ sizeof (sb), &sb);
+ if (grub_errno == GRUB_ERR_OUT_OF_RANGE)
+ grub_error (GRUB_ERR_BAD_FS, "not a valid erofs filesystem");
OUT_OF_RANGE is already treated the same asBAD_FS in context of opening a file
+ if (!data)
+ {
+ grub_error (GRUB_ERR_OUT_OF_MEMORY, "out of memory");
Ditto
+static grub_ssize_t
+grub_erofs_read (grub_file_t file, char *buf, grub_size_t len)
+{
+ struct grub_erofs_data *data = "">
+ struct grub_fshelp_node *inode = &data->inode;
+ grub_off_t off = file->offset;
+ grub_uint64_t ret = 0, file_size;
+ grub_err_t err;
+
+ if (!inode->inode_loaded)
+ {
+ err = erofs_read_inode (data, inode);
+ if (err != GRUB_ERR_NONE)
+ {
+ grub_error (GRUB_ERR_IO, "cannot read @ inode %" PRIuGRUB_UINT64_T, inode->ino);
Any reason to transform error? This could be something else than I/O error. Why not just propagate it?
+
+ err = erofs_read_raw_data (inode, (grub_uint8_t *) buf, len, off, &ret);
+ if (err != GRUB_ERR_NONE)
+ {
+ grub_error (GRUB_ERR_IO, "cannot read file @ inode %" PRIuGRUB_UINT64_T, inode->ino);
Again, I propose to just propagate underlying error.
+grub_size_t
+grub_strnlen (const char *s, grub_size_t n)
Already said in a different email