[syzbot] [ocfs2?] WARNING: bad unlock balance in ocfs2_read_blocks

[syzbot] [ocfs2?] WARNING: bad unlock balance in ocfs2_read_blocks

[syzbot]

Hello,

syzbot found the following issue on:

HEAD commit:    c3f2d783a459 Merge tag 'mm-hotfixes-stable-2024-08-17-19-3..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=102f82dd980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7229118d88b4a71b
dashboard link: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=11a0ec05980000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=13ac3093980000

Downloadable assets:
disk image (non-bootable): https://storage.googleapis.com/syzbot-assets/7bc7510fe41f/non_bootable_disk-c3f2d783.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/4d927f7c3cfd/vmlinux-c3f2d783.xz
kernel image: https://storage.googleapis.com/syzbot-assets/ea54bdfad24b/bzImage-c3f2d783.xz
mounted in repro #1: https://storage.googleapis.com/syzbot-assets/f0e7bccc5087/mount_0.gz
mounted in repro #2: https://storage.googleapis.com/syzbot-assets/5861bd127de9/mount_1.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com

loop0: detected capacity change from 0 to 9061
(syz-executor268,5095,0):ocfs2_read_blocks:240 ERROR: status = -12
=====================================
WARNING: bad unlock balance detected!
6.11.0-rc3-syzkaller-00338-gc3f2d783a459 #0 Not tainted
-------------------------------------
syz-executor268/5095 is trying to release lock (&oi->ip_io_mutex) at:
[<ffffffff837f37f6>] ocfs2_read_blocks+0x11c6/0x1620 fs/ocfs2/buffer_head_io.c:394
but there are no more locks to release!

other info that might help us debug this:
1 lock held by syz-executor268/5095:
 #0: ffff8880207680e0 (&type->s_umount_key#43/1){+.+.}-{3:3}, at: alloc_super+0x221/0x9d0 fs/super.c:344

stack backtrace:
CPU: 0 UID: 0 PID: 5095 Comm: syz-executor268 Not tainted 6.11.0-rc3-syzkaller-00338-gc3f2d783a459 #0
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-debian-1.16.3-2~bpo12+1 04/01/2014
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:93 [inline]
 dump_stack_lvl+0x241/0x360 lib/dump_stack.c:119
 print_unlock_imbalance_bug+0x256/0x2c0 kernel/locking/lockdep.c:5199
 __lock_release kernel/locking/lockdep.c:5436 [inline]
 lock_release+0x5cb/0xa30 kernel/locking/lockdep.c:5780
 __mutex_unlock_slowpath+0xe2/0x750 kernel/locking/mutex.c:912
 ocfs2_read_blocks+0x11c6/0x1620 fs/ocfs2/buffer_head_io.c:394
 ocfs2_map_slot_buffers fs/ocfs2/slot_map.c:385 [inline]
 ocfs2_init_slot_info+0xb35/0x13d0 fs/ocfs2/slot_map.c:424
 ocfs2_initialize_super fs/ocfs2/super.c:2274 [inline]
 ocfs2_fill_super+0x31b7/0x5880 fs/ocfs2/super.c:994
 mount_bdev+0x20a/0x2d0 fs/super.c:1679
 legacy_get_tree+0xee/0x190 fs/fs_context.c:662
 vfs_get_tree+0x90/0x2a0 fs/super.c:1800
 do_new_mount+0x2be/0xb40 fs/namespace.c:3472
 do_mount fs/namespace.c:3812 [inline]
 __do_sys_mount fs/namespace.c:4020 [inline]
 __se_sys_mount+0x2d6/0x3c0 fs/namespace.c:3997
 do_syscall_x64 arch/x86/entry/common.c:52 [inline]
 do_syscall_64+0xf3/0x230 arch/x86/entry/common.c:83
 entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7f0669b38cea
Code: d8 64 89 02 48 c7 c0 ff ff ff ff eb a6 e8 5e 04 00 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f0669aed048 EFLAGS: 00000282 ORIG_RAX: 00000000000000a5
RAX: ffffffffffffffda RBX: 0000000020000100 RCX: 00007f0669b38cea
RDX: 0000000020000100 RSI: 00000000200044c0 RDI: 00007f0669aed0a0
RBP: 00000000200044c0 R08: 00007f0669aed0e0 R09: 00000000ffffffe8
R10: 0000000000000000 R11: 0000000000000282 R12: 00007f0669aed0e0
R13: 000000000000447d R14: 0000000000000000 R15: 0000000020002340
 </TASK>
(syz-executor268,5095,0):ocfs2_map_slot_buffers:388 ERROR: status = -12
(syz-executor268,5095,0):ocfs2_init_slot_info:426 ERROR: status = -12
(syz-executor268,5095,0):ocfs2_initialize_super:2276 ERROR: status = -12
(syz-executor268,5095,0):ocfs2_fill_super:1178 ERROR: status = -12


---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzkaller@googlegroups.com.

syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.

If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title

If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.

If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)

If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report

If you want to undo deduplication, reply with:
#syz undup

Re: [syzbot] Re: [syzbot] [ocfs2?] WARNING: bad unlock balance in ocfs2_read_blocks

[syzbot]

For archival purposes, forwarding an incoming command email to
linux-kernel@vger.kernel.org.

***

Subject: Re: [syzbot] [ocfs2?] WARNING: bad unlock balance in ocfs2_read_blocks
Author: lizhi.xu@windriver.com

remove metadata io lock when sb getblk fail

#syz test: upstream c3f2d783a459

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index cdb9b9bdea1f..e62c7e1de4eb 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		if (bhs[i] == NULL) {
 			bhs[i] = sb_getblk(sb, block++);
 			if (bhs[i] == NULL) {
-				ocfs2_metadata_cache_io_unlock(ci);
 				status = -ENOMEM;
 				mlog_errno(status);
 				/* Don't forget to put previous bh! */

[PATCH] ocfs2: remove unreasonable unlock

[Lizhi Xu]

There was a lock release before exiting, so remove the unreasonable unlock.

Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ocfs2/buffer_head_io.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index cdb9b9bdea1f..e62c7e1de4eb 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		if (bhs[i] == NULL) {
 			bhs[i] = sb_getblk(sb, block++);
 			if (bhs[i] == NULL) {
-				ocfs2_metadata_cache_io_unlock(ci);
 				status = -ENOMEM;
 				mlog_errno(status);
 				/* Don't forget to put previous bh! */
-- 
2.43.0

Re: [PATCH] ocfs2: remove unreasonable unlock

[heming.zhao]

On 8/19/24 10:51, Lizhi Xu wrote:
> There was a lock release before exiting, so remove the unreasonable unlock.
> 
> Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>   fs/ocfs2/buffer_head_io.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index cdb9b9bdea1f..e62c7e1de4eb 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		if (bhs[i] == NULL) {
>   			bhs[i] = sb_getblk(sb, block++);
>   			if (bhs[i] == NULL) {
> -				ocfs2_metadata_cache_io_unlock(ci);
>   				status = -ENOMEM;
>   				mlog_errno(status);
>   				/* Don't forget to put previous bh! */

The fix looks good to me. However, I found another issue in this function.

In the for-loop after the 'read_failure' label, the condition
'(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
When this contidion is true, this for-loop will call ocfs2_set_buffer_uptodate(ci, bh),
which then triggers a NULL pointer access error.

If you agree with my analysis, please add the new fix and send a v2 patch.

Thanks,
Heming

Re: [PATCH] ocfs2: remove unreasonable unlock

[Lizhi Xu]

On Tue, 20 Aug 2024 12:04:39 +0800, Heming wrote:
> > There was a lock release before exiting, so remove the unreasonable unlock.
> >
> > Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
> > Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > ---
> >   fs/ocfs2/buffer_head_io.c | 1 -
> >   1 file changed, 1 deletion(-)
> >
> > diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> > index cdb9b9bdea1f..e62c7e1de4eb 100644
> > --- a/fs/ocfs2/buffer_head_io.c
> > +++ b/fs/ocfs2/buffer_head_io.c
> > @@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
> >   		if (bhs[i] == NULL) {
> >   			bhs[i] = sb_getblk(sb, block++);
> >   			if (bhs[i] == NULL) {
> > -				ocfs2_metadata_cache_io_unlock(ci);
> >   				status = -ENOMEM;
> >   				mlog_errno(status);
> >   				/* Don't forget to put previous bh! */
> 
> The fix looks good to me. However, I found another issue in this function.
> 
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer_uptodate(ci, bh),
> which then triggers a NULL pointer access error.
> 
> If you agree with my analysis, please add the new fix and send a v2 patch.
I agree with your analysis conclusion, but this is not the same issue
as the current one, so I will split it into two patches. 
The first patch fixes the unbalanced lock issue, and the second patch will
be used to fix the UAF problem of BH.

BR,
Lizhi

[PATCH V2 1/2] ocfs2: remove unreasonable unlock

[Lizhi Xu]

There was a lock release before exiting, so remove the unreasonable unlock.

Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ocfs2/buffer_head_io.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index cdb9b9bdea1f..e62c7e1de4eb 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		if (bhs[i] == NULL) {
 			bhs[i] = sb_getblk(sb, block++);
 			if (bhs[i] == NULL) {
-				ocfs2_metadata_cache_io_unlock(ci);
 				status = -ENOMEM;
 				mlog_errno(status);
 				/* Don't forget to put previous bh! */
-- 
2.43.0

[PATCH V2 2/2] ocfs2: Fix uaf in ocfs2_read_blocks

[Lizhi Xu]

In the for-loop after the 'read_failure' label, the condition
'(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
When this contidion is true, this for-loop will call ocfs2_set_buffer
_uptodate(ci, bh), which then triggers a NULL pointer access error.

Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ocfs2/buffer_head_io.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index e62c7e1de4eb..b4a76f45253d 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -384,6 +384,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 					goto read_failure;
 			}
 		}
+		if (!bh)
+			continue;
 
 		/* Always set the buffer in the cache, even if it was
 		 * a forced read, or read-ahead which hasn't yet
-- 
2.43.0

Re: [PATCH V2 2/2] ocfs2: Fix uaf in ocfs2_read_blocks

[Heming Zhao]

On 8/20/24 14:55, Lizhi Xu wrote:
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer
> _uptodate(ci, bh), which then triggers a NULL pointer access error.
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>   fs/ocfs2/buffer_head_io.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..b4a76f45253d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -384,6 +384,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   					goto read_failure;
>   			}
>   		}
> +		if (!bh)
> +			continue;

If you like the style of the above two lines, put them after the
line "bh = bhs[i];", or the code below is more concise:
```
                   * completed. */
+ 		if (bh)
   			ocfs2_set_buffer_uptodate(ci, bh);
```

Thanks,
Heming
>   
>   		/* Always set the buffer in the cache, even if it was
>   		 * a forced read, or read-ahead which hasn't yet

[PATCH V3] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Lizhi Xu]

In the for-loop after the 'read_failure' label, the condition
'(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
When this contidion is true, this for-loop will call ocfs2_set_buffer
_uptodate(ci, bh), which then triggers a NULL pointer access error.

Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index e62c7e1de4eb..8f714406528d 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		/* Always set the buffer in the cache, even if it was
 		 * a forced read, or read-ahead which hasn't yet
 		 * completed. */
-		ocfs2_set_buffer_uptodate(ci, bh);
+		if (bh)
+			ocfs2_set_buffer_uptodate(ci, bh);
 	}
 	ocfs2_metadata_cache_io_unlock(ci);
 
-- 
2.43.0

Re: [PATCH V3] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Heming Zhao]

Could you resend v3 with both patches?
And add my "Reviewed-by" tag to another patch.

btw, you miss change log area from v2.

-Heming

On 8/20/24 15:37, Lizhi Xu wrote:
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer
> _uptodate(ci, bh), which then triggers a NULL pointer access error.
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>   fs/ocfs2/buffer_head_io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		/* Always set the buffer in the cache, even if it was
>   		 * a forced read, or read-ahead which hasn't yet
>   		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>   	}
>   	ocfs2_metadata_cache_io_unlock(ci);
>   

[PATCH V3 1/2] ocfs2: remove unreasonable unlock

[Lizhi Xu]

There was a lock release before exiting, so remove the unreasonable unlock.

Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
---
 fs/ocfs2/buffer_head_io.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index cdb9b9bdea1f..e62c7e1de4eb 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		if (bhs[i] == NULL) {
 			bhs[i] = sb_getblk(sb, block++);
 			if (bhs[i] == NULL) {
-				ocfs2_metadata_cache_io_unlock(ci);
 				status = -ENOMEM;
 				mlog_errno(status);
 				/* Don't forget to put previous bh! */
-- 
2.43.0

[PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Lizhi Xu]

In the for-loop after the 'read_failure' label, the condition
'(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
When this contidion is true, this for-loop will call ocfs2_set_buffer
_uptodate(ci, bh), which then triggers a NULL pointer access error.

Changes from V2:
* Make the code more concise

Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
---
 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index e62c7e1de4eb..8f714406528d 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		/* Always set the buffer in the cache, even if it was
 		 * a forced read, or read-ahead which hasn't yet
 		 * completed. */
-		ocfs2_set_buffer_uptodate(ci, bh);
+		if (bh)
+			ocfs2_set_buffer_uptodate(ci, bh);
 	}
 	ocfs2_metadata_cache_io_unlock(ci);
 
-- 
2.43.0

Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Heming Zhao]

On 8/20/24 17:45, Lizhi Xu wrote:
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer
> _uptodate(ci, bh), which then triggers a NULL pointer access error.
> 
> Changes from V2:
> * Make the code more concise
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>

I didn't give you my "Reviewed-by" tag for this patch, and you
can add my tag only after I send it to you.
(take easy, you can get my "Reviewed-by" tag now.)
Please remember this rule for next time.

Another issue with this mail is that the change log should be
placed before the file list, not in the commit message section.

ref: Documentation/process/submitting-patches.rst

Thanks,
Heming

> ---
>   fs/ocfs2/buffer_head_io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		/* Always set the buffer in the cache, even if it was
>   		 * a forced read, or read-ahead which hasn't yet
>   		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>   	}
>   	ocfs2_metadata_cache_io_unlock(ci);
>   

Re: [PATCH] ocfs2: remove unreasonable unlock

[Lizhi Xu]

On Tue, 20 Aug 2024 19:32:03 +0800, Heming wrote:
> > In the for-loop after the 'read_failure' label, the condition
> > '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> > When this contidion is true, this for-loop will call ocfs2_set_buffer
> > _uptodate(ci, bh), which then triggers a NULL pointer access error.
> >
> > Changes from V2:
> > * Make the code more concise
> >
> > Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> > Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> > Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> 
> I didn't give you my "Reviewed-by" tag for this patch, and you
> can add my tag only after I send it to you.
> (take easy, you can get my "Reviewed-by" tag now.)
> Please remember this rule for next time.
Got it.
> 
> Another issue with this mail is that the change log should be
> placed before the file list, not in the commit message section.
Thanks. Its like following:
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V2 -> V3: Make the code more concise

 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

BR,
Lizhi

Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Joseph Qi]

On 8/20/24 5:45 PM, Lizhi Xu wrote:
> In the for-loop after the 'read_failure' label, the condition
> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
> When this contidion is true, this for-loop will call ocfs2_set_buffer
> _uptodate(ci, bh), which then triggers a NULL pointer access error.
> 

Or it may simplified as the following:

When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
bh is NULL.

> Changes from V2:
> * Make the code more concise
> 

This is not the right place for changelog.

Thanks,
Joseph

> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/buffer_head_io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>  		/* Always set the buffer in the cache, even if it was
>  		 * a forced read, or read-ahead which hasn't yet
>  		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>  	}
>  	ocfs2_metadata_cache_io_unlock(ci);
>  

Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Joseph Qi]

And this is not a UAF case, but NULL pointer dereference.
So I suggest change the subject to:
ocfs2: fix possible NULL pointer dereference in ocfs2_set_buffer_uptodate

On 8/21/24 8:08 AM, Joseph Qi wrote:
> 
> 
> On 8/20/24 5:45 PM, Lizhi Xu wrote:
>> In the for-loop after the 'read_failure' label, the condition
>> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
>> When this contidion is true, this for-loop will call ocfs2_set_buffer
>> _uptodate(ci, bh), which then triggers a NULL pointer access error.
>>
> 
> Or it may simplified as the following:
> 
> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
> bh is NULL.
> 
>> Changes from V2:
>> * Make the code more concise
>>
> 
> This is not the right place for changelog.
> 
> Thanks,
> Joseph
> 
>> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
>> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
>> ---
>>  fs/ocfs2/buffer_head_io.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
>> index e62c7e1de4eb..8f714406528d 100644
>> --- a/fs/ocfs2/buffer_head_io.c
>> +++ b/fs/ocfs2/buffer_head_io.c
>> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>>  		/* Always set the buffer in the cache, even if it was
>>  		 * a forced read, or read-ahead which hasn't yet
>>  		 * completed. */
>> -		ocfs2_set_buffer_uptodate(ci, bh);
>> +		if (bh)
>> +			ocfs2_set_buffer_uptodate(ci, bh);
>>  	}
>>  	ocfs2_metadata_cache_io_unlock(ci);
>>  

Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Heming Zhao]

On 8/21/24 10:34, Joseph Qi wrote:
> And this is not a UAF case, but NULL pointer dereference.
> So I suggest change the subject to:
> ocfs2: fix possible NULL pointer dereference in ocfs2_set_buffer_uptodate

I agree with above too.
I didn't care about the patch subject in previous review jobs, 'UAF' is not suitable.

-Heming

> 
> On 8/21/24 8:08 AM, Joseph Qi wrote:
>>
>>
>> On 8/20/24 5:45 PM, Lizhi Xu wrote:
>>> In the for-loop after the 'read_failure' label, the condition
>>> '(bh == NULL) && flags includes OCFS2_BH_READAHEAD' is missing.
>>> When this contidion is true, this for-loop will call ocfs2_set_buffer
>>> _uptodate(ci, bh), which then triggers a NULL pointer access error.
>>>
>>
>> Or it may simplified as the following:
>>
>> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
>> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
>> bh is NULL.
>>
>>> Changes from V2:
>>> * Make the code more concise
>>>
>>
>> This is not the right place for changelog.
>>
>> Thanks,
>> Joseph
>>
>>> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
>>> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
>>> Reviewed-by: Heming Zhao <heming.zhao@suse.com>
>>> ---
>>>   fs/ocfs2/buffer_head_io.c | 3 ++-
>>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
>>> index e62c7e1de4eb..8f714406528d 100644
>>> --- a/fs/ocfs2/buffer_head_io.c
>>> +++ b/fs/ocfs2/buffer_head_io.c
>>> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>>>   		/* Always set the buffer in the cache, even if it was
>>>   		 * a forced read, or read-ahead which hasn't yet
>>>   		 * completed. */
>>> -		ocfs2_set_buffer_uptodate(ci, bh);
>>> +		if (bh)
>>> +			ocfs2_set_buffer_uptodate(ci, bh);
>>>   	}
>>>   	ocfs2_metadata_cache_io_unlock(ci);
>>>   

Re: [PATCH V3 2/2] ocfs2: Fix uaf in ocfs2_set_buffer_uptodate

[Lizhi Xu]

On Wed, 21 Aug 2024 10:39:39 +0800, Heming Zhao wrote:
> > And this is not a UAF case, but NULL pointer dereference.
> > So I suggest change the subject to:
> > ocfs2: fix possible NULL pointer dereference in ocfs2_set_buffer_uptodate
> 
> I agree with above too.
> I didn't care about the patch subject in previous review jobs, 'UAF' is not suitable.
> 
> -Heming
OK, I will update and send this patch V4 separately.

Lizhi

[PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Lizhi Xu]

When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
bh is NULL.

Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
V3 -> V4: Update comments and subject

 fs/ocfs2/buffer_head_io.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
index e62c7e1de4eb..8f714406528d 100644
--- a/fs/ocfs2/buffer_head_io.c
+++ b/fs/ocfs2/buffer_head_io.c
@@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
 		/* Always set the buffer in the cache, even if it was
 		 * a forced read, or read-ahead which hasn't yet
 		 * completed. */
-		ocfs2_set_buffer_uptodate(ci, bh);
+		if (bh)
+			ocfs2_set_buffer_uptodate(ci, bh);
 	}
 	ocfs2_metadata_cache_io_unlock(ci);
 
-- 
2.43.0

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[heming.zhao]

Hi,

Where is my "Reviewed-by" tag, and where is [patch 1/2]?

On 8/21/24 14:14, Lizhi Xu wrote:
> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
> bh is NULL.
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
> V3 -> V4: Update comments and subject> 
>   fs/ocfs2/buffer_head_io.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		/* Always set the buffer in the cache, even if it was
>   		 * a forced read, or read-ahead which hasn't yet
>   		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>   	}
>   	ocfs2_metadata_cache_io_unlock(ci);
>   

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Lizhi Xu]

On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
add it by yourself.

In my previous email, I explicitly stated that only this patch should
be sent separately, as the first patch has already been reviewed by two
reviewers. If the second patch is updated with the first patch, I
personally think it is unnecessary.

[patch 1/2]: https://lore.kernel.org/all/20240820094512.2228159-1-lizhi.xu@windriver.com/ 

Lizhi

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[heming.zhao]

Hi,

On 8/21/24 14:55, Lizhi Xu wrote:
> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
> add it by yourself.

Good answer!

This patch issue was found by me, and I also pointed out how to fix it, then take the time
to review your code. But in the end, you removed my "Reviewed-by" tag.

> 
> In my previous email, I explicitly stated that only this patch should
> be sent separately, as the first patch has already been reviewed by two
> reviewers. If the second patch is updated with the first patch, I
> personally think it is unnecessary.
> 
> [patch 1/2]: https://lore.kernel.org/all/20240820094512.2228159-1-lizhi.xu@windriver.com/
> 
> Lizhi

It looks like you don't have basis knowledge of how to send patches.

I will never reply to or review any of your mails/patches.

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Joseph Qi]

On 8/21/24 3:37 PM, heming.zhao@suse.com wrote:
> Hi,
> 
> On 8/21/24 14:55, Lizhi Xu wrote:
>> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
>>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
>> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
>> add it by yourself.
> 
> Good answer!
> 
> This patch issue was found by me, and I also pointed out how to fix it, then take the time
> to review your code. But in the end, you removed my "Reviewed-by" tag.
> 

Seems a misunderstanding, take it easy:) 
Lizhi may think since this is a new version, it needs a new round review.

>>
>> In my previous email, I explicitly stated that only this patch should
>> be sent separately, as the first patch has already been reviewed by two
>> reviewers. If the second patch is updated with the first patch, I
>> personally think it is unnecessary.
>>
>> [patch 1/2]: https://lore.kernel.org/all/20240820094512.2228159-1-lizhi.xu@windriver.com/
>>
>> Lizhi
> 
> It looks like you don't have basis knowledge of how to send patches.
> 
> I will never reply to or review any of your mails/patches.

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Lizhi Xu]

On Wed, 21 Aug 2024 15:58:36 +0800, Joseph Qi wrote:
> On 8/21/24 3:37 PM, heming.zhao@suse.com wrote:
> > Hi,
> >
> > On 8/21/24 14:55, Lizhi Xu wrote:
> >> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
> >>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
> >> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
> >> add it by yourself.
> >
> > Good answer!
> >
> > This patch issue was found by me, and I also pointed out how to fix it, then take the time
> > to review your code. But in the end, you removed my "Reviewed-by" tag.
> >
> 
> Seems a misunderstanding, take it easy:)
> Lizhi may think since this is a new version, it needs a new round review.
Yeah, the subject and comments have all been changed.
Thank you for defending me:)

BR,
Lizhi

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Heming Zhao]

On 8/21/24 17:14, Lizhi Xu wrote:
> On Wed, 21 Aug 2024 15:58:36 +0800, Joseph Qi wrote:
>> On 8/21/24 3:37 PM, heming.zhao@suse.com wrote:
>>> Hi,
>>>
>>> On 8/21/24 14:55, Lizhi Xu wrote:
>>>> On Wed, 21 Aug 2024 14:23:08 +0800, Heming Zhao wrote:
>>>>> Where is my "Reviewed-by" tag, and where is [patch 1/2]?
>>>> Sorry about your "Reviewed-by" tag, I remove it, if you don't mind, you can
>>>> add it by yourself.
>>>
>>> Good answer!
>>>
>>> This patch issue was found by me, and I also pointed out how to fix it, then take the time
>>> to review your code. But in the end, you removed my "Reviewed-by" tag.
>>>
>>
>> Seems a misunderstanding, take it easy:)
>> Lizhi may think since this is a new version, it needs a new round review.
> Yeah, the subject and comments have all been changed.
> Thank you for defending me:)

OK.

> 
> BR,
> Lizhi

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Andrew Morton]

OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].

For clarity can we please have a full resend of both patches?

And let's please have a [0/n] cover letter which describes the problems
which are being addressed and which also briefly describes how they
were addressed.

Also, it appears that both of these fixes should be backported into
-stable kernels.  So let's please try to identify when these bugs were
introduced and to add a suitable Fixes: to the individual changelogs.

Thanks.

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Andrew Morton]

On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
> 
> For clarity can we please have a full resend of both patches?
> 
> And let's please have a [0/n] cover letter which describes the problems
> which are being addressed and which also briefly describes how they
> were addressed.
> 
> Also, it appears that both of these fixes should be backported into
> -stable kernels.  So let's please try to identify when these bugs were
> introduced and to add a suitable Fixes: to the individual changelogs.
> 

Again, can we please have a full resend of these two patches with the
above issues addressed?  Particularly the identification of the Fixes:
targets.

Thanks.

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Heming Zhao]

On 9/2/24 08:54, Andrew Morton wrote:
> On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
> 
>> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
>>
>> For clarity can we please have a full resend of both patches?
>>
>> And let's please have a [0/n] cover letter which describes the problems
>> which are being addressed and which also briefly describes how they
>> were addressed.
>>
>> Also, it appears that both of these fixes should be backported into
>> -stable kernels.  So let's please try to identify when these bugs were
>> introduced and to add a suitable Fixes: to the individual changelogs.
>>
> 
> Again, can we please have a full resend of these two patches with the
> above issues addressed?  Particularly the identification of the Fixes:
> targets.
> 
> Thanks.

Hello Andrew & Joseph,

If Lizhi still doesn't respond by this Friday, I will send his latest patch set again.

-Heming

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Joseph Qi]

On 9/2/24 9:03 AM, Heming Zhao wrote:
> On 9/2/24 08:54, Andrew Morton wrote:
>> On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>>
>>> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
>>>
>>> For clarity can we please have a full resend of both patches?
>>>
>>> And let's please have a [0/n] cover letter which describes the problems
>>> which are being addressed and which also briefly describes how they
>>> were addressed.
>>>
>>> Also, it appears that both of these fixes should be backported into
>>> -stable kernels.  So let's please try to identify when these bugs were
>>> introduced and to add a suitable Fixes: to the individual changelogs.
>>>
>>
>> Again, can we please have a full resend of these two patches with the
>> above issues addressed?  Particularly the identification of the Fixes:
>> targets.
>>
>> Thanks.
> 
> Hello Andrew & Joseph,
> 
> If Lizhi still doesn't respond by this Friday, I will send his latest patch set again.
> 
I'll do that, thanks.

Joseph

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Lizhi Xu]

On Mon, 2 Sep 2024 10:20:38 +0800, Joseph Qi wrote:
>On 9/2/24 9:03 AM, Heming Zhao wrote:
>> On 9/2/24 08:54, Andrew Morton wrote:
>>> On Wed, 21 Aug 2024 14:39:11 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:
>>>
>>>> OK I think I found the correct patches - v3 of [1/2] and v4 of [2/2].
>>>>
>>>> For clarity can we please have a full resend of both patches?
>>>>
>>>> And let's please have a [0/n] cover letter which describes the problems
>>>> which are being addressed and which also briefly describes how they
>>>> were addressed.
>>>>
>>>> Also, it appears that both of these fixes should be backported into
>>>> -stable kernels.  So let's please try to identify when these bugs were
>>>> introduced and to add a suitable Fixes: to the individual changelogs.
>>>>
>>>
>>> Again, can we please have a full resend of these two patches with the
>>> above issues addressed?  Particularly the identification of the Fixes:
>>> targets.
>>>
>>> Thanks.
>>
>> Hello Andrew & Joseph,
>>
>> If Lizhi still doesn't respond by this Friday, I will send his latest patch set again.
>>
>I'll do that, thanks.
Sorry, I didn't notice these emails. Thanks for your help.

BR,
Lizhi

Re: [PATCH V4 2/2] ocfs2: Fix possible null-ptr-deref in ocfs2_set_buffer_uptodate

[Joseph Qi]

On 8/21/24 2:14 PM, Lizhi Xu wrote:
> When doing cleanup, if flags without OCFS2_BH_READAHEAD, it may trigger
> NULL pointer dereference in the following ocfs2_set_buffer_uptodate() if
> bh is NULL.
> 
> Reported-and-suggested-by: Heming Zhao <heming.zhao@suse.com>
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
> V3 -> V4: Update comments and subject
> 
>  fs/ocfs2/buffer_head_io.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index e62c7e1de4eb..8f714406528d 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -388,7 +388,8 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>  		/* Always set the buffer in the cache, even if it was
>  		 * a forced read, or read-ahead which hasn't yet
>  		 * completed. */
> -		ocfs2_set_buffer_uptodate(ci, bh);
> +		if (bh)
> +			ocfs2_set_buffer_uptodate(ci, bh);
>  	}
>  	ocfs2_metadata_cache_io_unlock(ci);
>  

Re: [PATCH V3 1/2] ocfs2: remove unreasonable unlock

[Joseph Qi]

On 8/20/24 5:45 PM, Lizhi Xu wrote:
> There was a lock release before exiting, so remove the unreasonable unlock.
> 
> Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>

Reviewed-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> ---
>  fs/ocfs2/buffer_head_io.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index cdb9b9bdea1f..e62c7e1de4eb 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>  		if (bhs[i] == NULL) {
>  			bhs[i] = sb_getblk(sb, block++);
>  			if (bhs[i] == NULL) {
> -				ocfs2_metadata_cache_io_unlock(ci);
>  				status = -ENOMEM;
>  				mlog_errno(status);
>  				/* Don't forget to put previous bh! */

Re: [PATCH V2 1/2] ocfs2: remove unreasonable unlock

[Heming Zhao]

On 8/20/24 14:55, Lizhi Xu wrote:
> There was a lock release before exiting, so remove the unreasonable unlock.
> 
> Reported-and-tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
> Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
> ---
>   fs/ocfs2/buffer_head_io.c | 1 -
>   1 file changed, 1 deletion(-)
> 
> diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c
> index cdb9b9bdea1f..e62c7e1de4eb 100644
> --- a/fs/ocfs2/buffer_head_io.c
> +++ b/fs/ocfs2/buffer_head_io.c
> @@ -235,7 +235,6 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr,
>   		if (bhs[i] == NULL) {
>   			bhs[i] = sb_getblk(sb, block++);
>   			if (bhs[i] == NULL) {
> -				ocfs2_metadata_cache_io_unlock(ci);
>   				status = -ENOMEM;
>   				mlog_errno(status);
>   				/* Don't forget to put previous bh! */

Looks good to me.

Reviewed-by: Heming Zhao <heming.zhao@suse.com>

Re: [syzbot] [ocfs2?] WARNING: bad unlock balance in ocfs2_read_blocks

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com
Tested-by: syzbot+ab134185af9ef88dfed5@syzkaller.appspotmail.com

Tested on:

commit:         c3f2d783 Merge tag 'mm-hotfixes-stable-2024-08-17-19-3..
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=10a60fcb980000
kernel config:  https://syzkaller.appspot.com/x/.config?x=7229118d88b4a71b
dashboard link: https://syzkaller.appspot.com/bug?extid=ab134185af9ef88dfed5
compiler:       Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
patch:          https://syzkaller.appspot.com/x/patch.diff?x=121f77f3980000

Note: testing is done by a robot and is best-effort only.