| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20240927120933.okzud3qdygqplekm@quack3>
Date: Fri, 27 Sep 2024 14:09:33 +0200
From: Jan Kara <jack@...e.cz>
To: Zhao Mengmeng <zhaomzhao@....com>
Cc: jack@...e.com, zhaomengmeng@...inos.cn, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 3/3] udf: refactor inode_bmap() to handle error
On Thu 26-09-24 20:07:53, Zhao Mengmeng wrote:
> From: Zhao Mengmeng <zhaomengmeng@...inos.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@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=7a4842f0b1801230a989
> Tested-by: syzbot+7a4842f0b1801230a989@...kaller.appspotmail.com
> Signed-off-by: Zhao Mengmeng <zhaomengmeng@...inos.cn>
> Suggested-by: Jan Kara <jack@...e.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@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists