[PATCH 4.19 5.4 5.10 5.15 6.1 6.6 6.7] nilfs2: fix potential bug in end_buffer_async_write

[PATCH 4.19 5.4 5.10 5.15 6.1 6.6 6.7] nilfs2: fix potential bug in end_buffer_async_write

[Ryusuke Konishi]

commit 5bc09b397cbf1221f8a8aacb1152650c9195b02b upstream.

According to a syzbot report, end_buffer_async_write(), which handles the
completion of block device writes, may detect abnormal condition of the
buffer async_write flag and cause a BUG_ON failure when using nilfs2.

Nilfs2 itself does not use end_buffer_async_write().  But, the async_write
flag is now used as a marker by commit 7f42ec394156 ("nilfs2: fix issue
with race condition of competition between segments for dirty blocks") as
a means of resolving double list insertion of dirty blocks in
nilfs_lookup_dirty_data_buffers() and nilfs_lookup_node_buffers() and the
resulting crash.

This modification is safe as long as it is used for file data and b-tree
node blocks where the page caches are independent.  However, it was
irrelevant and redundant to also introduce async_write for segment summary
and super root blocks that share buffers with the backing device.  This
led to the possibility that the BUG_ON check in end_buffer_async_write
would fail as described above, if independent writebacks of the backing
device occurred in parallel.

The use of async_write for segment summary buffers has already been
removed in a previous change.

Fix this issue by removing the manipulation of the async_write flag for
the remaining super root block buffer.

Link: https://lkml.kernel.org/r/20240203161645.4992-1-konishi.ryusuke@gmail.com
Fixes: 7f42ec394156 ("nilfs2: fix issue with race condition of competition between segments for dirty blocks")
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Reported-by: syzbot+5c04210f7c7f897c1e7f@syzkaller.appspotmail.com
Closes: https://lkml.kernel.org/r/00000000000019a97c05fd42f8c8@google.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---
Please queue this patch to these stable trees instead of the patch
that failed to apply to them.

This patch is tailored to account for page/folio conversion and can
be applied from v4.8 to v6.7.

Also, all the builds and tests I did on each stable tree passed.

Thanks,
Ryusuke Konishi

 fs/nilfs2/segment.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
index 55e31cc903d1..0f21dbcd0bfb 100644
--- a/fs/nilfs2/segment.c
+++ b/fs/nilfs2/segment.c
@@ -1703,7 +1703,6 @@ static void nilfs_segctor_prepare_write(struct nilfs_sc_info *sci)
 
 		list_for_each_entry(bh, &segbuf->sb_payload_buffers,
 				    b_assoc_buffers) {
-			set_buffer_async_write(bh);
 			if (bh == segbuf->sb_super_root) {
 				if (bh->b_page != bd_page) {
 					lock_page(bd_page);
@@ -1714,6 +1713,7 @@ static void nilfs_segctor_prepare_write(struct nilfs_sc_info *sci)
 				}
 				break;
 			}
+			set_buffer_async_write(bh);
 			if (bh->b_page != fs_page) {
 				nilfs_begin_page_io(fs_page);
 				fs_page = bh->b_page;
@@ -1799,7 +1799,6 @@ static void nilfs_abort_logs(struct list_head *logs, int err)
 
 		list_for_each_entry(bh, &segbuf->sb_payload_buffers,
 				    b_assoc_buffers) {
-			clear_buffer_async_write(bh);
 			if (bh == segbuf->sb_super_root) {
 				clear_buffer_uptodate(bh);
 				if (bh->b_page != bd_page) {
@@ -1808,6 +1807,7 @@ static void nilfs_abort_logs(struct list_head *logs, int err)
 				}
 				break;
 			}
+			clear_buffer_async_write(bh);
 			if (bh->b_page != fs_page) {
 				nilfs_end_page_io(fs_page, err);
 				fs_page = bh->b_page;
@@ -1895,8 +1895,9 @@ static void nilfs_segctor_complete_write(struct nilfs_sc_info *sci)
 				 BIT(BH_Delay) | BIT(BH_NILFS_Volatile) |
 				 BIT(BH_NILFS_Redirected));
 
-			set_mask_bits(&bh->b_state, clear_bits, set_bits);
 			if (bh == segbuf->sb_super_root) {
+				set_buffer_uptodate(bh);
+				clear_buffer_dirty(bh);
 				if (bh->b_page != bd_page) {
 					end_page_writeback(bd_page);
 					bd_page = bh->b_page;
@@ -1904,6 +1905,7 @@ static void nilfs_segctor_complete_write(struct nilfs_sc_info *sci)
 				update_sr = true;
 				break;
 			}
+			set_mask_bits(&bh->b_state, clear_bits, set_bits);
 			if (bh->b_page != fs_page) {
 				nilfs_end_page_io(fs_page, 0);
 				fs_page = bh->b_page;
-- 
2.39.3

Re: [PATCH 4.19 5.4 5.10 5.15 6.1 6.6 6.7] nilfs2: fix potential bug in end_buffer_async_write

[Greg Kroah-Hartman]

On Wed, Feb 21, 2024 at 06:29:28AM +0900, Ryusuke Konishi wrote:
> commit 5bc09b397cbf1221f8a8aacb1152650c9195b02b upstream.
> 
> According to a syzbot report, end_buffer_async_write(), which handles the
> completion of block device writes, may detect abnormal condition of the
> buffer async_write flag and cause a BUG_ON failure when using nilfs2.
> 
> Nilfs2 itself does not use end_buffer_async_write().  But, the async_write
> flag is now used as a marker by commit 7f42ec394156 ("nilfs2: fix issue
> with race condition of competition between segments for dirty blocks") as
> a means of resolving double list insertion of dirty blocks in
> nilfs_lookup_dirty_data_buffers() and nilfs_lookup_node_buffers() and the
> resulting crash.
> 
> This modification is safe as long as it is used for file data and b-tree
> node blocks where the page caches are independent.  However, it was
> irrelevant and redundant to also introduce async_write for segment summary
> and super root blocks that share buffers with the backing device.  This
> led to the possibility that the BUG_ON check in end_buffer_async_write
> would fail as described above, if independent writebacks of the backing
> device occurred in parallel.
> 
> The use of async_write for segment summary buffers has already been
> removed in a previous change.
> 
> Fix this issue by removing the manipulation of the async_write flag for
> the remaining super root block buffer.
> 
> Link: https://lkml.kernel.org/r/20240203161645.4992-1-konishi.ryusuke@gmail.com
> Fixes: 7f42ec394156 ("nilfs2: fix issue with race condition of competition between segments for dirty blocks")
> Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
> Reported-by: syzbot+5c04210f7c7f897c1e7f@syzkaller.appspotmail.com
> Closes: https://lkml.kernel.org/r/00000000000019a97c05fd42f8c8@google.com
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Now queued up, thanks.

greg k-h