[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] fs/ext2.c: fix handling of missing sparse extent leafs
From: |
Daniel Kiper |
Subject: |
Re: [PATCH] fs/ext2.c: fix handling of missing sparse extent leafs |
Date: |
Tue, 28 Sep 2021 16:03:22 +0200 |
User-agent: |
NeoMutt/20170113 (1.7.2) |
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 <krzysztof.nowicki@nokia.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com> 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