Linux-ext4 Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 0/2 v2] ext4: Handle directories with holes better
@ 2019-12-02 17:02 Jan Kara
  2019-12-02 17:02 ` [PATCH 1/2] ext4: Fix ext4_empty_dir() for directories with holes Jan Kara
  2019-12-02 17:02 ` [PATCH 2/2] ext4: Check for directory entries too close to block end Jan Kara
  0 siblings, 2 replies; 6+ messages in thread
From: Jan Kara @ 2019-12-02 17:02 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-ext4, Jan Kara

Hello,

here is a fixed up version of ext4 patch to fix ext4_empty_dir() for
directories with holes. There's also another patch that tightens checks in
ext4_check_dir_entry() so that directory iteration code cannot read data
beyond end of the buffer.

								Honza

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 1/2] ext4: Fix ext4_empty_dir() for directories with holes
  2019-12-02 17:02 [PATCH 0/2 v2] ext4: Handle directories with holes better Jan Kara
@ 2019-12-02 17:02 ` Jan Kara
  2019-12-09  0:12   ` Theodore Y. Ts'o
  2019-12-02 17:02 ` [PATCH 2/2] ext4: Check for directory entries too close to block end Jan Kara
  1 sibling, 1 reply; 6+ messages in thread
From: Jan Kara @ 2019-12-02 17:02 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-ext4, Jan Kara, stable

Function ext4_empty_dir() doesn't correctly handle directories with
holes and crashes on bh->b_data dereference when bh is NULL. Reorganize
the loop to use 'offset' variable all the times instead of comparing
pointers to current direntry with bh->b_data pointer. Also add more
strict checking of '.' and '..' directory entries to avoid entering loop
in possibly invalid state on corrupted filesystems.

References: CVE-2019-19037
CC: stable@vger.kernel.org
Fixes: 4e19d6b65fb4 ("ext4: allow directory holes")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/namei.c | 32 ++++++++++++++++++--------------
 1 file changed, 18 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index a427d2031a8d..abf52f98f3ca 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -2808,7 +2808,7 @@ bool ext4_empty_dir(struct inode *inode)
 {
 	unsigned int offset;
 	struct buffer_head *bh;
-	struct ext4_dir_entry_2 *de, *de1;
+	struct ext4_dir_entry_2 *de;
 	struct super_block *sb;
 
 	if (ext4_has_inline_data(inode)) {
@@ -2833,19 +2833,25 @@ bool ext4_empty_dir(struct inode *inode)
 		return true;
 
 	de = (struct ext4_dir_entry_2 *) bh->b_data;
-	de1 = ext4_next_entry(de, sb->s_blocksize);
-	if (le32_to_cpu(de->inode) != inode->i_ino ||
-			le32_to_cpu(de1->inode) == 0 ||
-			strcmp(".", de->name) || strcmp("..", de1->name)) {
-		ext4_warning_inode(inode, "directory missing '.' and/or '..'");
+	if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data, bh->b_size,
+				 0) ||
+	    le32_to_cpu(de->inode) != inode->i_ino || strcmp(".", de->name)) {
+		ext4_warning_inode(inode, "directory missing '.'");
+		brelse(bh);
+		return true;
+	}
+	offset = ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize);
+	de = ext4_next_entry(de, sb->s_blocksize);
+	if (ext4_check_dir_entry(inode, NULL, de, bh, bh->b_data, bh->b_size,
+				 offset) ||
+	    le32_to_cpu(de->inode) == 0 || strcmp("..", de->name)) {
+		ext4_warning_inode(inode, "directory missing '..'");
 		brelse(bh);
 		return true;
 	}
-	offset = ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize) +
-		 ext4_rec_len_from_disk(de1->rec_len, sb->s_blocksize);
-	de = ext4_next_entry(de1, sb->s_blocksize);
+	offset += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize);
 	while (offset < inode->i_size) {
-		if ((void *) de >= (void *) (bh->b_data+sb->s_blocksize)) {
+		if (!(offset & (sb->s_blocksize - 1))) {
 			unsigned int lblock;
 			brelse(bh);
 			lblock = offset >> EXT4_BLOCK_SIZE_BITS(sb);
@@ -2856,12 +2862,11 @@ bool ext4_empty_dir(struct inode *inode)
 			}
 			if (IS_ERR(bh))
 				return true;
-			de = (struct ext4_dir_entry_2 *) bh->b_data;
 		}
+		de = (struct ext4_dir_entry_2 *) (bh->b_data +
+					(offset & (sb->s_blocksize - 1)));
 		if (ext4_check_dir_entry(inode, NULL, de, bh,
 					 bh->b_data, bh->b_size, offset)) {
-			de = (struct ext4_dir_entry_2 *)(bh->b_data +
-							 sb->s_blocksize);
 			offset = (offset | (sb->s_blocksize - 1)) + 1;
 			continue;
 		}
@@ -2870,7 +2875,6 @@ bool ext4_empty_dir(struct inode *inode)
 			return false;
 		}
 		offset += ext4_rec_len_from_disk(de->rec_len, sb->s_blocksize);
-		de = ext4_next_entry(de, sb->s_blocksize);
 	}
 	brelse(bh);
 	return true;
-- 
2.16.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH 2/2] ext4: Check for directory entries too close to block end
  2019-12-02 17:02 [PATCH 0/2 v2] ext4: Handle directories with holes better Jan Kara
  2019-12-02 17:02 ` [PATCH 1/2] ext4: Fix ext4_empty_dir() for directories with holes Jan Kara
@ 2019-12-02 17:02 ` Jan Kara
  2019-12-09  0:42   ` Theodore Y. Ts'o
  2019-12-09  0:43   ` [PATCH] ext4: optimize __ext4_check_dir_entry() Theodore Ts'o
  1 sibling, 2 replies; 6+ messages in thread
From: Jan Kara @ 2019-12-02 17:02 UTC (permalink / raw)
  To: Ted Tso; +Cc: linux-ext4, Jan Kara, stable

ext4_check_dir_entry() currently does not catch a case when a directory
entry ends so close to the block end that the header of the next
directory entry would not fit in the remaining space. This can lead to
directory iteration code trying to access address beyond end of current
buffer head leading to oops.

CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/ext4/dir.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 9fdd2b269d61..6305d5ec25af 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -81,6 +81,11 @@ int __ext4_check_dir_entry(const char *function, unsigned int line,
 		error_msg = "rec_len is too small for name_len";
 	else if (unlikely(((char *) de - buf) + rlen > size))
 		error_msg = "directory entry overrun";
+	else if (unlikely(((char *) de - buf) + rlen >
+			  size - EXT4_DIR_REC_LEN(1) &&
+			  ((char *) de - buf) + rlen != size)) {
+		error_msg = "directory entry too close to block end";
+	}
 	else if (unlikely(le32_to_cpu(de->inode) >
 			le32_to_cpu(EXT4_SB(dir->i_sb)->s_es->s_inodes_count)))
 		error_msg = "inode out of bounds";
-- 
2.16.4


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] ext4: Fix ext4_empty_dir() for directories with holes
  2019-12-02 17:02 ` [PATCH 1/2] ext4: Fix ext4_empty_dir() for directories with holes Jan Kara
@ 2019-12-09  0:12   ` Theodore Y. Ts'o
  0 siblings, 0 replies; 6+ messages in thread
From: Theodore Y. Ts'o @ 2019-12-09  0:12 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4, stable

On Mon, Dec 02, 2019 at 06:02:12PM +0100, Jan Kara wrote:
> Function ext4_empty_dir() doesn't correctly handle directories with
> holes and crashes on bh->b_data dereference when bh is NULL. Reorganize
> the loop to use 'offset' variable all the times instead of comparing
> pointers to current direntry with bh->b_data pointer. Also add more
> strict checking of '.' and '..' directory entries to avoid entering loop
> in possibly invalid state on corrupted filesystems.
> 
> References: CVE-2019-19037
> CC: stable@vger.kernel.org
> Fixes: 4e19d6b65fb4 ("ext4: allow directory holes")
> Signed-off-by: Jan Kara <jack@suse.cz>

Applied, thanks.

					- Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] ext4: Check for directory entries too close to block end
  2019-12-02 17:02 ` [PATCH 2/2] ext4: Check for directory entries too close to block end Jan Kara
@ 2019-12-09  0:42   ` Theodore Y. Ts'o
  2019-12-09  0:43   ` [PATCH] ext4: optimize __ext4_check_dir_entry() Theodore Ts'o
  1 sibling, 0 replies; 6+ messages in thread
From: Theodore Y. Ts'o @ 2019-12-09  0:42 UTC (permalink / raw)
  To: Jan Kara; +Cc: linux-ext4, stable

On Mon, Dec 02, 2019 at 06:02:13PM +0100, Jan Kara wrote:
> ext4_check_dir_entry() currently does not catch a case when a directory
> entry ends so close to the block end that the header of the next
> directory entry would not fit in the remaining space. This can lead to
> directory iteration code trying to access address beyond end of current
> buffer head leading to oops.
> 
> CC: stable@vger.kernel.org
> Signed-off-by: Jan Kara <jack@suse.cz>

Thanks, applied.

						- Ted

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH] ext4: optimize __ext4_check_dir_entry()
  2019-12-02 17:02 ` [PATCH 2/2] ext4: Check for directory entries too close to block end Jan Kara
  2019-12-09  0:42   ` Theodore Y. Ts'o
@ 2019-12-09  0:43   ` Theodore Ts'o
  1 sibling, 0 replies; 6+ messages in thread
From: Theodore Ts'o @ 2019-12-09  0:43 UTC (permalink / raw)
  To: Ext4 Developers List; +Cc: Theodore Ts'o

Make __ext4_check_dir_entry() a bit easier to understand, and reduce
the object size of the function by over 11%.

Signed-off-by: Theodore Ts'o <tytso@mit.edu>
---

As a follow-up to "ext4: check for directory entries too close to block end"

 fs/ext4/dir.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 6305d5ec25af..9f00fc0bf21d 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -72,6 +72,7 @@ int __ext4_check_dir_entry(const char *function, unsigned int line,
 	const char *error_msg = NULL;
 	const int rlen = ext4_rec_len_from_disk(de->rec_len,
 						dir->i_sb->s_blocksize);
+	const int next_offset = ((char *) de - buf) + rlen;
 
 	if (unlikely(rlen < EXT4_DIR_REC_LEN(1)))
 		error_msg = "rec_len is smaller than minimal";
@@ -79,13 +80,11 @@ int __ext4_check_dir_entry(const char *function, unsigned int line,
 		error_msg = "rec_len % 4 != 0";
 	else if (unlikely(rlen < EXT4_DIR_REC_LEN(de->name_len)))
 		error_msg = "rec_len is too small for name_len";
-	else if (unlikely(((char *) de - buf) + rlen > size))
+	else if (unlikely(next_offset > size))
 		error_msg = "directory entry overrun";
-	else if (unlikely(((char *) de - buf) + rlen >
-			  size - EXT4_DIR_REC_LEN(1) &&
-			  ((char *) de - buf) + rlen != size)) {
+	else if (unlikely(next_offset > size - EXT4_DIR_REC_LEN(1) &&
+			  next_offset != size))
 		error_msg = "directory entry too close to block end";
-	}
 	else if (unlikely(le32_to_cpu(de->inode) >
 			le32_to_cpu(EXT4_SB(dir->i_sb)->s_es->s_inodes_count)))
 		error_msg = "inode out of bounds";
-- 
2.23.0


^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, back to index

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-02 17:02 [PATCH 0/2 v2] ext4: Handle directories with holes better Jan Kara
2019-12-02 17:02 ` [PATCH 1/2] ext4: Fix ext4_empty_dir() for directories with holes Jan Kara
2019-12-09  0:12   ` Theodore Y. Ts'o
2019-12-02 17:02 ` [PATCH 2/2] ext4: Check for directory entries too close to block end Jan Kara
2019-12-09  0:42   ` Theodore Y. Ts'o
2019-12-09  0:43   ` [PATCH] ext4: optimize __ext4_check_dir_entry() Theodore Ts'o

Linux-ext4 Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-ext4/0 linux-ext4/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-ext4 linux-ext4/ https://lore.kernel.org/linux-ext4 \
		linux-ext4@vger.kernel.org
	public-inbox-index linux-ext4

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-ext4


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git