On Wed 04-12-24 09:32:26, Jakub Acs wrote:
> commit 32f123a3f34283f9c6446de87861696f0502b02e upstream.
>
> udf_getblk() has a single call site. Fold it there.
>
> Signed-off-by: Jan Kara <jack@xxxxxxx>
>
> [acsjakub: backport-adjusting changes]
> udf_getblk() has changed between 6.1 and the backported commit namely
> in commit 541e047b14c8 ("udf: Use udf_map_block() in udf_getblk()")
>
> Backport using the form of udf_getblk present in 6.1., that means use
> udf_get_block() instead of udf_map_block() and use dummy in buffer_new()
> and buffer_mapped().
>
> Closes: https://syzkaller.appspot.com/bug?extid=a38e34ca637c224f4a79
> Signed-off-by: Jakub Acs <acsjakub@xxxxxxxxx>
> ---
> While doing the backport I have noticed potential side effect of the
> upstream commit (present in the mainline):
>
> If we take the if-branch of 'if (map.oflags & UDF_BLK_NEW)', we will
> return the bh without the 'if (bh_read(bh, 0) >= 0)' check. Prior to
> the folding, the check wouldn't be skipped, was this intentional by the
> upstream commit?
Absolutely. bh_read() is pointless if you fill in the buffer contents
yourself (as we do in the 'if (map.oflags & UDF_BLK_NEW)' branch).
Honza
> ---
> fs/udf/inode.c | 46 +++++++++++++++++++++-------------------------
> 1 file changed, 21 insertions(+), 25 deletions(-)
>
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index d7d6ccd0af06..626450101412 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -369,29 +369,6 @@ static int udf_get_block(struct inode *inode, sector_t block,
> return err;
> }
>
> -static struct buffer_head *udf_getblk(struct inode *inode, udf_pblk_t block,
> - int create, int *err)
> -{
> - struct buffer_head *bh;
> - struct buffer_head dummy;
> -
> - dummy.b_state = 0;
> - dummy.b_blocknr = -1000;
> - *err = udf_get_block(inode, block, &dummy, create);
> - if (!*err && buffer_mapped(&dummy)) {
> - bh = sb_getblk(inode->i_sb, dummy.b_blocknr);
> - if (buffer_new(&dummy)) {
> - lock_buffer(bh);
> - memset(bh->b_data, 0x00, inode->i_sb->s_blocksize);
> - set_buffer_uptodate(bh);
> - unlock_buffer(bh);
> - mark_buffer_dirty_inode(bh, inode);
> - }
> - return bh;
> - }
> -
> - return NULL;
> -}
>
> /* Extend the file with new blocks totaling 'new_block_bytes',
> * return the number of extents added
> @@ -1108,10 +1085,29 @@ struct buffer_head *udf_bread(struct inode *inode, udf_pblk_t block,
> int create, int *err)
> {
> struct buffer_head *bh = NULL;
> + struct buffer_head dummy;
>
> - bh = udf_getblk(inode, block, create, err);
> - if (!bh)
> + dummy.b_state = 0;
> + dummy.b_blocknr = -1000;
> +
> + *err = udf_get_block(inode, block, &dummy, create);
> + if (*err || !buffer_mapped(&dummy))
> + return NULL
> +
> + bh = sb_getblk(inode->i_sb, dummy.b_blocknr);
> + if (!bh) {
> + *err = -ENOMEM;
> return NULL;
> + }
> +
> + if (buffer_new(&dummy)) {
> + lock_buffer(bh);
> + memset(bh->b_data, 0x00, inode->i_sb->s_blocksize);
> + set_buffer_uptodate(bh);
> + unlock_buffer(bh);
> + mark_buffer_dirty_inode(bh, inode);
> + return bh;
> + }
>
> if (bh_read(bh, 0) >= 0)
> return bh;
>
> base-commit: e4d90d63d385228b1e0bcf31cc15539bbbc28f7f
> --
> 2.40.1
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
