Messages in this thread | | | Date | Mon, 24 Oct 2022 17:38:36 +0200 | From | Jan Kara <> | Subject | Re: [PATCH] udf: Avoid double brelse() in udf_rename() |
| |
On Sun 23-10-22 18:57:41, Shigeru Yoshida wrote: > syzbot reported a warning like below [1]: > > VFS: brelse: Trying to free free buffer > WARNING: CPU: 2 PID: 7301 at fs/buffer.c:1145 __brelse+0x67/0xa0 > ... > Call Trace: > <TASK> > invalidate_bh_lru+0x99/0x150 > smp_call_function_many_cond+0xe2a/0x10c0 > ? generic_remap_file_range_prep+0x50/0x50 > ? __brelse+0xa0/0xa0 > ? __mutex_lock+0x21c/0x12d0 > ? smp_call_on_cpu+0x250/0x250 > ? rcu_read_lock_sched_held+0xb/0x60 > ? lock_release+0x587/0x810 > ? __brelse+0xa0/0xa0 > ? generic_remap_file_range_prep+0x50/0x50 > on_each_cpu_cond_mask+0x3c/0x80 > blkdev_flush_mapping+0x13a/0x2f0 > blkdev_put_whole+0xd3/0xf0 > blkdev_put+0x222/0x760 > deactivate_locked_super+0x96/0x160 > deactivate_super+0xda/0x100 > cleanup_mnt+0x222/0x3d0 > task_work_run+0x149/0x240 > ? task_work_cancel+0x30/0x30 > do_exit+0xb29/0x2a40 > ? reacquire_held_locks+0x4a0/0x4a0 > ? do_raw_spin_lock+0x12a/0x2b0 > ? mm_update_next_owner+0x7c0/0x7c0 > ? rwlock_bug.part.0+0x90/0x90 > ? zap_other_threads+0x234/0x2d0 > do_group_exit+0xd0/0x2a0 > __x64_sys_exit_group+0x3a/0x50 > do_syscall_64+0x34/0xb0 > entry_SYSCALL_64_after_hwframe+0x63/0xcd > > The cause of the issue is that brelse() is called on both ofibh.sbh > and ofibh.ebh by udf_find_entry() when it returns NULL. However, > brelse() is called by udf_rename(), too. So, b_count on buffer_head > becomes unbalanced. > > This patch fixes the issue by not calling brelse() by udf_rename() > when udf_find_entry() returns NULL. > > Link: https://syzkaller.appspot.com/bug?id=8297f45698159c6bca8a1f87dc983667c1a1c851 [1] > Reported-by: syzbot+7902cd7684bc35306224@syzkaller.appspotmail.com > Signed-off-by: Shigeru Yoshida <syoshida@redhat.com>
Thanks! I've added the fix to my tree.
Honza
> --- > fs/udf/namei.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/udf/namei.c b/fs/udf/namei.c > index fb4c30e05245..d6081538bfc0 100644 > --- a/fs/udf/namei.c > +++ b/fs/udf/namei.c > @@ -1091,8 +1091,9 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir, > return -EINVAL; > > ofi = udf_find_entry(old_dir, &old_dentry->d_name, &ofibh, &ocfi); > - if (IS_ERR(ofi)) { > - retval = PTR_ERR(ofi); > + if (!ofi || IS_ERR(ofi)) { > + if (IS_ERR(ofi)) > + retval = PTR_ERR(ofi); > goto end_rename; > } > > @@ -1101,8 +1102,7 @@ static int udf_rename(struct user_namespace *mnt_userns, struct inode *old_dir, > > brelse(ofibh.sbh); > tloc = lelb_to_cpu(ocfi.icb.extLocation); > - if (!ofi || udf_get_lb_pblock(old_dir->i_sb, &tloc, 0) > - != old_inode->i_ino) > + if (udf_get_lb_pblock(old_dir->i_sb, &tloc, 0) != old_inode->i_ino) > goto end_rename; > > nfi = udf_find_entry(new_dir, &new_dentry->d_name, &nfibh, &ncfi); > -- > 2.37.3 > -- Jan Kara <jack@suse.com> SUSE Labs, CR
| |