[PATCH 0/7] udf: Couple more fixes for extent and directory handling

[PATCH 0/7] udf: Couple more fixes for extent and directory handling

[Jan Kara]

Hello,

these patches fix couple of problems with handling of errors when filesystem
goes out of space while adding indirect extents to the file and also reduces
stack usage for directory iteration code.

I'll merge these fixes through my tree.

								Honza

[PATCH 1/7] udf: Handle error when expanding directory

[Jan Kara]

When there is an error when adding extent to the directory to expand it,
make sure to propagate the error up properly. This is not expected to
happen currently but let's make the code more futureproof.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/namei.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index 800271b00f84..de169feacce9 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -192,8 +192,13 @@ static struct buffer_head *udf_expand_dir_adinicb(struct inode *inode,
 	epos.bh = NULL;
 	epos.block = iinfo->i_location;
 	epos.offset = udf_file_entry_alloc_offset(inode);
-	udf_add_aext(inode, &epos, &eloc, inode->i_size, 0);
+	ret = udf_add_aext(inode, &epos, &eloc, inode->i_size, 0);
 	brelse(epos.bh);
+	if (ret < 0) {
+		*err = ret;
+		udf_free_blocks(inode->i_sb, inode, &eloc, 0, 1);
+		return NULL;
+	}
 	mark_inode_dirty(inode);
 
 	/* Now fixup tags in moved directory entries */
-- 
2.35.3

[PATCH 2/7] udf: Handle error when adding extent to symlink

[Jan Kara]

When adding extent describing symlink data fails, make sure to handle
the error properly, propagate it up and free the already allocated
block.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/namei.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/udf/namei.c b/fs/udf/namei.c
index de169feacce9..2ade040483a1 100644
--- a/fs/udf/namei.c
+++ b/fs/udf/namei.c
@@ -609,8 +609,12 @@ static int udf_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 				iinfo->i_location.partitionReferenceNum;
 		bsize = sb->s_blocksize;
 		iinfo->i_lenExtents = bsize;
-		udf_add_aext(inode, &epos, &eloc, bsize, 0);
+		err = udf_add_aext(inode, &epos, &eloc, bsize, 0);
 		brelse(epos.bh);
+		if (err < 0) {
+			udf_free_blocks(sb, inode, &eloc, 0, 1);
+			goto out_no_entry;
+		}
 
 		block = udf_get_pblock(sb, block,
 				iinfo->i_location.partitionReferenceNum,
@@ -618,6 +622,7 @@ static int udf_symlink(struct user_namespace *mnt_userns, struct inode *dir,
 		epos.bh = udf_tgetblk(sb, block);
 		if (unlikely(!epos.bh)) {
 			err = -ENOMEM;
+			udf_free_blocks(sb, inode, &eloc, 0, 1);
 			goto out_no_entry;
 		}
 		lock_buffer(epos.bh);
-- 
2.35.3

[PATCH 3/7] udf: Handle error when adding extent to a file

[Jan Kara]

When adding extent to a file fails, so far we've silently squelshed the
error. Make sure to propagate it up properly.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c | 41 +++++++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 09417342d8b6..15b3e529854b 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -57,15 +57,15 @@ static int udf_update_inode(struct inode *, int);
 static int udf_sync_inode(struct inode *inode);
 static int udf_alloc_i_data(struct inode *inode, size_t size);
 static sector_t inode_getblk(struct inode *, sector_t, int *, int *);
-static int8_t udf_insert_aext(struct inode *, struct extent_position,
-			      struct kernel_lb_addr, uint32_t);
+static int udf_insert_aext(struct inode *, struct extent_position,
+			   struct kernel_lb_addr, uint32_t);
 static void udf_split_extents(struct inode *, int *, int, udf_pblk_t,
 			      struct kernel_long_ad *, int *);
 static void udf_prealloc_extents(struct inode *, int, int,
 				 struct kernel_long_ad *, int *);
 static void udf_merge_extents(struct inode *, struct kernel_long_ad *, int *);
-static void udf_update_extents(struct inode *, struct kernel_long_ad *, int,
-			       int, struct extent_position *);
+static int udf_update_extents(struct inode *, struct kernel_long_ad *, int,
+			      int, struct extent_position *);
 static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
 
 static void __udf_clear_extent_cache(struct inode *inode)
@@ -795,7 +795,9 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 	/* write back the new extents, inserting new extents if the new number
 	 * of extents is greater than the old number, and deleting extents if
 	 * the new number of extents is less than the old number */
-	udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
+	*err = udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
+	if (*err < 0)
+		goto out_free;
 
 	newblock = udf_get_pblock(inode->i_sb, newblocknum,
 				iinfo->i_location.partitionReferenceNum, 0);
@@ -1063,21 +1065,30 @@ static void udf_merge_extents(struct inode *inode, struct kernel_long_ad *laarr,
 	}
 }
 
-static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
-			       int startnum, int endnum,
-			       struct extent_position *epos)
+static int udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
+			      int startnum, int endnum,
+			      struct extent_position *epos)
 {
 	int start = 0, i;
 	struct kernel_lb_addr tmploc;
 	uint32_t tmplen;
+	int err;
 
 	if (startnum > endnum) {
 		for (i = 0; i < (startnum - endnum); i++)
 			udf_delete_aext(inode, *epos);
 	} else if (startnum < endnum) {
 		for (i = 0; i < (endnum - startnum); i++) {
-			udf_insert_aext(inode, *epos, laarr[i].extLocation,
-					laarr[i].extLength);
+			err = udf_insert_aext(inode, *epos,
+					      laarr[i].extLocation,
+					      laarr[i].extLength);
+			/*
+			 * If we fail here, we are likely corrupting the extent
+ 			 * list and leaking blocks. At least stop early to
+			 * limit the damage.
+			 */
+			if (err < 0)
+				return err;
 			udf_next_aext(inode, epos, &laarr[i].extLocation,
 				      &laarr[i].extLength, 1);
 			start++;
@@ -1089,6 +1100,7 @@ static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr
 		udf_write_aext(inode, epos, &laarr[i].extLocation,
 			       laarr[i].extLength, 1);
 	}
+	return 0;
 }
 
 struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
@@ -2107,12 +2119,13 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
 	return etype;
 }
 
-static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
-			      struct kernel_lb_addr neloc, uint32_t nelen)
+static int udf_insert_aext(struct inode *inode, struct extent_position epos,
+			   struct kernel_lb_addr neloc, uint32_t nelen)
 {
 	struct kernel_lb_addr oeloc;
 	uint32_t oelen;
 	int8_t etype;
+	int err;
 
 	if (epos.bh)
 		get_bh(epos.bh);
@@ -2122,10 +2135,10 @@ static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
 		neloc = oeloc;
 		nelen = (etype << 30) | oelen;
 	}
-	udf_add_aext(inode, &epos, &neloc, nelen, 1);
+	err = udf_add_aext(inode, &epos, &neloc, nelen, 1);
 	brelse(epos.bh);
 
-	return (nelen >> 30);
+	return err;
 }
 
 int8_t udf_delete_aext(struct inode *inode, struct extent_position epos)
-- 
2.35.3

[PATCH 4/7] udf: Allocate name buffer in directory iterator on heap

[Jan Kara]

Currently we allocate name buffer in directory iterators (struct
udf_fileident_iter) on stack. These structures are relatively large
(some 360 bytes on 64-bit architectures). For udf_rename() which needs
to keep three of these structures in parallel the stack usage becomes
rather heavy - 1536 bytes in total. Allocate the name buffer in the
iterator from heap to avoid excessive stack usage.

Link: https://lore.kernel.org/all/202212200558.lK9x1KW0-lkp@intel.com
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/directory.c | 23 +++++++++++++++--------
 fs/udf/udfdecl.h   |  2 +-
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index 9e6a54445f90..0f3cc095b2a3 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -248,9 +248,14 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
 	iter->elen = 0;
 	iter->epos.bh = NULL;
 	iter->name = NULL;
+	iter->namebuf = kmalloc(UDF_NAME_LEN_CS0, GFP_KERNEL);
+	if (!iter->namebuf)
+		return -ENOMEM;
 
-	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB)
-		return udf_copy_fi(iter);
+	if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB) {
+		err = udf_copy_fi(iter);
+		goto out;
+	}
 
 	if (inode_bmap(dir, iter->pos >> dir->i_blkbits, &iter->epos,
 		       &iter->eloc, &iter->elen, &iter->loffset) !=
@@ -260,17 +265,17 @@ int udf_fiiter_init(struct udf_fileident_iter *iter, struct inode *dir,
 		udf_err(dir->i_sb,
 			"position %llu not allocated in directory (ino %lu)\n",
 			(unsigned long long)pos, dir->i_ino);
-		return -EFSCORRUPTED;
+		err = -EFSCORRUPTED;
+		goto out;
 	}
 	err = udf_fiiter_load_bhs(iter);
 	if (err < 0)
-		return err;
+		goto out;
 	err = udf_copy_fi(iter);
-	if (err < 0) {
+out:
+	if (err < 0)
 		udf_fiiter_release(iter);
-		return err;
-	}
-	return 0;
+	return err;
 }
 
 int udf_fiiter_advance(struct udf_fileident_iter *iter)
@@ -307,6 +312,8 @@ void udf_fiiter_release(struct udf_fileident_iter *iter)
 	brelse(iter->bh[0]);
 	brelse(iter->bh[1]);
 	iter->bh[0] = iter->bh[1] = NULL;
+	kfree(iter->namebuf);
+	iter->namebuf = NULL;
 }
 
 static void udf_copy_to_bufs(void *buf1, int len1, void *buf2, int len2,
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index f764b4d15094..d35aa42bb577 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -99,7 +99,7 @@ struct udf_fileident_iter {
 	struct extent_position epos;	/* Position after the above extent */
 	struct fileIdentDesc fi;	/* Copied directory entry */
 	uint8_t *name;			/* Pointer to entry name */
-	uint8_t namebuf[UDF_NAME_LEN_CS0]; /* Storage for entry name in case
+	uint8_t *namebuf;		/* Storage for entry name in case
 					 * the name is split between two blocks
 					 */
 };
-- 
2.35.3

[PATCH 5/7] udf: Move setting of i_lenExtents into udf_do_extend_file()

[Jan Kara]

When expanding file for a write into a hole, we were not updating total
length of inode's extents properly. Move the update of i_lenExtents into
udf_do_extend_file() so that both expanding of file by truncate and
expanding of file by writing beyond EOF properly update the length of
extents. As a bonus, we also correctly update the length of extents when
only part of extents can be written.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 15b3e529854b..fe5b0ba600fa 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -419,6 +419,7 @@ static int udf_do_extend_file(struct inode *inode,
 			~(sb->s_blocksize - 1);
 	}
 
+	add = 0;
 	/* Can we merge with the previous extent? */
 	if ((last_ext->extLength & UDF_EXTENT_FLAG_MASK) ==
 					EXT_NOT_RECORDED_NOT_ALLOCATED) {
@@ -451,6 +452,7 @@ static int udf_do_extend_file(struct inode *inode,
 		if (new_block_bytes)
 			udf_next_aext(inode, last_pos, &tmploc, &tmplen, 0);
 	}
+	iinfo->i_lenExtents += add;
 
 	/* Managed to do everything necessary? */
 	if (!new_block_bytes)
@@ -469,6 +471,7 @@ static int udf_do_extend_file(struct inode *inode,
 				   last_ext->extLength, 1);
 		if (err)
 			goto out_err;
+		iinfo->i_lenExtents += add;
 		count++;
 	}
 	if (new_block_bytes) {
@@ -478,6 +481,7 @@ static int udf_do_extend_file(struct inode *inode,
 				   last_ext->extLength, 1);
 		if (err)
 			goto out_err;
+		iinfo->i_lenExtents += new_block_bytes;
 		count++;
 	}
 
@@ -585,7 +589,6 @@ static int udf_extend_file(struct inode *inode, loff_t newsize)
 	if (err < 0)
 		goto out;
 	err = 0;
-	iinfo->i_lenExtents = newsize;
 out:
 	brelse(epos.bh);
 	return err;
-- 
2.35.3

[PATCH 6/7] udf: Fix extension of the last extent in the file

[Jan Kara]

When extending the last extent in the file within the last block, we
wrongly computed the length of the last extent. This is mostly a
cosmetical problem since the extent does not contain any data and the
length will be fixed up by following operations but still.

Fixes: 1f3868f06855 ("udf: Fix extending file within last block")
Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index fe5b0ba600fa..075e0a9d766c 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -516,7 +516,7 @@ static void udf_do_extend_final_block(struct inode *inode,
 	 */
 	if (new_elen <= (last_ext->extLength & UDF_EXTENT_LENGTH_MASK))
 		return;
-	added_bytes = (last_ext->extLength & UDF_EXTENT_LENGTH_MASK) - new_elen;
+	added_bytes = new_elen - (last_ext->extLength & UDF_EXTENT_LENGTH_MASK);
 	last_ext->extLength += added_bytes;
 	UDF_I(inode)->i_lenExtents += added_bytes;
 
-- 
2.35.3

[PATCH 7/7] udf: Keep i_lenExtents consistent with the total length of extents

[Jan Kara]

When rounding the last extent to blocksize in inode_getblk() we forgot
to update also i_lenExtents to match the new extent length. This
inconsistency can later confuse some assertion checks.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/inode.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 075e0a9d766c..3621dd7fe5a7 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -677,6 +677,9 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
 			elen = EXT_RECORDED_ALLOCATED |
 				((elen + inode->i_sb->s_blocksize - 1) &
 				 ~(inode->i_sb->s_blocksize - 1));
+			iinfo->i_lenExtents =
+				ALIGN(iinfo->i_lenExtents,
+				      inode->i_sb->s_blocksize);
 			udf_write_aext(inode, &cur_epos, &eloc, elen, 1);
 		}
 		newblock = udf_get_lb_pblock(inode->i_sb, &eloc, offset);
-- 
2.35.3

Re: [PATCH 3/7] udf: Handle error when adding extent to a file

[Nathan Chancellor]

On Thu, Dec 22, 2022 at 11:16:00AM +0100, Jan Kara wrote:
> When adding extent to a file fails, so far we've silently squelshed the
> error. Make sure to propagate it up properly.
> 
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
>  fs/udf/inode.c | 41 +++++++++++++++++++++++++++--------------
>  1 file changed, 27 insertions(+), 14 deletions(-)
> 
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 09417342d8b6..15b3e529854b 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -57,15 +57,15 @@ static int udf_update_inode(struct inode *, int);
>  static int udf_sync_inode(struct inode *inode);
>  static int udf_alloc_i_data(struct inode *inode, size_t size);
>  static sector_t inode_getblk(struct inode *, sector_t, int *, int *);
> -static int8_t udf_insert_aext(struct inode *, struct extent_position,
> -			      struct kernel_lb_addr, uint32_t);
> +static int udf_insert_aext(struct inode *, struct extent_position,
> +			   struct kernel_lb_addr, uint32_t);
>  static void udf_split_extents(struct inode *, int *, int, udf_pblk_t,
>  			      struct kernel_long_ad *, int *);
>  static void udf_prealloc_extents(struct inode *, int, int,
>  				 struct kernel_long_ad *, int *);
>  static void udf_merge_extents(struct inode *, struct kernel_long_ad *, int *);
> -static void udf_update_extents(struct inode *, struct kernel_long_ad *, int,
> -			       int, struct extent_position *);
> +static int udf_update_extents(struct inode *, struct kernel_long_ad *, int,
> +			      int, struct extent_position *);
>  static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
>  
>  static void __udf_clear_extent_cache(struct inode *inode)
> @@ -795,7 +795,9 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
>  	/* write back the new extents, inserting new extents if the new number
>  	 * of extents is greater than the old number, and deleting extents if
>  	 * the new number of extents is less than the old number */
> -	udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
> +	*err = udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
> +	if (*err < 0)
> +		goto out_free;

This patch in next-20221226 as commit d8b39db5fab8 ("udf: Handle error when
adding extent to a file") causes the following clang warning:

  fs/udf/inode.c:805:6: error: variable 'newblock' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
          if (*err < 0)
              ^~~~~~~~
  fs/udf/inode.c:827:9: note: uninitialized use occurs here
          return newblock;
                 ^~~~~~~~
  fs/udf/inode.c:805:2: note: remove the 'if' if its condition is always false
          if (*err < 0)
          ^~~~~~~~~~~~~
  fs/udf/inode.c:607:34: note: initialize the variable 'newblock' to silence this warning
          udf_pblk_t newblocknum, newblock;
                                          ^
                                           = 0
  1 error generated.

>  	newblock = udf_get_pblock(inode->i_sb, newblocknum,
>  				iinfo->i_location.partitionReferenceNum, 0);
> @@ -1063,21 +1065,30 @@ static void udf_merge_extents(struct inode *inode, struct kernel_long_ad *laarr,
>  	}
>  }
>  
> -static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
> -			       int startnum, int endnum,
> -			       struct extent_position *epos)
> +static int udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
> +			      int startnum, int endnum,
> +			      struct extent_position *epos)
>  {
>  	int start = 0, i;
>  	struct kernel_lb_addr tmploc;
>  	uint32_t tmplen;
> +	int err;
>  
>  	if (startnum > endnum) {
>  		for (i = 0; i < (startnum - endnum); i++)
>  			udf_delete_aext(inode, *epos);
>  	} else if (startnum < endnum) {
>  		for (i = 0; i < (endnum - startnum); i++) {
> -			udf_insert_aext(inode, *epos, laarr[i].extLocation,
> -					laarr[i].extLength);
> +			err = udf_insert_aext(inode, *epos,
> +					      laarr[i].extLocation,
> +					      laarr[i].extLength);
> +			/*
> +			 * If we fail here, we are likely corrupting the extent
> + 			 * list and leaking blocks. At least stop early to
> +			 * limit the damage.
> +			 */
> +			if (err < 0)
> +				return err;
>  			udf_next_aext(inode, epos, &laarr[i].extLocation,
>  				      &laarr[i].extLength, 1);
>  			start++;
> @@ -1089,6 +1100,7 @@ static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr
>  		udf_write_aext(inode, epos, &laarr[i].extLocation,
>  			       laarr[i].extLength, 1);
>  	}
> +	return 0;
>  }
>  
>  struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
> @@ -2107,12 +2119,13 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
>  	return etype;
>  }
>  
> -static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
> -			      struct kernel_lb_addr neloc, uint32_t nelen)
> +static int udf_insert_aext(struct inode *inode, struct extent_position epos,
> +			   struct kernel_lb_addr neloc, uint32_t nelen)
>  {
>  	struct kernel_lb_addr oeloc;
>  	uint32_t oelen;
>  	int8_t etype;
> +	int err;
>  
>  	if (epos.bh)
>  		get_bh(epos.bh);
> @@ -2122,10 +2135,10 @@ static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
>  		neloc = oeloc;
>  		nelen = (etype << 30) | oelen;
>  	}
> -	udf_add_aext(inode, &epos, &neloc, nelen, 1);
> +	err = udf_add_aext(inode, &epos, &neloc, nelen, 1);
>  	brelse(epos.bh);
>  
> -	return (nelen >> 30);
> +	return err;
>  }
>  
>  int8_t udf_delete_aext(struct inode *inode, struct extent_position epos)
> -- 
> 2.35.3
> 
> 

Re: [PATCH 3/7] udf: Handle error when adding extent to a file

[Jan Kara]

On Sun 25-12-22 21:28:43, Nathan Chancellor wrote:
> On Thu, Dec 22, 2022 at 11:16:00AM +0100, Jan Kara wrote:
> > When adding extent to a file fails, so far we've silently squelshed the
> > error. Make sure to propagate it up properly.
> > 
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  fs/udf/inode.c | 41 +++++++++++++++++++++++++++--------------
> >  1 file changed, 27 insertions(+), 14 deletions(-)
> > 
> > diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> > index 09417342d8b6..15b3e529854b 100644
> > --- a/fs/udf/inode.c
> > +++ b/fs/udf/inode.c
> > @@ -57,15 +57,15 @@ static int udf_update_inode(struct inode *, int);
> >  static int udf_sync_inode(struct inode *inode);
> >  static int udf_alloc_i_data(struct inode *inode, size_t size);
> >  static sector_t inode_getblk(struct inode *, sector_t, int *, int *);
> > -static int8_t udf_insert_aext(struct inode *, struct extent_position,
> > -			      struct kernel_lb_addr, uint32_t);
> > +static int udf_insert_aext(struct inode *, struct extent_position,
> > +			   struct kernel_lb_addr, uint32_t);
> >  static void udf_split_extents(struct inode *, int *, int, udf_pblk_t,
> >  			      struct kernel_long_ad *, int *);
> >  static void udf_prealloc_extents(struct inode *, int, int,
> >  				 struct kernel_long_ad *, int *);
> >  static void udf_merge_extents(struct inode *, struct kernel_long_ad *, int *);
> > -static void udf_update_extents(struct inode *, struct kernel_long_ad *, int,
> > -			       int, struct extent_position *);
> > +static int udf_update_extents(struct inode *, struct kernel_long_ad *, int,
> > +			      int, struct extent_position *);
> >  static int udf_get_block(struct inode *, sector_t, struct buffer_head *, int);
> >  
> >  static void __udf_clear_extent_cache(struct inode *inode)
> > @@ -795,7 +795,9 @@ static sector_t inode_getblk(struct inode *inode, sector_t block,
> >  	/* write back the new extents, inserting new extents if the new number
> >  	 * of extents is greater than the old number, and deleting extents if
> >  	 * the new number of extents is less than the old number */
> > -	udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
> > +	*err = udf_update_extents(inode, laarr, startnum, endnum, &prev_epos);
> > +	if (*err < 0)
> > +		goto out_free;
> 
> This patch in next-20221226 as commit d8b39db5fab8 ("udf: Handle error when
> adding extent to a file") causes the following clang warning:
> 
>   fs/udf/inode.c:805:6: error: variable 'newblock' is used uninitialized whenever 'if' condition is true [-Werror,-Wsometimes-uninitialized]
>           if (*err < 0)
>               ^~~~~~~~
>   fs/udf/inode.c:827:9: note: uninitialized use occurs here
>           return newblock;
>                  ^~~~~~~~
>   fs/udf/inode.c:805:2: note: remove the 'if' if its condition is always false
>           if (*err < 0)
>           ^~~~~~~~~~~~~
>   fs/udf/inode.c:607:34: note: initialize the variable 'newblock' to silence this warning
>           udf_pblk_t newblocknum, newblock;
>                                           ^
>                                            = 0
>   1 error generated.

Thanks for the report. It should be fixed now.

								Honza


> 
> >  	newblock = udf_get_pblock(inode->i_sb, newblocknum,
> >  				iinfo->i_location.partitionReferenceNum, 0);
> > @@ -1063,21 +1065,30 @@ static void udf_merge_extents(struct inode *inode, struct kernel_long_ad *laarr,
> >  	}
> >  }
> >  
> > -static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
> > -			       int startnum, int endnum,
> > -			       struct extent_position *epos)
> > +static int udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr,
> > +			      int startnum, int endnum,
> > +			      struct extent_position *epos)
> >  {
> >  	int start = 0, i;
> >  	struct kernel_lb_addr tmploc;
> >  	uint32_t tmplen;
> > +	int err;
> >  
> >  	if (startnum > endnum) {
> >  		for (i = 0; i < (startnum - endnum); i++)
> >  			udf_delete_aext(inode, *epos);
> >  	} else if (startnum < endnum) {
> >  		for (i = 0; i < (endnum - startnum); i++) {
> > -			udf_insert_aext(inode, *epos, laarr[i].extLocation,
> > -					laarr[i].extLength);
> > +			err = udf_insert_aext(inode, *epos,
> > +					      laarr[i].extLocation,
> > +					      laarr[i].extLength);
> > +			/*
> > +			 * If we fail here, we are likely corrupting the extent
> > + 			 * list and leaking blocks. At least stop early to
> > +			 * limit the damage.
> > +			 */
> > +			if (err < 0)
> > +				return err;
> >  			udf_next_aext(inode, epos, &laarr[i].extLocation,
> >  				      &laarr[i].extLength, 1);
> >  			start++;
> > @@ -1089,6 +1100,7 @@ static void udf_update_extents(struct inode *inode, struct kernel_long_ad *laarr
> >  		udf_write_aext(inode, epos, &laarr[i].extLocation,
> >  			       laarr[i].extLength, 1);
> >  	}
> > +	return 0;
> >  }
> >  
> >  struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
> > @@ -2107,12 +2119,13 @@ int8_t udf_current_aext(struct inode *inode, struct extent_position *epos,
> >  	return etype;
> >  }
> >  
> > -static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
> > -			      struct kernel_lb_addr neloc, uint32_t nelen)
> > +static int udf_insert_aext(struct inode *inode, struct extent_position epos,
> > +			   struct kernel_lb_addr neloc, uint32_t nelen)
> >  {
> >  	struct kernel_lb_addr oeloc;
> >  	uint32_t oelen;
> >  	int8_t etype;
> > +	int err;
> >  
> >  	if (epos.bh)
> >  		get_bh(epos.bh);
> > @@ -2122,10 +2135,10 @@ static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
> >  		neloc = oeloc;
> >  		nelen = (etype << 30) | oelen;
> >  	}
> > -	udf_add_aext(inode, &epos, &neloc, nelen, 1);
> > +	err = udf_add_aext(inode, &epos, &neloc, nelen, 1);
> >  	brelse(epos.bh);
> >  
> > -	return (nelen >> 30);
> > +	return err;
> >  }
> >  
> >  int8_t udf_delete_aext(struct inode *inode, struct extent_position epos)
> > -- 
> > 2.35.3
> > 
> > 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR