[PATCH] udf: Check return code from udf_update_extents

Vladislav Efanov posted 1 patch 1 year, 10 months ago
fs/udf/inode.c | 26 +++++++++++++++++++++-----
1 file changed, 21 insertions(+), 5 deletions(-)
[PATCH] udf: Check return code from udf_update_extents
Posted by Vladislav Efanov 1 year, 10 months ago
udf_add_aext() does not create new extent and returns ENOSPC if new
block was not created by udf_bitmap_new_block(). The caller,
udf_insert_aext(), does not check this return code and returns OK to
its caller(udf_update_extents). Finally the error is being lost. So
an inconsistency in inode.i_size and extents length becomes.

Later this inconsistency leads to WARNING:

WARNING: CPU: 3 PID: 1104 at fs/udf/truncate.c:226
        udf_truncate_extents+0x7e0/0x8e0 fs/udf/truncate.c:226

RIP: 0010:udf_truncate_extents+0x7e0/0x8e0 fs/udf/truncate.c:226
Call Trace:
 udf_write_failed.isra.0+0x173/0x1c0 fs/udf/inode.c:179
 udf_write_begin+0x8d/0xb0 fs/udf/inode.c:214
 generic_perform_write+0x20a/0x4e0 mm/filemap.c:3333
 __generic_file_write_iter+0x252/0x610 mm/filemap.c:3462
 udf_file_write_iter+0x2cc/0x4e0 fs/udf/file.c:168
 call_write_iter include/linux/fs.h:1904 [inline]
 new_sync_write+0x42c/0x660 fs/read_write.c:518
 vfs_write+0x75b/0xa40 fs/read_write.c:605

Found by Linux Verification Center (linuxtesting.org) with syzkaller.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Vladislav Efanov <VEfanov@ispras.ru>
---
 fs/udf/inode.c | 26 +++++++++++++++++++++-----
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/fs/udf/inode.c b/fs/udf/inode.c
index 34e416327dd4..74b8695f23ca 100644
--- a/fs/udf/inode.c
+++ b/fs/udf/inode.c
@@ -64,7 +64,7 @@ static void udf_split_extents(struct inode *, int *, int, udf_pblk_t,
 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,
+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);
 
@@ -876,7 +876,11 @@ 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);
+	if (udf_update_extents(inode, laarr, startnum, endnum, &prev_epos) < 0) {
+		udf_debug("udf_update_extents rc != 0");
+		*err = -ENOSPC;
+		goto out_free;
+	}
 
 	newblock = udf_get_pblock(inode->i_sb, newblocknum,
 				iinfo->i_location.partitionReferenceNum, 0);
@@ -1159,21 +1163,26 @@ 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,
+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 rc;
 
 	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,
+			rc = udf_insert_aext(inode, *epos, laarr[i].extLocation,
 					laarr[i].extLength);
+			if (rc < 0) {
+				udf_debug("udfd_insert_aext.rc = %d", rc);
+				return rc;
+			}
 			udf_next_aext(inode, epos, &laarr[i].extLocation,
 				      &laarr[i].extLength, 1);
 			start++;
@@ -1185,6 +1194,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,
@@ -2209,6 +2219,7 @@ static int8_t udf_insert_aext(struct inode *inode, struct extent_position epos,
 	struct kernel_lb_addr oeloc;
 	uint32_t oelen;
 	int8_t etype;
+	int rc;
 
 	if (epos.bh)
 		get_bh(epos.bh);
@@ -2218,7 +2229,12 @@ 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);
+	rc = udf_add_aext(inode, &epos, &neloc, nelen, 1);
+	if (rc) {
+		udf_debug("udf_add_aext.rc = %d", rc);
+		brelse(epos.bh);
+		return rc;
+	}
 	brelse(epos.bh);
 
 	return (nelen >> 30);
-- 
2.34.1
Re: [PATCH] udf: Check return code from udf_update_extents
Posted by Jan Kara 1 year, 10 months ago
On Fri 20-01-23 12:10:28, Vladislav Efanov wrote:
> udf_add_aext() does not create new extent and returns ENOSPC if new
> block was not created by udf_bitmap_new_block(). The caller,
> udf_insert_aext(), does not check this return code and returns OK to
> its caller(udf_update_extents). Finally the error is being lost. So
> an inconsistency in inode.i_size and extents length becomes.
> 
> Later this inconsistency leads to WARNING:
> 
> WARNING: CPU: 3 PID: 1104 at fs/udf/truncate.c:226
>         udf_truncate_extents+0x7e0/0x8e0 fs/udf/truncate.c:226
> 
> RIP: 0010:udf_truncate_extents+0x7e0/0x8e0 fs/udf/truncate.c:226
> Call Trace:
>  udf_write_failed.isra.0+0x173/0x1c0 fs/udf/inode.c:179
>  udf_write_begin+0x8d/0xb0 fs/udf/inode.c:214
>  generic_perform_write+0x20a/0x4e0 mm/filemap.c:3333
>  __generic_file_write_iter+0x252/0x610 mm/filemap.c:3462
>  udf_file_write_iter+0x2cc/0x4e0 fs/udf/file.c:168
>  call_write_iter include/linux/fs.h:1904 [inline]
>  new_sync_write+0x42c/0x660 fs/read_write.c:518
>  vfs_write+0x75b/0xa40 fs/read_write.c:605
> 
> Found by Linux Verification Center (linuxtesting.org) with syzkaller.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Vladislav Efanov <VEfanov@ispras.ru>

Thanks for the fix but I have a very similar fix already queued in my tree
in linux-next: 19fd80de0a8 ("udf: Handle error when adding extent to a
file").

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR
Re: [PATCH] udf: Check return code from udf_update_extents
Posted by Ефанов Владислав Александрович 1 year, 10 months ago
Thank you for information.


Vlad.

On 24.01.2023 11:47, Jan Kara wrote:
> On Fri 20-01-23 12:10:28, Vladislav Efanov wrote:
>> udf_add_aext() does not create new extent and returns ENOSPC if new
>> block was not created by udf_bitmap_new_block(). The caller,
>> udf_insert_aext(), does not check this return code and returns OK to
>> its caller(udf_update_extents). Finally the error is being lost. So
>> an inconsistency in inode.i_size and extents length becomes.
>>
>> Later this inconsistency leads to WARNING:
>>
>> WARNING: CPU: 3 PID: 1104 at fs/udf/truncate.c:226
>>          udf_truncate_extents+0x7e0/0x8e0 fs/udf/truncate.c:226
>>
>> RIP: 0010:udf_truncate_extents+0x7e0/0x8e0 fs/udf/truncate.c:226
>> Call Trace:
>>   udf_write_failed.isra.0+0x173/0x1c0 fs/udf/inode.c:179
>>   udf_write_begin+0x8d/0xb0 fs/udf/inode.c:214
>>   generic_perform_write+0x20a/0x4e0 mm/filemap.c:3333
>>   __generic_file_write_iter+0x252/0x610 mm/filemap.c:3462
>>   udf_file_write_iter+0x2cc/0x4e0 fs/udf/file.c:168
>>   call_write_iter include/linux/fs.h:1904 [inline]
>>   new_sync_write+0x42c/0x660 fs/read_write.c:518
>>   vfs_write+0x75b/0xa40 fs/read_write.c:605
>>
>> Found by Linux Verification Center (linuxtesting.org) with syzkaller.
>>
>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>> Signed-off-by: Vladislav Efanov <VEfanov@ispras.ru>
> 
> Thanks for the fix but I have a very similar fix already queued in my tree
> in linux-next: 19fd80de0a8 ("udf: Handle error when adding extent to a
> file").
> 
> 								Honza