[PATCH 0/4] 4.17-rc1 stable tagged ext4 patches for 3.18.y

[PATCH 0/4] 4.17-rc1 stable tagged ext4 patches for 3.18.y

[Harsh Shandilya]

These are all the ext4 patches that were tagged for -stable and failed
to apply to 3.18.y.

Side note: Patch e15dc99dbb9c ("ALSA: pcm: Fix endless loop for XRUN recovery in OSS emulation")
which was tagged for -stable is not required on 3.18.y so I have skipped
the backport.

Theodore Ts'o (4):
  ext4: add validity checks for bitmap block numbers
  ext4: fail ext4_iget for root directory if unallocated
  ext4: don't allow r/w mounts if metadata blocks overlap the superblock
  ext4: force revalidation of directory pointer after seekdir(2)

 fs/ext4/balloc.c | 16 ++++++++++++++--
 fs/ext4/dir.c    |  8 +++++---
 fs/ext4/ialloc.c |  7 +++++++
 fs/ext4/inode.c  |  6 ++++++
 fs/ext4/super.c  |  6 ++++++
 5 files changed, 38 insertions(+), 5 deletions(-)

-- 
2.15.0.2308.g658a28aa74af

[PATCH 1/4] ext4: add validity checks for bitmap block numbers

[Harsh Shandilya]

From: Theodore Ts'o <tytso@mit.edu>

An privileged attacker can cause a crash by mounting a crafted ext4
image which triggers a out-of-bounds read in the function
ext4_valid_block_bitmap() in fs/ext4/balloc.c.

This issue has been assigned CVE-2018-1093.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199181
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1560782
Reported-by: Wen Xu <wen.xu@gatech.edu>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
---
 fs/ext4/balloc.c | 16 ++++++++++++++--
 fs/ext4/ialloc.c |  7 +++++++
 2 files changed, 21 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index cb3860817fed..47a3145f3531 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -340,20 +340,25 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
 	/* check whether block bitmap block number is set */
 	blk = ext4_block_bitmap(sb, desc);
 	offset = blk - group_first_block;
-	if (!ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
 		/* bad block bitmap */
 		return blk;
 
 	/* check whether the inode bitmap block number is set */
 	blk = ext4_inode_bitmap(sb, desc);
 	offset = blk - group_first_block;
-	if (!ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
 		/* bad block bitmap */
 		return blk;
 
 	/* check whether the inode table block number is set */
 	blk = ext4_inode_table(sb, desc);
 	offset = blk - group_first_block;
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= sb->s_blocksize)
+		return blk;
 	next_zero_bit = ext4_find_next_zero_bit(bh->b_data,
 			EXT4_B2C(sbi, offset + EXT4_SB(sb)->s_itb_per_group),
 			EXT4_B2C(sbi, offset));
@@ -416,6 +421,7 @@ struct buffer_head *
 ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 {
 	struct ext4_group_desc *desc;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct buffer_head *bh;
 	ext4_fsblk_t bitmap_blk;
 
@@ -423,6 +429,12 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 	if (!desc)
 		return NULL;
 	bitmap_blk = ext4_block_bitmap(sb, desc);
+	if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
+	    (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
+		ext4_error(sb, "Invalid block bitmap block %llu in "
+			   "block_group %u", bitmap_blk, block_group);
+		return ERR_PTR(-EFSCORRUPTED);
+	}
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
 		ext4_error(sb, "Cannot get buffer for block bitmap - "
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index 9f230e589ecc..dc1233cc00b2 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -123,6 +123,7 @@ static struct buffer_head *
 ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 {
 	struct ext4_group_desc *desc;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct buffer_head *bh = NULL;
 	ext4_fsblk_t bitmap_blk;
 	struct ext4_group_info *grp;
@@ -133,6 +134,12 @@ ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 		return NULL;
 
 	bitmap_blk = ext4_inode_bitmap(sb, desc);
+	if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
+	    (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
+		ext4_error(sb, "Invalid inode bitmap blk %llu in "
+			   "block_group %u", bitmap_blk, block_group);
+		return ERR_PTR(-EFSCORRUPTED);
+	}
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
 		ext4_error(sb, "Cannot read inode bitmap - "
-- 
2.15.0.2308.g658a28aa74af

[PATCH 2/4] ext4: fail ext4_iget for root directory if unallocated

[Harsh Shandilya]

From: Theodore Ts'o <tytso@mit.edu>

If the root directory has an i_links_count of zero, then when the file
system is mounted, then when ext4_fill_super() notices the problem and
tries to call iput() the root directory in the error return path,
ext4_evict_inode() will try to free the inode on disk, before all of
the file system structures are set up, and this will result in an OOPS
caused by a NULL pointer dereference.

This issue has been assigned CVE-2018-1092.

https://bugzilla.kernel.org/show_bug.cgi?id=199179
https://bugzilla.redhat.com/show_bug.cgi?id=1560777

Reported-by: Wen Xu <wen.xu@gatech.edu>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
---
 fs/ext4/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index c2434d72681e..8513ff40f328 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -3975,6 +3975,12 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 		goto bad_inode;
 	raw_inode = ext4_raw_inode(&iloc);
 
+	if ((ino == EXT4_ROOT_INO) && (raw_inode->i_links_count == 0)) {
+		EXT4_ERROR_INODE(inode, "root inode unallocated");
+		ret = -EFSCORRUPTED;
+		goto bad_inode;
+	}
+
 	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
 		ei->i_extra_isize = le16_to_cpu(raw_inode->i_extra_isize);
 		if (EXT4_GOOD_OLD_INODE_SIZE + ei->i_extra_isize >
-- 
2.15.0.2308.g658a28aa74af

[PATCH 3/4] ext4: don't allow r/w mounts if metadata blocks overlap the superblock

[Harsh Shandilya]

From: Theodore Ts'o <tytso@mit.edu>

If some metadata block, such as an allocation bitmap, overlaps the
superblock, it's very likely that if the file system is mounted
read/write, the results will not be pretty.  So disallow r/w mounts
for file systems corrupted in this particular way.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
---
 fs/ext4/super.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index cc0a2298099d..263a2f9802f8 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2093,6 +2093,8 @@ static int ext4_check_descriptors(struct super_block *sb,
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
 				 "Block bitmap for group %u overlaps "
 				 "superblock", i);
+			if (!sb_rdonly(sb))
+				return 0;
 		}
 		if (block_bitmap < first_block || block_bitmap > last_block) {
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
@@ -2105,6 +2107,8 @@ static int ext4_check_descriptors(struct super_block *sb,
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
 				 "Inode bitmap for group %u overlaps "
 				 "superblock", i);
+			if (!sb_rdonly(sb))
+				return 0;
 		}
 		if (inode_bitmap < first_block || inode_bitmap > last_block) {
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
@@ -2117,6 +2121,8 @@ static int ext4_check_descriptors(struct super_block *sb,
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
 				 "Inode table for group %u overlaps "
 				 "superblock", i);
+			if (!sb_rdonly(sb))
+				return 0;
 		}
 		if (inode_table < first_block ||
 		    inode_table + sbi->s_itb_per_group - 1 > last_block) {
-- 
2.15.0.2308.g658a28aa74af

[PATCH 4/4] ext4: force revalidation of directory pointer after seekdir(2)

[Harsh Shandilya]

From: Theodore Ts'o <tytso@mit.edu>

A malicious user could force the directory pointer to be in an invalid
spot by using seekdir(2).  Use the mechanism we already have to notice
if the directory has changed since the last time we called
ext4_readdir() to force a revalidation of the pointer.

Reported-by: syzbot+1236ce66f79263e8a862@syzkaller.appspotmail.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org

Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
---
 fs/ext4/dir.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index c24143ea9c08..99f72558b33a 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -320,13 +320,15 @@ static loff_t ext4_dir_llseek(struct file *file, loff_t offset, int whence)
 {
 	struct inode *inode = file->f_mapping->host;
 	int dx_dir = is_dx_dir(inode);
-	loff_t htree_max = ext4_get_htree_eof(file);
+	loff_t ret, htree_max = ext4_get_htree_eof(file);
 
 	if (likely(dx_dir))
-		return generic_file_llseek_size(file, offset, whence,
+		ret = generic_file_llseek_size(file, offset, whence,
 						    htree_max, htree_max);
 	else
-		return ext4_llseek(file, offset, whence);
+		ret = ext4_llseek(file, offset, whence);
+	file->f_version = inode_peek_iversion(inode) - 1;
+	return ret;
 }
 
 /*
-- 
2.15.0.2308.g658a28aa74af

Re: [PATCH 0/4] 4.17-rc1 stable tagged ext4 patches for 3.18.y

[Harsh Shandilya]

On 21 April 2018 3:56:08 AM IST, Harsh Shandilya <harsh@prjkt.io> wrote:
>These are all the ext4 patches that were tagged for -stable and failed
>to apply to 3.18.y.
>
>Side note: Patch e15dc99dbb9c ("ALSA: pcm: Fix endless loop for XRUN
>recovery in OSS emulation")
>which was tagged for -stable is not required on 3.18.y so I have
>skipped
>the backport.
Please ignore this for the time being, I accidentally mailed the wrong set of patches. I'll post fixed and rebased versions of all patches later today, need to sleep this 5AM fogginess off.

tired and stupid,
Harsh Shandilya, PRJKT Development LLC

[PATCH 0/3] 4.17-rc1 stable tagged ext4 patches for 3.18.y

[Harsh Shandilya]

I've dropped one of the initial 4 patches since it required
a dependency patch that was ~250 lines in size and not really
suitable for -stable.

Theodore Ts'o (3):
  ext4: add validity checks for bitmap block numbers
  ext4: fail ext4_iget for root directory if unallocated
  ext4: don't allow r/w mounts if metadata blocks overlap the superblock

 fs/ext4/balloc.c | 16 ++++++++++++++--
 fs/ext4/ialloc.c |  8 +++++++-
 fs/ext4/inode.c  |  6 ++++++
 fs/ext4/super.c  |  6 ++++++
 4 files changed, 33 insertions(+), 3 deletions(-)

-- 
2.15.0.2308.g658a28aa74af

[PATCH 1/3] ext4: add validity checks for bitmap block numbers

[Harsh Shandilya]

From: Theodore Ts'o <tytso@mit.edu>

Commit 7dac4a1726a9c64a517d595c40e95e2d0d135f6f upstream.

An privileged attacker can cause a crash by mounting a crafted ext4
image which triggers a out-of-bounds read in the function
ext4_valid_block_bitmap() in fs/ext4/balloc.c.

This issue has been assigned CVE-2018-1093.

Backport notes:
3.18.y is missing commit 6a797d273783 ("ext4: call out CRC and corruption errors with specific error codes")
so the EFSCORRUPTED label doesn't exist. Replaced
all instances of EFSCORRUPTED with EUCLEAN since that's
what 6a797d273783 defined it as.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=199181
BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1560782
Reported-by: Wen Xu <wen.xu@gatech.edu>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
[harsh@prjkt.io: s/EFSCORRUPTED/EUCLEAN/ fs/ext4/balloc.c]
Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
---
 fs/ext4/balloc.c | 16 ++++++++++++++--
 fs/ext4/ialloc.c |  8 +++++++-
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/fs/ext4/balloc.c b/fs/ext4/balloc.c
index cb3860817fed..3b88f0ca0e82 100644
--- a/fs/ext4/balloc.c
+++ b/fs/ext4/balloc.c
@@ -340,20 +340,25 @@ static ext4_fsblk_t ext4_valid_block_bitmap(struct super_block *sb,
 	/* check whether block bitmap block number is set */
 	blk = ext4_block_bitmap(sb, desc);
 	offset = blk - group_first_block;
-	if (!ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
 		/* bad block bitmap */
 		return blk;
 
 	/* check whether the inode bitmap block number is set */
 	blk = ext4_inode_bitmap(sb, desc);
 	offset = blk - group_first_block;
-	if (!ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	    !ext4_test_bit(EXT4_B2C(sbi, offset), bh->b_data))
 		/* bad block bitmap */
 		return blk;
 
 	/* check whether the inode table block number is set */
 	blk = ext4_inode_table(sb, desc);
 	offset = blk - group_first_block;
+	if (offset < 0 || EXT4_B2C(sbi, offset) >= sb->s_blocksize ||
+	    EXT4_B2C(sbi, offset + sbi->s_itb_per_group) >= sb->s_blocksize)
+		return blk;
 	next_zero_bit = ext4_find_next_zero_bit(bh->b_data,
 			EXT4_B2C(sbi, offset + EXT4_SB(sb)->s_itb_per_group),
 			EXT4_B2C(sbi, offset));
@@ -416,6 +421,7 @@ struct buffer_head *
 ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 {
 	struct ext4_group_desc *desc;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct buffer_head *bh;
 	ext4_fsblk_t bitmap_blk;
 
@@ -423,6 +429,12 @@ ext4_read_block_bitmap_nowait(struct super_block *sb, ext4_group_t block_group)
 	if (!desc)
 		return NULL;
 	bitmap_blk = ext4_block_bitmap(sb, desc);
+	if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
+	    (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
+		ext4_error(sb, "Invalid block bitmap block %llu in "
+			   "block_group %u", bitmap_blk, block_group);
+		return ERR_PTR(-EUCLEAN);
+	}
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
 		ext4_error(sb, "Cannot get buffer for block bitmap - "
diff --git a/fs/ext4/ialloc.c b/fs/ext4/ialloc.c
index b7d49d2ab74f..9595daf6a44f 100644
--- a/fs/ext4/ialloc.c
+++ b/fs/ext4/ialloc.c
@@ -123,16 +123,22 @@ static struct buffer_head *
 ext4_read_inode_bitmap(struct super_block *sb, ext4_group_t block_group)
 {
 	struct ext4_group_desc *desc;
+	struct ext4_sb_info *sbi = EXT4_SB(sb);
 	struct buffer_head *bh = NULL;
 	ext4_fsblk_t bitmap_blk;
 	struct ext4_group_info *grp;
-	struct ext4_sb_info *sbi = EXT4_SB(sb);
 
 	desc = ext4_get_group_desc(sb, block_group, NULL);
 	if (!desc)
 		return NULL;
 
 	bitmap_blk = ext4_inode_bitmap(sb, desc);
+	if ((bitmap_blk <= le32_to_cpu(sbi->s_es->s_first_data_block)) ||
+	    (bitmap_blk >= ext4_blocks_count(sbi->s_es))) {
+		ext4_error(sb, "Invalid inode bitmap blk %llu in "
+			   "block_group %u", bitmap_blk, block_group);
+		return ERR_PTR(-EUCLEAN);
+	}
 	bh = sb_getblk(sb, bitmap_blk);
 	if (unlikely(!bh)) {
 		ext4_error(sb, "Cannot read inode bitmap - "
-- 
2.15.0.2308.g658a28aa74af

[PATCH 2/3] ext4: fail ext4_iget for root directory if unallocated

[Harsh Shandilya]

From: Theodore Ts'o <tytso@mit.edu>

Commit 8e4b5eae5decd9dfe5a4ee369c22028f90ab4c44 upstream.

If the root directory has an i_links_count of zero, then when the file
system is mounted, then when ext4_fill_super() notices the problem and
tries to call iput() the root directory in the error return path,
ext4_evict_inode() will try to free the inode on disk, before all of
the file system structures are set up, and this will result in an OOPS
caused by a NULL pointer dereference.

This issue has been assigned CVE-2018-1092.

https://bugzilla.kernel.org/show_bug.cgi?id=199179
https://bugzilla.redhat.com/show_bug.cgi?id=1560777

Reported-by: Wen Xu <wen.xu@gatech.edu>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
[harsh@prjkt.io: s/EFSCORRUPTED/EUCLEAN/ fs/ext4/inode.c]
Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
---
 fs/ext4/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 77df898ed45b..d2ec9d2aa82b 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -4217,6 +4217,12 @@ struct inode *ext4_iget(struct super_block *sb, unsigned long ino)
 		goto bad_inode;
 	raw_inode = ext4_raw_inode(&iloc);
 
+	if ((ino == EXT4_ROOT_INO) && (raw_inode->i_links_count == 0)) {
+		EXT4_ERROR_INODE(inode, "root inode unallocated");
+		ret = -EUCLEAN;
+		goto bad_inode;
+	}
+
 	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
 		ei->i_extra_isize = le16_to_cpu(raw_inode->i_extra_isize);
 		if (EXT4_GOOD_OLD_INODE_SIZE + ei->i_extra_isize >
-- 
2.15.0.2308.g658a28aa74af

[PATCH 3/3] ext4: don't allow r/w mounts if metadata blocks overlap the superblock

[Harsh Shandilya]

From: Theodore Ts'o <tytso@mit.edu>

Commit 18db4b4e6fc31eda838dd1c1296d67dbcb3dc957 upstream.

If some metadata block, such as an allocation bitmap, overlaps the
superblock, it's very likely that if the file system is mounted
read/write, the results will not be pretty.  So disallow r/w mounts
for file systems corrupted in this particular way.

Backport notes:
3.18.y is missing bc98a42c1f7d ("VFS: Convert sb->s_flags & MS_RDONLY to sb_rdonly(sb)")
and e462ec50cb5f ("VFS: Differentiate mount flags (MS_*) from internal superblock flags")
so we simply use the sb MS_RDONLY check from pre bc98a42c1f7d in place of the sb_rdonly
function used in the upstream variant of the patch.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Cc: stable@vger.kernel.org
Signed-off-by: Harsh Shandilya <harsh@prjkt.io>
---
 fs/ext4/super.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 26a0c5dd0c97..8e92cab056cb 100644
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -2112,6 +2112,8 @@ static int ext4_check_descriptors(struct super_block *sb,
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
 				 "Block bitmap for group %u overlaps "
 				 "superblock", i);
+			if (!(sb->s_flags & MS_RDONLY))
+				return 0;
 		}
 		if (block_bitmap < first_block || block_bitmap > last_block) {
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
@@ -2124,6 +2126,8 @@ static int ext4_check_descriptors(struct super_block *sb,
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
 				 "Inode bitmap for group %u overlaps "
 				 "superblock", i);
+			if (!(sb->s_flags & MS_RDONLY))
+				return 0;
 		}
 		if (inode_bitmap < first_block || inode_bitmap > last_block) {
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
@@ -2136,6 +2140,8 @@ static int ext4_check_descriptors(struct super_block *sb,
 			ext4_msg(sb, KERN_ERR, "ext4_check_descriptors: "
 				 "Inode table for group %u overlaps "
 				 "superblock", i);
+			if (!(sb->s_flags & MS_RDONLY))
+				return 0;
 		}
 		if (inode_table < first_block ||
 		    inode_table + sbi->s_itb_per_group - 1 > last_block) {
-- 
2.15.0.2308.g658a28aa74af

Re: [PATCH 3/3] ext4: don't allow r/w mounts if metadata blocks overlap the superblock

[Theodore Y. Ts'o]

On Sat, Apr 21, 2018 at 05:59:27PM +0530, Harsh Shandilya wrote:
> From: Theodore Ts'o <tytso@mit.edu>
> 
> Commit 18db4b4e6fc31eda838dd1c1296d67dbcb3dc957 upstream.

Thanks for resending with the upstream commit.  Could you include in
the future the kernel version the backport is meant for in the subject
line, e.g:

[PATCH 3.18.y 3/3] ext4: don't allow r/w mounts if metadata blocks

or 

[PATCH 4.9.y 3/3] ext4: don't allow r/w mounts if metadata blocks

Thanks!!

					- Ted

Re: [PATCH 3/3] ext4: don't allow r/w mounts if metadata blocks overlap the superblock

[Harsh Shandilya]

On 22 April 2018 1:37:44 AM IST, "Theodore Y. Ts'o" <tytso@mit.edu> wrote:
>On Sat, Apr 21, 2018 at 05:59:27PM +0530, Harsh Shandilya wrote:
>> From: Theodore Ts'o <tytso@mit.edu>
>> 
>> Commit 18db4b4e6fc31eda838dd1c1296d67dbcb3dc957 upstream.
>
>Thanks for resending with the upstream commit.  Could you include in
>the future the kernel version the backport is meant for in the subject
>line, e.g:
>
>[PATCH 3.18.y 3/3] ext4: don't allow r/w mounts if metadata blocks
>
>or 
>
>[PATCH 4.9.y 3/3] ext4: don't allow r/w mounts if metadata blocks
I mentioned that the patches are for 3.18.y in the cover letter title but you're right it should have been here as well. Noted for later :)

I've had to make changes to all the three patches, can you please Ack the backports so that Greg knows I didn't fubar anything? I'd appreciate it a lot.

thanks,
Harsh Shandilya, PRJKT Development LLC

Re: [PATCH 3/3] ext4: don't allow r/w mounts if metadata blocks overlap the superblock

[Theodore Y. Ts'o]

On Sun, Apr 22, 2018 at 08:00:44AM +0530, Harsh Shandilya wrote:
> >
> >[PATCH 4.9.y 3/3] ext4: don't allow r/w mounts if metadata blocks
> I mentioned that the patches are for 3.18.y in the cover letter title but you're right it should have been here as well. Noted for later :)
>

As far as I can tell I wasn't cc'ed on the cover letter, so I didn't
see it.

> I've had to make changes to all the three patches, can you please
> Ack the backports so that Greg knows I didn't fubar anything? I'd
> appreciate it a lot.

If you want me to review the patches, can you do me a favor?

It looks like you sent two interleaved patches, one with 4 patches,
ahnd one with 3 patches, and with a very confusing in-reply-to headers
which completely confused the mail threading.  So this is what I see
in my inbox, and it is a Complete Mess:

  Apr 21 Harsh Shandilya (6.6K) ┬─>[PATCH 4/4] ext4: force revalidation of directory pointer afte
  Apr 21 Harsh Shandilya (9.1K) ├─>[PATCH 1/4] ext4: add validity checks for bitmap block numbers
  Apr 21 Harsh Shandilya (9.5K) │ └─>[PATCH 1/3] ext4: add validity checks for bitmap block numbe
  Apr 21 Harsh Shandilya (6.7K) ├─>[PATCH 2/4] ext4: fail ext4_iget for root directory if unalloc
  Apr 21 Harsh Shandilya (6.9K) │ └─>[PATCH 2/3] ext4: fail ext4_iget for root directory if unall
  Apr 21 Harsh Shandilya (7.0K) └─>[PATCH 3/4] ext4: don't allow r/w mounts if metadata blocks ov
  Apr 21 Harsh Shandilya (7.4K)   └─>[PATCH 3/3] ext4: don't allow r/w mounts if metadata blocks 

(What is this all about?  I didn't get the cover letter.  Why are some
of the patches revised, and why is the subsequent patch series have
only 3 patches instead of 4?  Why wasn't this all explained the
PATCH-v2 cover letter?  (Or maybe it was, but I'll never know because
I wasn't sent it, if it exists.  :-)

So can you please resend with a subject prefix that looks like this:
"[PATCH-v3 3.18 1/4]"

And send it as a free-standard mail thread, with the cover-letter
message not chained to anything else, and with each just being just a
reply to the previous one?  I want something that looks like this:

  Mar 12 Darrick J. Wong (6.4K) [PATCH v4 0/4] e2scrub: online fsck for ext4
  Mar 12 Darrick J. Wong (7.3K) ├─>[PATCH 1/4] tune2fs: allow setting the filesystem error bit
  Mar 12 Darrick J. Wong ( 29K) ├─>[PATCH 2/4] e2scrub: create online fsck tool of sorts
  Mar 12 Darrick J. Wong (9.6K) ├─>[PATCH 3/4] e2scrub: create a script to scrub all ext* filesyst
  Mar 12 Darrick J. Wong ( 34K) └─>[PATCH 4/4] e2scrub: add service (cron, systemd) support

See the difference?

Thanks,

						- Ted

Re: [PATCH 3/3] ext4: don't allow r/w mounts if metadata blocks overlap the superblock

[Harsh Shandilya]

On 22 April 2018 9:31:06 AM IST, "Theodore Y. Ts'o" <tytso@mit.edu> wrote:
>On Sun, Apr 22, 2018 at 08:00:44AM +0530, Harsh Shandilya wrote:
>> >
>> >[PATCH 4.9.y 3/3] ext4: don't allow r/w mounts if metadata blocks
>> I mentioned that the patches are for 3.18.y in the cover letter title
>but you're right it should have been here as well. Noted for later :)
>>
>
>As far as I can tell I wasn't cc'ed on the cover letter, so I didn't
>see it.
>
>> I've had to make changes to all the three patches, can you please
>> Ack the backports so that Greg knows I didn't fubar anything? I'd
>> appreciate it a lot.
>
>If you want me to review the patches, can you do me a favor?
>
>It looks like you sent two interleaved patches, one with 4 patches,
>ahnd one with 3 patches, and with a very confusing in-reply-to headers
>which completely confused the mail threading.  So this is what I see
>in my inbox, and it is a Complete Mess:
>
>Apr 21 Harsh Shandilya (6.6K) ┬─>[PATCH 4/4] ext4: force revalidation
>of directory pointer afte
>Apr 21 Harsh Shandilya (9.1K) ├─>[PATCH 1/4] ext4: add validity checks
>for bitmap block numbers
>Apr 21 Harsh Shandilya (9.5K) │ └─>[PATCH 1/3] ext4: add validity
>checks for bitmap block numbe
>Apr 21 Harsh Shandilya (6.7K) ├─>[PATCH 2/4] ext4: fail ext4_iget for
>root directory if unalloc
>Apr 21 Harsh Shandilya (6.9K) │ └─>[PATCH 2/3] ext4: fail ext4_iget for
>root directory if unall
>Apr 21 Harsh Shandilya (7.0K) └─>[PATCH 3/4] ext4: don't allow r/w
>mounts if metadata blocks ov
>Apr 21 Harsh Shandilya (7.4K)   └─>[PATCH 3/3] ext4: don't allow r/w
>mounts if metadata blocks 
>
>(What is this all about?  I didn't get the cover letter.  Why are some
>of the patches revised, and why is the subsequent patch series have
>only 3 patches instead of 4?  Why wasn't this all explained the
>PATCH-v2 cover letter?  (Or maybe it was, but I'll never know because
>I wasn't sent it, if it exists.  :-)
>
>So can you please resend with a subject prefix that looks like this:
>"[PATCH-v3 3.18 1/4]"
>
>And send it as a free-standard mail thread, with the cover-letter
>message not chained to anything else, and with each just being just a
>reply to the previous one?  I want something that looks like this:
>
>Mar 12 Darrick J. Wong (6.4K) [PATCH v4 0/4] e2scrub: online fsck for
>ext4
>Mar 12 Darrick J. Wong (7.3K) ├─>[PATCH 1/4] tune2fs: allow setting the
>filesystem error bit
>Mar 12 Darrick J. Wong ( 29K) ├─>[PATCH 2/4] e2scrub: create online
>fsck tool of sorts
>Mar 12 Darrick J. Wong (9.6K) ├─>[PATCH 3/4] e2scrub: create a script
>to scrub all ext* filesyst
>Mar 12 Darrick J. Wong ( 34K) └─>[PATCH 4/4] e2scrub: add service
>(cron, systemd) support
>
>See the difference?
>
>Thanks,
>
>						- Ted

Yes I see where I screwed up, I'll have the revised series sent in five minutes. Sorry for the mess :(
-- 
Harsh Shandilya, PRJKT Development LLC