From: Harshad Shirwadkar <[email protected]>
This patch adds the following new APIs:
Count the total number of blocks occupied by inode including
intermediate extent tree nodes.
extern blk64_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
struct ext2_inode *inode);
Convert ext3_extent to ext2fs_extent.
extern void ext2fs_convert_extent(struct ext2fs_extent *to,
struct ext3_extent *from);
Signed-off-by: Harshad Shirwadkar <[email protected]>
---
lib/ext2fs/ext2fs.h | 4 +++
lib/ext2fs/extent.c | 63 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 67 insertions(+)
diff --git a/lib/ext2fs/ext2fs.h b/lib/ext2fs/ext2fs.h
index ec841006..fdcb28f6 100644
--- a/lib/ext2fs/ext2fs.h
+++ b/lib/ext2fs/ext2fs.h
@@ -1332,6 +1332,10 @@ extern errcode_t ext2fs_extent_fix_parents(ext2_extent_handle_t handle);
extern size_t ext2fs_max_extent_depth(ext2_extent_handle_t handle);
extern errcode_t ext2fs_fix_extents_checksums(ext2_filsys fs, ext2_ino_t ino,
struct ext2_inode *inode);
+extern blk64_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
+ struct ext2_inode *inode);
+extern errcode_t ext2fs_decode_extent(struct ext2fs_extent *to, void *from,
+ int len);
/* fallocate.c */
#define EXT2_FALLOCATE_ZERO_BLOCKS (0x1)
diff --git a/lib/ext2fs/extent.c b/lib/ext2fs/extent.c
index ac3dbfec..8d5fc1ab 100644
--- a/lib/ext2fs/extent.c
+++ b/lib/ext2fs/extent.c
@@ -1785,6 +1785,69 @@ out:
return errcode;
}
+errcode_t ext2fs_decode_extent(struct ext2fs_extent *to, void *addr, int len)
+{
+ struct ext3_extent *from = (struct ext3_extent *)addr;
+
+ if (len != sizeof(struct ext3_extent))
+ return EXT2_ET_INVALID_ARGUMENT;
+
+ to->e_pblk = ext2fs_le32_to_cpu(from->ee_start) +
+ ((__u64) ext2fs_le16_to_cpu(from->ee_start_hi)
+ << 32);
+ to->e_lblk = ext2fs_le32_to_cpu(from->ee_block);
+ to->e_len = ext2fs_le16_to_cpu(from->ee_len);
+ to->e_flags |= EXT2_EXTENT_FLAGS_LEAF;
+ if (to->e_len > EXT_INIT_MAX_LEN) {
+ to->e_len -= EXT_INIT_MAX_LEN;
+ to->e_flags |= EXT2_EXTENT_FLAGS_UNINIT;
+ }
+
+ return 0;
+}
+
+blk64_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
+ struct ext2_inode *inode)
+{
+ ext2_extent_handle_t handle;
+ struct ext2fs_extent extent;
+ errcode_t errcode;
+ int i;
+ blk64_t blkcount = 0;
+ blk64_t *intermediate_nodes;
+
+ errcode = ext2fs_extent_open2(fs, ino, inode, &handle);
+ if (errcode)
+ goto out;
+
+ errcode = ext2fs_extent_get(handle, EXT2_EXTENT_ROOT, &extent);
+ if (errcode)
+ goto out;
+
+ ext2fs_get_array(handle->max_depth, sizeof(blk64_t),
+ &intermediate_nodes);
+ blkcount = handle->level;
+ while (!errcode) {
+ if (extent.e_flags & EXT2_EXTENT_FLAGS_LEAF) {
+ blkcount += extent.e_len;
+ for (i = 0; i < handle->level; i++) {
+ if (intermediate_nodes[i] !=
+ handle->path[i].end_blk) {
+ blkcount++;
+ intermediate_nodes[i] =
+ handle->path[i].end_blk;
+ }
+ }
+ }
+ errcode = ext2fs_extent_get(handle, EXT2_EXTENT_NEXT, &extent);
+ }
+ ext2fs_free_mem(&intermediate_nodes);
+out:
+ ext2fs_extent_free(handle);
+
+ return blkcount;
+}
+
#ifdef DEBUG
/*
* Override debugfs's prompt
--
2.30.0.284.gd98b1dd5eaa7-goog
On Wed, Jan 20, 2021 at 01:26:32PM -0800, Harshad Shirwadkar wrote:
> From: Harshad Shirwadkar <[email protected]>
>
> This patch adds the following new APIs:
>
> Count the total number of blocks occupied by inode including
> intermediate extent tree nodes.
> extern blk64_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
> struct ext2_inode *inode);
I wonder if this should be something like this instead:
extern errcode_t ext2fs_count_blocks(ext2_filsys fs, ext2_ino_t ino,
struct ext2_inode *inode, blk64_t *ret_count);
The problem is that ext2fs_count_blocks() calls a whole series of
ext2fs functions which could return errors:
> + errcode = ext2fs_extent_open2(fs, ino, inode, &handle);
> + if (errcode)
> + goto out;
> +
> + errcode = ext2fs_extent_get(handle, EXT2_EXTENT_ROOT, &extent);
> + if (errcode)
> + goto out;
... and any of these functions could return an error. So we need to
make sure errors are faithfully returned to the caller and handled
correctly, instead of just having ext2fs_count_blocks returning a
value of 0.
I then started taking a look at the users of ext2fs_count_blocks() in
e2fsck, and I ran into more concerns. One of the problems here is
that some of these functions get called by kernel code --- and kernel
code has a different error handling convetion of negative errno's.
And in some cases, I see we are doing this:
static int ext4_fc_handle_inode(e2fsck_t ctx, struct ext4_fc_tl *tl)
{
...
ret = ext2fs_read_inode_full(ctx->fs, ino, (struct ext2_inode *)inode,
inode_len);
if (ret)
goto out;
...
out:
ext2fs_free_mem(&inode);
return ret;
}
The problem here is that ext2fs_read_inode_full() returns an
errcode_t, and this is getting cast to an int and returned as if it
were a kernel error code.
Also note that ext4_fc_replay() can return 0 or 1:
#define JBD2_FC_REPLAY_STOP 0
#define JBD2_FC_REPLAY_CONTINUE 1
Fortunately, none of the functions that ext4_fc_*() call seem to be
ones which could return in an ext2fs library returning EPERM (which is
errno 1), but you see the potential risks of conflating an errcode_t
and kernel negative errno convention.
This is going to be a bit tricky to deal with, since an errcode_t can
be a errno code, but it can also be one of the codes defined in
lib/ext2fs/ext2_err.et, which get translated to numbers like:
#define EXT2_ET_DIR_CORRUPTED (2133571363L)
#define EXT2_ET_SHORT_READ (2133571364L)
#define EXT2_ET_SHORT_WRITE (2133571365L)
(See lib/ext2fs/ext2_err.h in the build directory of e2fsprogs and the
com_err library found in lib/et.)
So what we may need to do is to create a function which does a simple
mapping of errcode_t values to negative errno's. It doesn't need to
be exact; in fact, a first pass might just map all errcode_t's greater
than 256 to something like -EFAULT, and all normal errno's to -errno.
We might also want to have it print a diagnistic message to stderr
that prints error_message(retval) was encoutered in function __func__
at line __LINE__. Hopefully in actual practice they won't happen
(unless a malicious attacker is feeding us a fuzzed file sytem), but
if it does, it would be good if there is a useful message so we can
actually debug what happened.
- Ted