Re: [PATCH v2 6.1] udf: Fold udf_getblk() into udf_bread()

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@suse.cz

[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@amazon.de

---

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