[PATCH v2 4/6] fs: ext4: check block numbers before releasing them

[Ana Bobillo <anabobillo3@gmail.com>]

From: Ana Bobillo <anabobillo3@gmail.com>

When ext4fs_write() replaces an existing file, ext4fs_delete_file()
releases the file's blocks: the data blocks found by
read_allocated_block() and, for a file using indirect blocks, the
blocks listed in its indirect blocks. Each block number comes from the
inode or from an indirect block on disk and is turned into a block
group index used for fs->blk_bmaps[] and ext4fs_get_group_descriptor()
without any check. A block number past the end of the filesystem
makes ext4fs_reset_block_bmap() and ext4fs_bg_free_blocks_inc() write
outside the bitmap array and the group descriptor table.

Add ext4fs_blk_to_group(), which computes the block group of a block
as Linux ext4_get_group_no_and_offset() does, from the first data
block. As Linux does, it rejects a block that does not lie after the
first data block, which holds the superblock, and before the end of the
filesystem. Use it wherever the delete path computes the block group of
a block, in place of the open-coded division whose special case for
1 KiB blocks stood in for a first data block of 1.

Make the delete_*_indirect_block() helpers return an error, so that
ext4fs_delete_file() stops rather than releasing blocks of a corrupt
inode: -EINVAL for an invalid block number, -ENOMEM when out of
memory and -EIO when a read or the journal fails. That includes the
reads of the double and triple indirect blocks and of the blocks the
triple indirect block lists, which used to be unchecked: when one
failed, the zeroed buffer ended the walk at once, so the indirect block
was released and the blocks listed in it were leaked.

Signed-off-by: Ana Bobillo <anabobillo3@gmail.com>
---
Changes in v2:
- Move ext4fs_blk_to_group() to ext4_common.c and compute the group
  from the first data block, as Linux does, without the 1 KiB case
- Check the reads of the double and triple indirect blocks and of the
  blocks the triple indirect block lists
- Return -EIO for read and journal errors, -ENOMEM when out of memory

 fs/ext4/ext4_common.c |  35 +++++++++++++
 fs/ext4/ext4_common.h |   1 +
 fs/ext4/ext4_write.c  | 115 ++++++++++++++++++++++--------------------
 3 files changed, 97 insertions(+), 54 deletions(-)

diff --git a/fs/ext4/ext4_common.c b/fs/ext4/ext4_common.c
index cff112fb99b..650597873f8 100644
--- a/fs/ext4/ext4_common.c
+++ b/fs/ext4/ext4_common.c
@@ -963,6 +963,41 @@ int ext4fs_filename_unlink(char *filename)
 	return -1;
 }
 
+/**
+ * ext4fs_blk_to_group() - get the block group of a block
+ *
+ * The block number may come from disk, so check that it lies after the
+ * first data block, which holds the superblock, and before the end of the
+ * filesystem, as Linux does, before it is used to index the block bitmaps
+ * and group descriptors. The group is computed as Linux
+ * ext4_get_group_no_and_offset() does.
+ *
+ * @fs:		filesystem
+ * @blknr:	block number
+ * Return:	block group of @blknr, or -EINVAL if it is not a valid block
+ */
+int ext4fs_blk_to_group(struct ext_filesystem *fs, uint64_t blknr)
+{
+	struct ext2_sblock *sblock = &ext4fs_root->sblock;
+	uint32_t first_data_block = le32_to_cpu(sblock->first_data_block);
+	uint32_t bg_idx;
+
+	if (blknr <= first_data_block ||
+	    blknr >= le32_to_cpu(sblock->total_blocks))
+		goto err;
+
+	/* blknr is below total_blocks, so it fits in 32 bits */
+	bg_idx = ((uint32_t)blknr - first_data_block) /
+		 le32_to_cpu(sblock->blocks_per_group);
+	if (bg_idx >= fs->no_blkgrp)
+		goto err;
+
+	return bg_idx;
+err:
+	printf("Invalid block number %llu\n", (unsigned long long)blknr);
+	return -EINVAL;
+}
+
 uint32_t ext4fs_get_new_blk_no(void)
 {
 	short i;
diff --git a/fs/ext4/ext4_common.h b/fs/ext4/ext4_common.h
index 346752092b0..eecc11d9995 100644
--- a/fs/ext4/ext4_common.h
+++ b/fs/ext4/ext4_common.h
@@ -63,6 +63,7 @@ uint32_t ext4fs_div_roundup(uint32_t size, uint32_t n);
 uint16_t ext4fs_checksum_update(unsigned int i);
 int ext4fs_get_parent_inode_num(const char *dirname, char *dname, int flags);
 int ext4fs_update_parent_dentry(char *filename, int file_type);
+int ext4fs_blk_to_group(struct ext_filesystem *fs, uint64_t blknr);
 uint32_t ext4fs_get_new_blk_no(void);
 int ext4fs_get_new_inode_no(void);
 void ext4fs_reset_block_bmap(long int blockno, unsigned char *buffer,
diff --git a/fs/ext4/ext4_write.c b/fs/ext4/ext4_write.c
index 5f185387030..ea7843775b3 100644
--- a/fs/ext4/ext4_write.c
+++ b/fs/ext4/ext4_write.c
@@ -140,31 +140,29 @@ fail:
 	return -1;
 }
 
-static void delete_single_indirect_block(struct ext2_inode *inode)
+static int delete_single_indirect_block(struct ext2_inode *inode)
 {
 	struct ext2_block_group *bgd = NULL;
 	static int prev_bg_bmap_idx = -1;
 	uint32_t blknr;
-	int remainder;
 	int bg_idx;
 	int status;
-	uint32_t blk_per_grp = le32_to_cpu(ext4fs_root->sblock.blocks_per_group);
+	int ret = -EIO;
 	struct ext_filesystem *fs = get_fs();
 	char *journal_buffer = zalloc(fs->blksz);
 	if (!journal_buffer) {
 		printf("No memory\n");
-		return;
+		return -ENOMEM;
 	}
 
 	/* deleting the single indirect block associated with inode */
 	if (inode->b.blocks.indir_block != 0) {
 		blknr = le32_to_cpu(inode->b.blocks.indir_block);
 		debug("SIPB releasing %u\n", blknr);
-		bg_idx = blknr / blk_per_grp;
-		if (fs->blksz == 1024) {
-			remainder = blknr % blk_per_grp;
-			if (!remainder)
-				bg_idx--;
+		bg_idx = ext4fs_blk_to_group(fs, blknr);
+		if (bg_idx < 0) {
+			ret = bg_idx;
+			goto fail;
 		}
 		ext4fs_reset_block_bmap(blknr, fs->blk_bmaps[bg_idx], bg_idx);
 		/* get  block group descriptor table */
@@ -184,19 +182,21 @@ static void delete_single_indirect_block(struct ext2_inode *inode)
 			prev_bg_bmap_idx = bg_idx;
 		}
 	}
+	ret = 0;
 fail:
 	free(journal_buffer);
+
+	return ret;
 }
 
-static void delete_double_indirect_block(struct ext2_inode *inode)
+static int delete_double_indirect_block(struct ext2_inode *inode)
 {
 	int i;
 	short status;
 	static int prev_bg_bmap_idx = -1;
 	uint32_t blknr;
-	int remainder;
 	int bg_idx;
-	uint32_t blk_per_grp = le32_to_cpu(ext4fs_root->sblock.blocks_per_group);
+	int ret = -EIO;
 	__le32 *di_buffer = NULL;
 	void *dib_start_addr = NULL;
 	struct ext2_block_group *bgd = NULL;
@@ -204,29 +204,31 @@ static void delete_double_indirect_block(struct ext2_inode *inode)
 	char *journal_buffer = zalloc(fs->blksz);
 	if (!journal_buffer) {
 		printf("No memory\n");
-		return;
+		return -ENOMEM;
 	}
 
 	if (inode->b.blocks.double_indir_block != 0) {
 		di_buffer = zalloc(fs->blksz);
 		if (!di_buffer) {
 			printf("No memory\n");
+			ret = -ENOMEM;
 			goto fail;
 		}
 		dib_start_addr = di_buffer;
 		blknr = le32_to_cpu(inode->b.blocks.double_indir_block);
 		status = ext4fs_devread((lbaint_t)blknr * fs->sect_perblk, 0,
 					fs->blksz, (char *)di_buffer);
+		if (status == 0)
+			goto fail;
 		for (i = 0; i < fs->blksz / sizeof(int); i++) {
 			if (*di_buffer == 0)
 				break;
 
 			debug("DICB releasing %u\n", *di_buffer);
-			bg_idx = le32_to_cpu(*di_buffer) / blk_per_grp;
-			if (fs->blksz == 1024) {
-				remainder = le32_to_cpu(*di_buffer) % blk_per_grp;
-				if (!remainder)
-					bg_idx--;
+			bg_idx = ext4fs_blk_to_group(fs, le32_to_cpu(*di_buffer));
+			if (bg_idx < 0) {
+				ret = bg_idx;
+				goto fail;
 			}
 			/* get  block group descriptor table */
 			bgd = ext4fs_get_group_descriptor(fs, bg_idx);
@@ -255,11 +257,10 @@ static void delete_double_indirect_block(struct ext2_inode *inode)
 
 		/* removing the parent double indirect block */
 		blknr = le32_to_cpu(inode->b.blocks.double_indir_block);
-		bg_idx = blknr / blk_per_grp;
-		if (fs->blksz == 1024) {
-			remainder = blknr % blk_per_grp;
-			if (!remainder)
-				bg_idx--;
+		bg_idx = ext4fs_blk_to_group(fs, blknr);
+		if (bg_idx < 0) {
+			ret = bg_idx;
+			goto fail;
 		}
 		/* get  block group descriptor table */
 		bgd = ext4fs_get_group_descriptor(fs, bg_idx);
@@ -280,20 +281,22 @@ static void delete_double_indirect_block(struct ext2_inode *inode)
 		}
 		debug("DIPB releasing %d\n", blknr);
 	}
+	ret = 0;
 fail:
 	free(dib_start_addr);
 	free(journal_buffer);
+
+	return ret;
 }
 
-static void delete_triple_indirect_block(struct ext2_inode *inode)
+static int delete_triple_indirect_block(struct ext2_inode *inode)
 {
 	int i, j;
 	short status;
 	static int prev_bg_bmap_idx = -1;
 	uint32_t blknr;
-	int remainder;
 	int bg_idx;
-	uint32_t blk_per_grp = le32_to_cpu(ext4fs_root->sblock.blocks_per_group);
+	int ret = -EIO;
 	__le32 *tigp_buffer = NULL;
 	void *tib_start_addr = NULL;
 	__le32 *tip_buffer = NULL;
@@ -303,39 +306,46 @@ static void delete_triple_indirect_block(struct ext2_inode *inode)
 	char *journal_buffer = zalloc(fs->blksz);
 	if (!journal_buffer) {
 		printf("No memory\n");
-		return;
+		return -ENOMEM;
 	}
 
 	if (inode->b.blocks.triple_indir_block != 0) {
 		tigp_buffer = zalloc(fs->blksz);
 		if (!tigp_buffer) {
 			printf("No memory\n");
+			ret = -ENOMEM;
 			goto fail;
 		}
 		tib_start_addr = tigp_buffer;
 		blknr = le32_to_cpu(inode->b.blocks.triple_indir_block);
 		status = ext4fs_devread((lbaint_t)blknr * fs->sect_perblk, 0,
 					fs->blksz, (char *)tigp_buffer);
+		if (status == 0)
+			goto fail;
 		for (i = 0; i < fs->blksz / sizeof(int); i++) {
 			if (*tigp_buffer == 0)
 				break;
 			debug("tigp buffer releasing %u\n", *tigp_buffer);
 
 			tip_buffer = zalloc(fs->blksz);
-			if (!tip_buffer)
+			if (!tip_buffer) {
+				ret = -ENOMEM;
 				goto fail;
+			}
 			tipb_start_addr = tip_buffer;
 			status = ext4fs_devread((lbaint_t)le32_to_cpu(*tigp_buffer) *
 						fs->sect_perblk, 0, fs->blksz,
 						(char *)tip_buffer);
+			if (status == 0)
+				goto fail;
 			for (j = 0; j < fs->blksz / sizeof(int); j++) {
 				if (le32_to_cpu(*tip_buffer) == 0)
 					break;
-				bg_idx = le32_to_cpu(*tip_buffer) / blk_per_grp;
-				if (fs->blksz == 1024) {
-					remainder = le32_to_cpu(*tip_buffer) % blk_per_grp;
-					if (!remainder)
-						bg_idx--;
+				bg_idx = ext4fs_blk_to_group(fs,
+							     le32_to_cpu(*tip_buffer));
+				if (bg_idx < 0) {
+					ret = bg_idx;
+					goto fail;
 				}
 
 				ext4fs_reset_block_bmap(le32_to_cpu(*tip_buffer),
@@ -373,11 +383,10 @@ static void delete_triple_indirect_block(struct ext2_inode *inode)
 			 * removing the grand parent blocks
 			 * which is connected to inode
 			 */
-			bg_idx = le32_to_cpu(*tigp_buffer) / blk_per_grp;
-			if (fs->blksz == 1024) {
-				remainder = le32_to_cpu(*tigp_buffer) % blk_per_grp;
-				if (!remainder)
-					bg_idx--;
+			bg_idx = ext4fs_blk_to_group(fs, le32_to_cpu(*tigp_buffer));
+			if (bg_idx < 0) {
+				ret = bg_idx;
+				goto fail;
 			}
 			ext4fs_reset_block_bmap(le32_to_cpu(*tigp_buffer),
 						fs->blk_bmaps[bg_idx], bg_idx);
@@ -408,11 +417,10 @@ static void delete_triple_indirect_block(struct ext2_inode *inode)
 
 		/* removing the grand parent triple indirect block */
 		blknr = le32_to_cpu(inode->b.blocks.triple_indir_block);
-		bg_idx = blknr / blk_per_grp;
-		if (fs->blksz == 1024) {
-			remainder = blknr % blk_per_grp;
-			if (!remainder)
-				bg_idx--;
+		bg_idx = ext4fs_blk_to_group(fs, blknr);
+		if (bg_idx < 0) {
+			ret = bg_idx;
+			goto fail;
 		}
 		ext4fs_reset_block_bmap(blknr, fs->blk_bmaps[bg_idx], bg_idx);
 		/* get  block group descriptor table */
@@ -433,10 +441,13 @@ static void delete_triple_indirect_block(struct ext2_inode *inode)
 		}
 		debug("tigp buffer itself releasing %d\n", blknr);
 	}
+	ret = 0;
 fail:
 	free(tib_start_addr);
 	free(tipb_start_addr);
 	free(journal_buffer);
+
+	return ret;
 }
 
 static int ext4fs_delete_file(int inodeno)
@@ -444,7 +455,6 @@ static int ext4fs_delete_file(int inodeno)
 	struct ext2_inode inode;
 	short status;
 	int i;
-	int remainder;
 	long int blknr;
 	int bg_idx;
 	int ibmap_idx;
@@ -456,7 +466,6 @@ static int ext4fs_delete_file(int inodeno)
 	unsigned int inodes_per_block;
 	uint32_t blkno;
 	unsigned int blkoff;
-	uint32_t blk_per_grp = le32_to_cpu(ext4fs_root->sblock.blocks_per_group);
 	uint32_t inode_per_grp = le32_to_cpu(ext4fs_root->sblock.inodes_per_group);
 	struct ext2_inode *inode_buffer = NULL;
 	struct ext2_block_group *bgd = NULL;
@@ -489,9 +498,10 @@ static int ext4fs_delete_file(int inodeno)
 				inode.b.blocks.dir_blocks;
 		debug("del: dep=%d entries=%d\n", eh->eh_depth, eh->eh_entries);
 	} else {
-		delete_single_indirect_block(&inode);
-		delete_double_indirect_block(&inode);
-		delete_triple_indirect_block(&inode);
+		if (delete_single_indirect_block(&inode) ||
+		    delete_double_indirect_block(&inode) ||
+		    delete_triple_indirect_block(&inode))
+			goto fail;
 	}
 
 	/* release data blocks */
@@ -501,12 +511,9 @@ static int ext4fs_delete_file(int inodeno)
 			continue;
 		if (blknr < 0)
 			goto fail;
-		bg_idx = blknr / blk_per_grp;
-		if (fs->blksz == 1024) {
-			remainder = blknr % blk_per_grp;
-			if (!remainder)
-				bg_idx--;
-		}
+		bg_idx = ext4fs_blk_to_group(fs, blknr);
+		if (bg_idx < 0)
+			goto fail;
 		ext4fs_reset_block_bmap(blknr, fs->blk_bmaps[bg_idx],
 					bg_idx);
 		debug("EXT4 Block releasing %ld: %d\n", blknr, bg_idx);