From: Zhao Mengmeng <zhaomengmeng@kylinos.cn>
Same as udf_current_aext(), take pointer to etype to store the extent
type, while return 0 for success and <0 on error. On situations like
ftruncate, udf_extend_file() can 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>
---
fs/udf/directory.c | 13 ++++++++-----
fs/udf/inode.c | 31 ++++++++++++++++++++-----------
fs/udf/partition.c | 6 ++++--
fs/udf/truncate.c | 5 +++--
fs/udf/udfdecl.h | 5 +++--
5 files changed, 38 insertions(+), 22 deletions(-)
diff --git a/fs/udf/directory.c b/fs/udf/directory.c
index f865538c985d..cb40e1ade9f6 100644
--- a/fs/udf/directory.c
+++ b/fs/udf/directory.c
@@ -243,6 +243,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;
@@ -262,9 +263,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,
@@ -460,6 +461,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;
@@ -474,8 +476,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 ba980ce5e13a..f1b8f0a0d202 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)) {
+ inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset, &etype);
+ if (etype == (EXT_RECORDED_ALLOCATED >> 30)) {
map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc,
offset);
map->oflags |= UDF_BLK_MAPPED;
@@ -660,8 +661,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 (err < 0 && err != -ENODATA)
+ 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);
@@ -2363,14 +2366,19 @@ 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;
iinfo = UDF_I(inode);
if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
@@ -2380,10 +2388,11 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
}
*elen = 0;
do {
- if (udf_next_aext(inode, pos, eloc, elen, &etype, 1)) {
+ err = udf_next_aext(inode, pos, eloc, elen, etype, 1);
+ if (err < 0) {
*offset = (bcount - lbcount) >> blocksize_bits;
iinfo->i_lenExtents = lbcount;
- return -1;
+ return err;
}
lbcount += *elen;
} while (lbcount <= bcount);
@@ -2391,5 +2400,5 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
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 b7361222f988..a70b6ae4ab8a 100644
--- a/fs/udf/truncate.c
+++ b/fs/udf/truncate.c
@@ -188,6 +188,7 @@ int udf_truncate_extents(struct inode *inode)
loff_t byte_offset;
int adsize;
struct udf_inode_info *iinfo = UDF_I(inode);
+ int err = 0;
if (iinfo->i_alloc_type == ICBTAG_FLAG_AD_SHORT)
adsize = sizeof(struct short_ad);
@@ -196,10 +197,10 @@ 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) {
+ if (err < 0) {
/* We should extend the file? */
WARN_ON(byte_offset);
return 0;
diff --git a/fs/udf/udfdecl.h b/fs/udf/udfdecl.h
index 5067ed68a8b4..d159f20d61e8 100644
--- a/fs/udf/udfdecl.h
+++ b/fs/udf/udfdecl.h
@@ -157,8 +157,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.0On Wed 18-09-24 17:36:34, Zhao Mengmeng wrote:
> From: Zhao Mengmeng <zhaomengmeng@kylinos.cn>
>
> Same as udf_current_aext(), take pointer to etype to store the extent
> type, while return 0 for success and <0 on error. On situations like
> ftruncate, udf_extend_file() can 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>
Overall looks good. I have a few comments below.
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index ba980ce5e13a..f1b8f0a0d202 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)) {
> + inode_bmap(inode, map->lblk, &epos, &eloc, &elen, &offset, &etype);
Here we should be checking for error and returning it...
> + if (etype == (EXT_RECORDED_ALLOCATED >> 30)) {
> map->pblk = udf_get_lb_pblock(inode->i_sb, &eloc,
> offset);
> map->oflags |= UDF_BLK_MAPPED;
> @@ -660,8 +661,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 (err < 0 && err != -ENODATA)
> + 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);
> @@ -2363,14 +2366,19 @@ 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;
>
> iinfo = UDF_I(inode);
> if (!udf_read_extent_cache(inode, bcount, &lbcount, pos)) {
> @@ -2380,10 +2388,11 @@ int8_t inode_bmap(struct inode *inode, sector_t block,
> }
> *elen = 0;
> do {
> - if (udf_next_aext(inode, pos, eloc, elen, &etype, 1)) {
> + err = udf_next_aext(inode, pos, eloc, elen, etype, 1);
> + if (err < 0) {
OK, you've added the error handling in this patch. That is good. But still
we should be setting offset and i_lenExtents only in -ENODATA case.
Otherwise we just want to return the error.
> *offset = (bcount - lbcount) >> blocksize_bits;
> iinfo->i_lenExtents = lbcount;
> - return -1;
> + return err;
> }
...
> @@ -196,10 +197,10 @@ 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) {
> + if (err < 0) {
Here we should be distinguisting -ENODATA (WARN and return 0) and other
error (which we need to return).
Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR
© 2016 - 2024 Red Hat, Inc.