From: Baokun Li <libaokun1@huawei.com>

The syzbot constructs a directory that has no dirblock but is non-inline,
i.e. the first directory block is a hole. And no errors are reported when
creating files in this directory in the following flow.

    ext4_mknod
     ...
      ext4_add_entry
        // Read block 0
        ext4_read_dirblock(dir, block, DIRENT)
          bh = ext4_bread(NULL, inode, block, 0)
          if (!bh && (type == INDEX || type == DIRENT_HTREE))
          // The first directory block is a hole
          // But type == DIRENT, so no error is reported.

After that, we get a directory block without '.' and '..' but with a valid
dentry. This may cause some code that relies on dot or dotdot (such as
make_indexed_dir()) to crash.

Therefore when ext4_read_dirblock() finds that the first directory block
is a hole report that the filesystem is corrupted and return an error to
avoid loading corrupted data from disk causing something bad.

Reported-by: syzbot+ae688d469e36fb5138d0@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ae688d469e36fb5138d0
Fixes: 4e19d6b65fb4 ("ext4: allow directory holes")
Cc: stable@kernel.org
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/namei.c | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 35881e3dd880..6a95713f9193 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -151,10 +151,11 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
 
 		return bh;
 	}
-	if (!bh && (type == INDEX || type == DIRENT_HTREE)) {
+	/* The first directory block must not be a hole. */
+	if (!bh && (type == INDEX || type == DIRENT_HTREE || block == 0)) {
 		ext4_error_inode(inode, func, line, block,
-				 "Directory hole found for htree %s block",
-				 (type == INDEX) ? "index" : "leaf");
+				 "Directory hole found for htree %s block %u",
+				 (type == INDEX) ? "index" : "leaf", block);
 		return ERR_PTR(-EFSCORRUPTED);
 	}
 	if (!bh)
@@ -3084,10 +3085,7 @@ bool ext4_empty_dir(struct inode *inode)
 		EXT4_ERROR_INODE(inode, "invalid size");
 		return false;
 	}
-	/* The first directory block must not be a hole,
-	 * so treat it as DIRENT_HTREE
-	 */
-	bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE);
+	bh = ext4_read_dirblock(inode, 0, EITHER);
 	if (IS_ERR(bh))
 		return false;
 
@@ -3529,10 +3527,7 @@ static struct buffer_head *ext4_get_first_dir_block(handle_t *handle,
 		struct ext4_dir_entry_2 *de;
 		unsigned int offset;
 
-		/* The first directory block must not be a hole, so
-		 * treat it as DIRENT_HTREE
-		 */
-		bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE);
+		bh = ext4_read_dirblock(inode, 0, EITHER);
 		if (IS_ERR(bh)) {
 			*retval = PTR_ERR(bh);
 			return NULL;
-- 
2.39.2

On Tue 02-07-24 21:23:49, libaokun@huaweicloud.com wrote:
> From: Baokun Li <libaokun1@huawei.com>
> 
> The syzbot constructs a directory that has no dirblock but is non-inline,
> i.e. the first directory block is a hole. And no errors are reported when
> creating files in this directory in the following flow.
> 
>     ext4_mknod
>      ...
>       ext4_add_entry
>         // Read block 0
>         ext4_read_dirblock(dir, block, DIRENT)
>           bh = ext4_bread(NULL, inode, block, 0)
>           if (!bh && (type == INDEX || type == DIRENT_HTREE))
>           // The first directory block is a hole
>           // But type == DIRENT, so no error is reported.
> 
> After that, we get a directory block without '.' and '..' but with a valid
> dentry. This may cause some code that relies on dot or dotdot (such as
> make_indexed_dir()) to crash.
> 
> Therefore when ext4_read_dirblock() finds that the first directory block
> is a hole report that the filesystem is corrupted and return an error to
> avoid loading corrupted data from disk causing something bad.
> 
> Reported-by: syzbot+ae688d469e36fb5138d0@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ae688d469e36fb5138d0
> Fixes: 4e19d6b65fb4 ("ext4: allow directory holes")
> Cc: stable@kernel.org
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/namei.c | 17 ++++++-----------
>  1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 35881e3dd880..6a95713f9193 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -151,10 +151,11 @@ static struct buffer_head *__ext4_read_dirblock(struct inode *inode,
>  
>  		return bh;
>  	}
> -	if (!bh && (type == INDEX || type == DIRENT_HTREE)) {
> +	/* The first directory block must not be a hole. */
> +	if (!bh && (type == INDEX || type == DIRENT_HTREE || block == 0)) {
>  		ext4_error_inode(inode, func, line, block,
> -				 "Directory hole found for htree %s block",
> -				 (type == INDEX) ? "index" : "leaf");
> +				 "Directory hole found for htree %s block %u",
> +				 (type == INDEX) ? "index" : "leaf", block);
>  		return ERR_PTR(-EFSCORRUPTED);
>  	}
>  	if (!bh)
> @@ -3084,10 +3085,7 @@ bool ext4_empty_dir(struct inode *inode)
>  		EXT4_ERROR_INODE(inode, "invalid size");
>  		return false;
>  	}
> -	/* The first directory block must not be a hole,
> -	 * so treat it as DIRENT_HTREE
> -	 */
> -	bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE);
> +	bh = ext4_read_dirblock(inode, 0, EITHER);
>  	if (IS_ERR(bh))
>  		return false;
>  
> @@ -3529,10 +3527,7 @@ static struct buffer_head *ext4_get_first_dir_block(handle_t *handle,
>  		struct ext4_dir_entry_2 *de;
>  		unsigned int offset;
>  
> -		/* The first directory block must not be a hole, so
> -		 * treat it as DIRENT_HTREE
> -		 */
> -		bh = ext4_read_dirblock(inode, 0, DIRENT_HTREE);
> +		bh = ext4_read_dirblock(inode, 0, EITHER);
>  		if (IS_ERR(bh)) {
>  			*retval = PTR_ERR(bh);
>  			return NULL;
> -- 
> 2.39.2
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR