On Fri, Sep 10, 2021 at 09:54:31AM +0200, Krzysztof Nowicki wrote: > When a file on ext4 is stored as sparse the data belonging to > zero-filled blocks is not written to storage and the extent map is > missing entries for these blocks. Such case can happen both for depth > 0 extents (leafs) as well as higher-level tables. > > Consider a scenario of a file which has a zero-filled > beginning (e.g. ISO image). In such case real data starts at block > 8. If such a file is stored using 2-level extent structure the extent > list in the inode will be depth 1 and will have an entry to a depth > 0 (leaf) extent header for blocks 8-n. > > Unfortunately existing GRUB2 ext2 driver is only able to handle > missing entries in leaf extent tables, for which the > grub_ext2_read_block() function returns 0. In case the whole leaf > extent list is missing for a block the function fails with "invalid > extent" error. > > The fix for this problem relies on the grub_ext4_find_leaf() helper > function to distinguish two error cases: missing extent and error > walking through the extent tree. The existing error message is raised > only for the latter case, while for the missing leaf extent zero is > returned from grub_ext2_read_block() indicating a sparse block. > > Signed-off-by: Krzysztof Nowicki <[email protected]>
Reviewed-by: Daniel Kiper <[email protected]> except two nitpicks below which I fix before committing... > --- > grub-core/fs/ext2.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/grub-core/fs/ext2.c b/grub-core/fs/ext2.c > index ac33bcd68..884ffaa97 100644 > --- a/grub-core/fs/ext2.c > +++ b/grub-core/fs/ext2.c > @@ -407,13 +407,15 @@ grub_ext2_blockgroup (struct grub_ext2_data *data, > grub_uint64_t group, > sizeof (struct grub_ext2_block_group), blkgrp); > } > > -static struct grub_ext4_extent_header * > +static grub_err_t > grub_ext4_find_leaf (struct grub_ext2_data *data, > struct grub_ext4_extent_header *ext_block, > - grub_uint32_t fileblock) > + grub_uint32_t fileblock, > + struct grub_ext4_extent_header **leaf) > { > struct grub_ext4_extent_idx *index; > void *buf = NULL; > + *leaf = NULL; > > while (1) > { > @@ -426,7 +428,10 @@ grub_ext4_find_leaf (struct grub_ext2_data *data, > goto fail; > > if (ext_block->depth == 0) > - return ext_block; > + { > + *leaf = ext_block; > + return GRUB_ERR_NONE; > + } > > for (i = 0; i < grub_le_to_cpu16 (ext_block->entries); i++) > { > @@ -435,7 +440,10 @@ grub_ext4_find_leaf (struct grub_ext2_data *data, > } > > if (--i < 0) > - goto fail; > + { > + grub_free (buf); > + return GRUB_ERR_NONE; > + } > > block = grub_le_to_cpu16 (index[i].leaf_hi); > block = (block << 32) | grub_le_to_cpu32 (index[i].leaf); > @@ -452,7 +460,7 @@ grub_ext4_find_leaf (struct grub_ext2_data *data, > } > fail: > grub_free (buf); > - return 0; > + return GRUB_ERR_BAD_FS; > } > > static grub_disk_addr_t > @@ -474,13 +482,16 @@ grub_ext2_read_block (grub_fshelp_node_t node, > grub_disk_addr_t fileblock) > int i; > grub_disk_addr_t ret; > > - leaf = grub_ext4_find_leaf (data, (struct grub_ext4_extent_header *) > inode->blocks.dir_blocks, fileblock); > - if (! leaf) > + if (grub_ext4_find_leaf (data, (struct grub_ext4_extent_header *) > inode->blocks.dir_blocks, fileblock, &leaf)) if (grub_ext4_find_leaf (...) != GRUB_ERR_NONE) > { > grub_error (GRUB_ERR_BAD_FS, "invalid extent"); > return -1; > } > > + if (!leaf) if (leaf == NULL) > + /* Leaf for the given block is absent (i.e. sparse) */ > + return 0; > + > ext = (struct grub_ext4_extent *) (leaf + 1); > for (i = 0; i < grub_le_to_cpu16 (leaf->entries); i++) > { Daniel _______________________________________________ Grub-devel mailing list [email protected] https://lists.gnu.org/mailman/listinfo/grub-devel
