[PATCH] block: don't take i_rwsem twice in the direct write fallback

[PATCH] block: don't take i_rwsem twice in the direct write fallback

[Zhenxian Ma]

commit 2fec4fd97cc3 ("block: take i_rwsem for the direct I/O write
fallback") takes i_rwsem around blkdev_buffered_write() in the direct
write fallback.  commit 1b1be9ea4e34 ("block: take the inode lock for
buffered write fallback") fixes the same race by taking i_rwsem inside
blkdev_buffered_write().  With both applied, the fallback takes i_rwsem
shared twice.

Drop the outer locking in the fallback.

Fixes: 1b1be9ea4e34 ("block: take the inode lock for buffered write fallback")
Reported-by: syzbot+bd975ef54294bec73f0e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=bd975ef54294bec73f0e
Cc: Tal Zussman <tz2294@columbia.edu>
Cc: Julian Sun <sunjunchao@bytedance.com>
Assisted-by: LLM
Signed-off-by: Zhenxian Ma <mzx199711@gmail.com>
---
 block/fops.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 0e4c42c8b74f..ddab4940d76b 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -809,13 +809,8 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 				else if (!blkdev_dio_fua(iocb, bdev))
 					need_sync = true;
 			} else {
-				ssize_t ret2;
-
-				inode_lock_shared(bd_inode);
-				ret2 = blkdev_buffered_write(iocb, from);
-				inode_unlock_shared(bd_inode);
 				ret = direct_write_fallback(iocb, from, ret,
-							    ret2);
+						blkdev_buffered_write(iocb, from));
 				need_sync = true;
 			}
 		} else if (ret > 0 && !blkdev_dio_fua(iocb, bdev)) {
-- 
2.39.3

Re: [PATCH] block: don't take i_rwsem twice in the direct write fallback

[Bart Van Assche]

On 10/21/26 2:33 AM, Zhenxian Ma wrote:
> commit 2fec4fd97cc3 ("block: take i_rwsem for the direct I/O write
> fallback") takes i_rwsem around blkdev_buffered_write() in the direct
> write fallback.  commit 1b1be9ea4e34 ("block: take the inode lock for
> buffered write fallback") fixes the same race by taking i_rwsem inside
> blkdev_buffered_write().  With both applied, the fallback takes i_rwsem
> shared twice.
> 
> Drop the outer locking in the fallback.
> 
> Fixes: 1b1be9ea4e34 ("block: take the inode lock for buffered write fallback")
> Reported-by: syzbot+bd975ef54294bec73f0e@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=bd975ef54294bec73f0e
> Cc: Tal Zussman <tz2294@columbia.edu>
> Cc: Julian Sun <sunjunchao@bytedance.com>
> Assisted-by: LLM
> Signed-off-by: Zhenxian Ma <mzx199711@gmail.com>
> ---
>   block/fops.c | 7 +------
>   1 file changed, 1 insertion(+), 6 deletions(-)
> 
> diff --git a/block/fops.c b/block/fops.c
> index 0e4c42c8b74f..ddab4940d76b 100644
> --- a/block/fops.c
> +++ b/block/fops.c
> @@ -809,13 +809,8 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
>   				else if (!blkdev_dio_fua(iocb, bdev))
>   					need_sync = true;
>   			} else {
> -				ssize_t ret2;
> -
> -				inode_lock_shared(bd_inode);
> -				ret2 = blkdev_buffered_write(iocb, from);
> -				inode_unlock_shared(bd_inode);
>   				ret = direct_write_fallback(iocb, from, ret,
> -							    ret2);
> +						blkdev_buffered_write(iocb, from));
>   				need_sync = true;
>   			}
>   		} else if (ret > 0 && !blkdev_dio_fua(iocb, bdev)) {

Please keep the ret2 variable and its assignment. I prefer to see
function calls appear in the code in the same order as they are called.

Additionally, please improve the patch description:
* "takes i_rwsem shared twice" doesn't say why that's a problem. Please
   explain that rw_semaphore read locks aren't recursive.
* Including a short excerpt of the syzbot report about the deadlock
   would help.

Thanks,

Bart.

[PATCH v2] block: fix recursive i_rwsem locking in blkdev_write_iter()

[Zhenxian Ma]

Commit 2fec4fd97cc3 ("block: take i_rwsem for the direct I/O write
fallback") takes i_rwsem around blkdev_buffered_write() in the direct
write fallback to close a race with set_blocksize().  Commit
1b1be9ea4e34 ("block: take the inode lock for buffered write fallback")
fixes the same race by taking i_rwsem inside blkdev_buffered_write()
instead.  With both commits applied, the fallback path takes i_rwsem
recursively:

  blkdev_write_iter()
    inode_lock_shared(bd_inode)
    blkdev_buffered_write()
      inode_lock_shared(inode)

syzbot reports this as:

  WARNING: possible recursive locking detected
  syz.0.40/6005 is trying to acquire lock:
  ffff888024956f68 (&sb->s_type->i_mutex_key#10){++++}-{4:4}, at: blkdev_buffered_write+0xd7/0x160 block/fops.c:738
  but task is already holding lock:
  ffff888024956f68 (&sb->s_type->i_mutex_key#10){++++}-{4:4}, at: blkdev_write_iter+0x6c5/0x810 block/fops.c:815

rw_semaphore read locks are not recursive: once a writer is queued,
new readers block behind it.  If set_blocksize() calls inode_lock()
between the two acquisitions, it waits for the first read lock to be
released, while the second inode_lock_shared() waits behind
set_blocksize(), and the task deadlocks.

Drop the outer locking in the fallback.

Fixes: 1b1be9ea4e34 ("block: take the inode lock for buffered write fallback")
Reported-by: syzbot+bd975ef54294bec73f0e@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=bd975ef54294bec73f0e
Cc: Tal Zussman <tz2294@columbia.edu>
Cc: Julian Sun <sunjunchao@bytedance.com>
Assisted-by: LLM
Signed-off-by: Zhenxian Ma <mzx199711@gmail.com>
---
Changes in v2:
- Keep ret2 so calls appear in the order they are executed (Bart).
- Explain why taking i_rwsem shared twice can deadlock, since
  rw_semaphore read locks are not recursive, and include an excerpt
  of the syzbot report (Bart).
- Link to v1: https://lore.kernel.org/linux-block/20261021093332.17334-1-mzx199711@gmail.com/

 block/fops.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/block/fops.c b/block/fops.c
index 0e4c42c8b74f..373667c6b65c 100644
--- a/block/fops.c
+++ b/block/fops.c
@@ -811,9 +811,7 @@ static ssize_t blkdev_write_iter(struct kiocb *iocb, struct iov_iter *from)
 			} else {
 				ssize_t ret2;
 
-				inode_lock_shared(bd_inode);
 				ret2 = blkdev_buffered_write(iocb, from);
-				inode_unlock_shared(bd_inode);
 				ret = direct_write_fallback(iocb, from, ret,
 							    ret2);
 				need_sync = true;
-- 
2.39.3

Re: [PATCH v2] block: fix recursive i_rwsem locking in blkdev_write_iter()

[Bart Van Assche]

On 10/21/26 11:25 PM, Zhenxian Ma wrote:
> Drop the outer locking in the fallback.

Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v2] block: fix recursive i_rwsem locking in blkdev_write_iter()

[Zhenxian Ma]

  Hi Shin'ichiro,

  As you mentioned in another series, this is a known issue with this
  patch and has already been fixed by:
   https://lore.kernel.org/linux-block/20261022062513.77372-1-mzx199711@gmail.com/

  Thanks,
  Ma

On Fri, Oct 23, 2026 at 12:19 AM Bart Van Assche <bvanassche@acm.org> wrote:
>
> On 10/21/26 11:25 PM, Zhenxian Ma wrote:
> > Drop the outer locking in the fallback.
>
> Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v2] block: fix recursive i_rwsem locking in blkdev_write_iter()

[Shin'ichiro Kawasaki]

On Oct 27, 2026 / 14:14, Zhenxian Ma wrote:
>   Hi Shin'ichiro,
> 
>   As you mentioned in another series, this is a known issue with this
>   patch and has already been fixed by:
>    https://lore.kernel.org/linux-block/20261022062513.77372-1-mzx199711@gmail.com/

Hi Ma, thanks. I confirmed that the fix works.

Re: [PATCH v2] block: fix recursive i_rwsem locking in blkdev_write_iter()

[Jens Axboe]

On Thu, 22 Oct 2026 14:25:13 +0800, Zhenxian Ma wrote:
> Commit 2fec4fd97cc3 ("block: take i_rwsem for the direct I/O write
> fallback") takes i_rwsem around blkdev_buffered_write() in the direct
> write fallback to close a race with set_blocksize().  Commit
> 1b1be9ea4e34 ("block: take the inode lock for buffered write fallback")
> fixes the same race by taking i_rwsem inside blkdev_buffered_write()
> instead.  With both commits applied, the fallback path takes i_rwsem
> recursively:
> 
> [...]

Applied, thanks!

[1/1] block: fix recursive i_rwsem locking in blkdev_write_iter()
      commit: 21b88cc8bafceee6c6c82503e3b2cc6f1d0c78bf

Best regards,
-- 
Jens Axboe