From: Zhao Mengmeng <zhaomengmeng@kylinos.cn>

Refactor inode_bmap() to handle error since udf_next_aext() can return
error now. On situations like ftruncate, udf_extend_file() can now
detect errors and bail out early without resorting to checking for
particular offsets and assuming internal behavior of these functions.

Reported-by: syzbot+7a4842f0b1801230a989@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=7a4842f0b1801230a989
Tested-by: syzbot+7a4842f0b1801230a989@syzkaller.appspotmail.com
Signed-off-by: Zhao Mengmeng <zhaomengmeng@kylinos.cn>
Suggested-by: Jan Kara <jack@suse.cz>
---
 fs/udf/directory.c | 13 ++++++++-----
 fs/udf/inode.c     | 29 ++++++++++++++++++-----------
 fs/udf/partition.c |  6 ++++--
 fs/udf/truncate.c  |  8 +++++---
 fs/udf/udfdecl.h   |  5 +++--
 5 files changed, 38 insertions(+), 23 deletions(-)

diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index 82922a4ae425..4b8bb77eaffa 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -246,6 +246,7 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
 {
 	struct udf_inode_info *iinfo = UDF_I(dir);
 	int err = 0;
+	int8_t etype;
 
 	iter->dir = dir;
 	iter->bh[0] = iter->bh[1] = NULL;
@@ -265,9 +266,9 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
 		goto out;
 	}
 
-	if (inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos,
-		       &iter->eloc, &iter->elen, &iter->loffset) !=
-	    (EXT_RECORDED_ALLOCATED >> 30)) {
+	err = inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos, &iter->eloc,
+		   &iter->elen, &iter->loffset, &etype);
+	if (err || etype != (EXT_RECORDED_ALLOCATED >> 30)) {
 		if (pos == dir->i_size)
 			return 0;
 		udf_err(dir->i_sb,
@@ -463,6 +464,7 @@ int udf_fiiter_append_blk(struct udf_fileident_iter *iter)
 	sector_t block;
 	uint32_t old_elen = iter->elen;
 	int err;
+	int8_t etype;
 
 	if (WARN_ON_ONCE(iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB))
 		return -EINVAL;
@@ -477,8 +479,9 @@ int udf_fiiter_append_blk(struct udf_fileident_iter *iter)
 		udf_fiiter_update_elen(iter, old_elen);
 		return err;
 	}
-	if (inode_bmap(iter->dir, block, &iter->epos, &iter->eloc, &iter->elen,
-		       &iter->loffset) != (EXT_RECORDED_ALLOCATED >> 30)) {
+	err = inode_bmap(iter->dir, block, &iter->epos, &iter->eloc, &iter->elen,
+		   &iter->loffset, &etype);
+	if (err || etype != (EXT_RECORDED_ALLOCATED >> 30)) {
 		udf_err(iter->dir->i_sb,
 			"block %llu not allocated in directory (ino %lu)\n",
 			(unsigned long long)block, iter->dir->i_ino);
diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 6c4f104e2bf7..be9356f0eecf 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -418,10 +418,11 @@ static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
 		uint32_t elen;
 		sector_t offset;
 		struct extent_position epos = {};
+		int8_t etype;
 
 		down_read(&iinfo->i_data_sem);
-		if (inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset)
-				== (EXT_RECORDED_ALLOCATED >> 30)) {
+		err = inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset, &etype);
+		if (!err && etype == (EXT_RECORDED_ALLOCATED >> 30)) {
 			map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc,
 							offset);
 			map->oflags |= UDF_BLK_MAPPED;
@@ -664,8 +665,10 @@ static int udf_extend_file(struct inode *inode, loff_t newsize)
 	 */
 	udf_discard_prealloc(inode);
 
-	etype = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset);
-	within_last_ext = (etype != -1);
+	err = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset, &etype);
+	if (UDF_EXT_ERR(err))
+		goto out;
+	within_last_ext = (!err);
 	/* We don't expect extents past EOF... */
 	WARN_ON_ONCE(within_last_ext &&
 		     elen > ((loff_t)offset + 1) << inode->i_blkbits);
@@ -2391,13 +2394,17 @@ int8_t udf_delete_aext(struct inode *inode, struct extent_position epos)
 	return (elen >> 30);
 }
 
-int8_t inode_bmap(struct inode *inode, sector_t block,
-		  struct extent_position *pos, struct kernel_lb_addr *eloc,
-		  uint32_t *elen, sector_t *offset)
+/*
+ * return 0 when iudf_next_aext() loop success.
+ * return err < 0 and err != -ENODATA indicates error.
+ * return err == -ENODATA indicates hit EOF.
+ */
+int inode_bmap(struct inode *inode, sector_t block, struct extent_position *pos,
+	       struct kernel_lb_addr *eloc, uint32_t *elen, sector_t *offset,
+	       int8_t *etype)
 {
 	unsigned char blocksize_bits = inode->i_sb->s_blocksize_bits;
 	loff_t lbcount = 0, bcount = (loff_t) block << blocksize_bits;
-	int8_t etype;
 	struct udf_inode_info *iinfo;
 	int err = 0;
 
@@ -2409,18 +2416,18 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
 	}
 	*elen = 0;
 	do {
-		err = udf_next_aext(inode, pos, eloc, elen, &etype, 1);
+		err = udf_next_aext(inode, pos, eloc, elen, etype, 1);
 		if (UDF_EXT_EOF(err)) {
 			*offset = (bcount - lbcount) >> blocksize_bits;
 			iinfo->i_lenExtents = lbcount;
 		}
 		if (err < 0)
-			return -1;
+			return err;
 		lbcount += *elen;
 	} while (lbcount <= bcount);
 	/* update extent cache */
 	udf_update_extent_cache(inode, lbcount - *elen, pos);
 	*offset = (bcount + *elen - lbcount) >> blocksize_bits;
 
-	return etype;
+	return 0;
 }
diff --git a/fs/udf/partition.c b/fs/udf/partition.c
index af877991edc1..c441d4ae1f96 100644
--- a/fs/udf/partition.c
+++ b/fs/udf/partition.c
@@ -282,9 +282,11 @@ static uint32_t udf_try_read_meta(struct inode *inode, uint32_t block,
 	sector_t ext_offset;
 	struct extent_position epos = {};
 	uint32_t phyblock;
+	int8_t etype;
+	int err = 0;
 
-	if (inode_bmap(inode, block, &epos, &eloc, &elen, &ext_offset) !=
-						(EXT_RECORDED_ALLOCATED >> 30))
+	err = inode_bmap(inode, block, &epos, &eloc, &elen, &ext_offset, &etype);
+	if (err || etype != (EXT_RECORDED_ALLOCATED >> 30))
 		phyblock = 0xFFFFFFFF;
 	else {
 		map = &UDF_SB(sb)->s_partmaps[partition];
diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
index af06f7101859..d13ba9fd1309 100644
--- a/fs/udf/truncate.c
+++ b/fs/udf/truncate.c
@@ -208,10 +208,12 @@ int udf_truncate_extents(struct inode *inode)
 	else
 		BUG();
 
-	etype = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset);
+	err = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset, &etype);
 	byte_offset = (offset << sb->s_blocksize_bits) +
-		(inode->i_size & (sb->s_blocksize - 1));
-	if (etype == -1) {
+		      (inode->i_size & (sb->s_blocksize - 1));
+	if (UDF_EXT_ERR(err))
+		return err;
+	if (UDF_EXT_EOF(err)) {
 		/* We should extend the file? */
 		WARN_ON(byte_offset);
 		return 0;
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 206077da9968..a156ed95189a 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -160,8 +160,9 @@ extern struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
 extern int udf_setsize(struct inode *, loff_t);
 extern void udf_evict_inode(struct inode *);
 extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
-extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
-			 struct kernel_lb_addr *, uint32_t *, sector_t *);
+extern int inode_bmap(struct inode *inode, sector_t block,
+		      struct extent_position *pos, struct kernel_lb_addr *eloc,
+		      uint32_t *elen, sector_t *offset, int8_t *etype);
 int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
 extern int udf_setup_indirect_aext(struct inode *inode, udf_pblk_t block,
 				   struct extent_position *epos);
-- 
2.43.0

On Thu 26-09-24 20:07:53, Zhao Mengmeng wrote:
> From: Zhao Mengmeng <zhaomengmeng@kylinos.cn>
> 
> Refactor inode_bmap() to handle error since udf_next_aext() can return
> error now. On situations like ftruncate, udf_extend_file() can now
> detect errors and bail out early without resorting to checking for
> particular offsets and assuming internal behavior of these functions.
> 
> Reported-by: syzbot+7a4842f0b1801230a989@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7a4842f0b1801230a989
> Tested-by: syzbot+7a4842f0b1801230a989@syzkaller.appspotmail.com
> Signed-off-by: Zhao Mengmeng <zhaomengmeng@kylinos.cn>
> Suggested-by: Jan Kara <jack@suse.cz>

This patch looks good to me now too. Thanks!

								Honza

> ---
>  fs/udf/directory.c | 13 ++++++++-----
>  fs/udf/inode.c     | 29 ++++++++++++++++++-----------
>  fs/udf/partition.c |  6 ++++--
>  fs/udf/truncate.c  |  8 +++++---
>  fs/udf/udfdecl.h   |  5 +++--
>  5 files changed, 38 insertions(+), 23 deletions(-)
> 
> diff --git a/fs/udf/directory.c b/fs/udf/directory.c
> index 82922a4ae425..4b8bb77eaffa 100644
> --- a/fs/udf/directory.c
> +++ b/fs/udf/directory.c
> @@ -246,6 +246,7 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
>  {
>  	struct udf_inode_info *iinfo = UDF_I(dir);
>  	int err = 0;
> +	int8_t etype;
>  
>  	iter->dir = dir;
>  	iter->bh[0] = iter->bh[1] = NULL;
> @@ -265,9 +266,9 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
>  		goto out;
>  	}
>  
> -	if (inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos,
> -		       &iter->eloc, &iter->elen, &iter->loffset) !=
> -	    (EXT_RECORDED_ALLOCATED >> 30)) {
> +	err = inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos, &iter->eloc,
> +		   &iter->elen, &iter->loffset, &etype);
> +	if (err || etype != (EXT_RECORDED_ALLOCATED >> 30)) {
>  		if (pos == dir->i_size)
>  			return 0;
>  		udf_err(dir->i_sb,
> @@ -463,6 +464,7 @@ int udf_fiiter_append_blk(struct udf_fileident_iter *iter)
>  	sector_t block;
>  	uint32_t old_elen = iter->elen;
>  	int err;
> +	int8_t etype;
>  
>  	if (WARN_ON_ONCE(iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB))
>  		return -EINVAL;
> @@ -477,8 +479,9 @@ int udf_fiiter_append_blk(struct udf_fileident_iter *iter)
>  		udf_fiiter_update_elen(iter, old_elen);
>  		return err;
>  	}
> -	if (inode_bmap(iter->dir, block, &iter->epos, &iter->eloc, &iter->elen,
> -		       &iter->loffset) != (EXT_RECORDED_ALLOCATED >> 30)) {
> +	err = inode_bmap(iter->dir, block, &iter->epos, &iter->eloc, &iter->elen,
> +		   &iter->loffset, &etype);
> +	if (err || etype != (EXT_RECORDED_ALLOCATED >> 30)) {
>  		udf_err(iter->dir->i_sb,
>  			"block %llu not allocated in directory (ino %lu)\n",
>  			(unsigned long long)block, iter->dir->i_ino);
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 6c4f104e2bf7..be9356f0eecf 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -418,10 +418,11 @@ static int udf_map_block(struct inode *inode, struct udf_map_rq *map)
>  		uint32_t elen;
>  		sector_t offset;
>  		struct extent_position epos = {};
> +		int8_t etype;
>  
>  		down_read(&iinfo->i_data_sem);
> -		if (inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset)
> -				== (EXT_RECORDED_ALLOCATED >> 30)) {
> +		err = inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset, &etype);
> +		if (!err && etype == (EXT_RECORDED_ALLOCATED >> 30)) {
>  			map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc,
>  							offset);
>  			map->oflags |= UDF_BLK_MAPPED;
> @@ -664,8 +665,10 @@ static int udf_extend_file(struct inode *inode, loff_t newsize)
>  	 */
>  	udf_discard_prealloc(inode);
>  
> -	etype = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset);
> -	within_last_ext = (etype != -1);
> +	err = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset, &etype);
> +	if (UDF_EXT_ERR(err))
> +		goto out;
> +	within_last_ext = (!err);
>  	/* We don't expect extents past EOF... */
>  	WARN_ON_ONCE(within_last_ext &&
>  		     elen > ((loff_t)offset + 1) << inode->i_blkbits);
> @@ -2391,13 +2394,17 @@ int8_t udf_delete_aext(struct inode *inode, struct extent_position epos)
>  	return (elen >> 30);
>  }
>  
> -int8_t inode_bmap(struct inode *inode, sector_t block,
> -		  struct extent_position *pos, struct kernel_lb_addr *eloc,
> -		  uint32_t *elen, sector_t *offset)
> +/*
> + * return 0 when iudf_next_aext() loop success.
> + * return err < 0 and err != -ENODATA indicates error.
> + * return err == -ENODATA indicates hit EOF.
> + */
> +int inode_bmap(struct inode *inode, sector_t block, struct extent_position *pos,
> +	       struct kernel_lb_addr *eloc, uint32_t *elen, sector_t *offset,
> +	       int8_t *etype)
>  {
>  	unsigned char blocksize_bits = inode->i_sb->s_blocksize_bits;
>  	loff_t lbcount = 0, bcount = (loff_t) block << blocksize_bits;
> -	int8_t etype;
>  	struct udf_inode_info *iinfo;
>  	int err = 0;
>  
> @@ -2409,18 +2416,18 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
>  	}
>  	*elen = 0;
>  	do {
> -		err = udf_next_aext(inode, pos, eloc, elen, &etype, 1);
> +		err = udf_next_aext(inode, pos, eloc, elen, etype, 1);
>  		if (UDF_EXT_EOF(err)) {
>  			*offset = (bcount - lbcount) >> blocksize_bits;
>  			iinfo->i_lenExtents = lbcount;
>  		}
>  		if (err < 0)
> -			return -1;
> +			return err;
>  		lbcount += *elen;
>  	} while (lbcount <= bcount);
>  	/* update extent cache */
>  	udf_update_extent_cache(inode, lbcount - *elen, pos);
>  	*offset = (bcount + *elen - lbcount) >> blocksize_bits;
>  
> -	return etype;
> +	return 0;
>  }
> diff --git a/fs/udf/partition.c b/fs/udf/partition.c
> index af877991edc1..c441d4ae1f96 100644
> --- a/fs/udf/partition.c
> +++ b/fs/udf/partition.c
> @@ -282,9 +282,11 @@ static uint32_t udf_try_read_meta(struct inode *inode, uint32_t block,
>  	sector_t ext_offset;
>  	struct extent_position epos = {};
>  	uint32_t phyblock;
> +	int8_t etype;
> +	int err = 0;
>  
> -	if (inode_bmap(inode, block, &epos, &eloc, &elen, &ext_offset) !=
> -						(EXT_RECORDED_ALLOCATED >> 30))
> +	err = inode_bmap(inode, block, &epos, &eloc, &elen, &ext_offset, &etype);
> +	if (err || etype != (EXT_RECORDED_ALLOCATED >> 30))
>  		phyblock = 0xFFFFFFFF;
>  	else {
>  		map = &UDF_SB(sb)->s_partmaps[partition];
> diff --git a/fs/udf/truncate.c b/fs/udf/truncate.c
> index af06f7101859..d13ba9fd1309 100644
> --- a/fs/udf/truncate.c
> +++ b/fs/udf/truncate.c
> @@ -208,10 +208,12 @@ int udf_truncate_extents(struct inode *inode)
>  	else
>  		BUG();
>  
> -	etype = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset);
> +	err = inode_bmap(inode, first_block, &epos, &eloc, &elen, &offset, &etype);
>  	byte_offset = (offset << sb->s_blocksize_bits) +
> -		(inode->i_size & (sb->s_blocksize - 1));
> -	if (etype == -1) {
> +		      (inode->i_size & (sb->s_blocksize - 1));
> +	if (UDF_EXT_ERR(err))
> +		return err;
> +	if (UDF_EXT_EOF(err)) {
>  		/* We should extend the file? */
>  		WARN_ON(byte_offset);
>  		return 0;
> diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
> index 206077da9968..a156ed95189a 100644
> --- a/fs/udf/udfdecl.h
> +++ b/fs/udf/udfdecl.h
> @@ -160,8 +160,9 @@ extern struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
>  extern int udf_setsize(struct inode *, loff_t);
>  extern void udf_evict_inode(struct inode *);
>  extern int udf_write_inode(struct inode *, struct writeback_control *wbc);
> -extern int8_t inode_bmap(struct inode *, sector_t, struct extent_position *,
> -			 struct kernel_lb_addr *, uint32_t *, sector_t *);
> +extern int inode_bmap(struct inode *inode, sector_t block,
> +		      struct extent_position *pos, struct kernel_lb_addr *eloc,
> +		      uint32_t *elen, sector_t *offset, int8_t *etype);
>  int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
>  extern int udf_setup_indirect_aext(struct inode *inode, udf_pblk_t block,
>  				   struct extent_position *epos);
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR