[PATCH] udf: Mark LVID buffer as uptodate before marking it dirty

[PATCH] udf: Mark LVID buffer as uptodate before marking it dirty

[syzbot]

From: Aleksandr Nogikh <nogikh@google.com>

When an I/O error occurs while writing the Logical Volume Integrity
Descriptor (LVID) buffer to the block device, the block layer's completion
handler (`end_buffer_write_sync()`) clears the `BH_Uptodate` flag on the
buffer. However, the buffer still contains valid LVID data in memory. If
the filesystem is subsequently remounted read-write or synced,
`udf_open_lvid()` or `udf_sync_fs()` will modify the LVID buffer and call
`mark_buffer_dirty()`. This triggers a spurious
`WARN_ON_ONCE(!buffer_uptodate(bh))` warning in `mark_buffer_dirty()`
because the buffer is not marked uptodate, even though its in-memory
contents are valid and are about to be overwritten.

To prevent this spurious warning, unconditionally set the `BH_Uptodate`
flag before calling `mark_buffer_dirty()` in `udf_open_lvid()` and
`udf_sync_fs()`. This acknowledges that the in-memory buffer is valid and
matches the workaround previously applied to `udf_close_lvid()` in commit
853a0c25baf9 ("udf: Mark LVID buffer as uptodate before marking it dirty").
Extending this workaround ensures consistent behavior across all LVID
updates.

Buffer I/O error on dev loop0, logical block 128, lost sync page write
------------[ cut here ]------------
!buffer_uptodate(bh)
WARNING: fs/buffer.c:1087 at mark_buffer_dirty+0x299/0x410 fs/buffer.c:1087
...
Call Trace:
 <TASK>
 udf_open_lvid+0x369/0x5b0 fs/udf/super.c:2078
 udf_reconfigure+0x336/0x540 fs/udf/super.c:679
 reconfigure_super+0x232/0x8f0 fs/super.c:1080
 vfs_cmd_reconfigure fs/fsopen.c:268 [inline]
 vfs_fsconfig_locked+0x171/0x320 fs/fsopen.c:297
 __do_sys_fsconfig fs/fsopen.c:463 [inline]
 __se_sys_fsconfig+0x6b9/0x810 fs/fsopen.c:350
 do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
 </TASK>

Fixes: 853a0c25baf9 ("udf: Mark LVID buffer as uptodate before marking it dirty")
Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview syzbot
Reported-by: syzbot+0306b38d9ed6ef71467d@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=0306b38d9ed6ef71467d
Link: https://syzkaller.appspot.com/ai_job?id=05f8e20f-f080-4c7f-a206-08dbc15cb4a1
Signed-off-by: Aleksandr Nogikh <nogikh@google.com>

---
diff --git a/fs/udf/super.c b/fs/udf/super.c
index cad2e15d6..7fa2e52cf 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -2040,6 +2040,17 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
 	return 0;
 }
 
+static void udf_mark_buffer_dirty(struct buffer_head *bh)
+{
+	/*
+	 * We set buffer uptodate unconditionally here to avoid spurious
+	 * warnings from mark_buffer_dirty() when previous EIO has marked
+	 * the buffer as !uptodate
+	 */
+	set_buffer_uptodate(bh);
+	mark_buffer_dirty(bh);
+}
+
 static void udf_finalize_lvid(struct logicalVolIntegrityDesc *lvid)
 {
 	struct timespec64 ts;
@@ -2075,7 +2086,7 @@ static void udf_open_lvid(struct super_block *sb)
 		UDF_SET_FLAG(sb, UDF_FLAG_INCONSISTENT);
 
 	udf_finalize_lvid(lvid);
-	mark_buffer_dirty(bh);
+	udf_mark_buffer_dirty(bh);
 	sbi->s_lvid_dirty = 0;
 	mutex_unlock(&sbi->s_alloc_mutex);
 	/* Make opening of filesystem visible on the media immediately */
@@ -2108,14 +2119,8 @@ static void udf_close_lvid(struct super_block *sb)
 	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_INCONSISTENT))
 		lvid->integrityType = cpu_to_le32(LVID_INTEGRITY_TYPE_CLOSE);
 
-	/*
-	 * We set buffer uptodate unconditionally here to avoid spurious
-	 * warnings from mark_buffer_dirty() when previous EIO has marked
-	 * the buffer as !uptodate
-	 */
-	set_buffer_uptodate(bh);
 	udf_finalize_lvid(lvid);
-	mark_buffer_dirty(bh);
+	udf_mark_buffer_dirty(bh);
 	sbi->s_lvid_dirty = 0;
 	mutex_unlock(&sbi->s_alloc_mutex);
 	/* Make closing of filesystem visible on the media immediately */
@@ -2397,7 +2402,7 @@ static int udf_sync_fs(struct super_block *sb, int wait)
 		 * Blockdevice will be synced later so we don't have to submit
 		 * the buffer for IO
 		 */
-		mark_buffer_dirty(bh);
+		udf_mark_buffer_dirty(bh);
 		sbi->s_lvid_dirty = 0;
 	}
 	mutex_unlock(&sbi->s_alloc_mutex);


base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
-- 
See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
You can comment on the patch as usual, syzbot will try to address
the comments and send a new version of the patch if necessary.
syzbot engineers can be reached at syzkaller@googlegroups.com.

Re: [PATCH] udf: Mark LVID buffer as uptodate before marking it dirty

[Jan Kara]

On Tue 23-06-26 12:30:52, syzbot wrote:
> From: Aleksandr Nogikh <nogikh@google.com>
> 
> When an I/O error occurs while writing the Logical Volume Integrity
> Descriptor (LVID) buffer to the block device, the block layer's completion
> handler (`end_buffer_write_sync()`) clears the `BH_Uptodate` flag on the
> buffer. However, the buffer still contains valid LVID data in memory. If
> the filesystem is subsequently remounted read-write or synced,
> `udf_open_lvid()` or `udf_sync_fs()` will modify the LVID buffer and call
> `mark_buffer_dirty()`. This triggers a spurious
> `WARN_ON_ONCE(!buffer_uptodate(bh))` warning in `mark_buffer_dirty()`
> because the buffer is not marked uptodate, even though its in-memory
> contents are valid and are about to be overwritten.
> 
> To prevent this spurious warning, unconditionally set the `BH_Uptodate`
> flag before calling `mark_buffer_dirty()` in `udf_open_lvid()` and
> `udf_sync_fs()`. This acknowledges that the in-memory buffer is valid and
> matches the workaround previously applied to `udf_close_lvid()` in commit
> 853a0c25baf9 ("udf: Mark LVID buffer as uptodate before marking it dirty").
> Extending this workaround ensures consistent behavior across all LVID
> updates.
> 
> Buffer I/O error on dev loop0, logical block 128, lost sync page write
> ------------[ cut here ]------------
> !buffer_uptodate(bh)
> WARNING: fs/buffer.c:1087 at mark_buffer_dirty+0x299/0x410 fs/buffer.c:1087
> ...
> Call Trace:
>  <TASK>
>  udf_open_lvid+0x369/0x5b0 fs/udf/super.c:2078
>  udf_reconfigure+0x336/0x540 fs/udf/super.c:679
>  reconfigure_super+0x232/0x8f0 fs/super.c:1080
>  vfs_cmd_reconfigure fs/fsopen.c:268 [inline]
>  vfs_fsconfig_locked+0x171/0x320 fs/fsopen.c:297
>  __do_sys_fsconfig fs/fsopen.c:463 [inline]
>  __se_sys_fsconfig+0x6b9/0x810 fs/fsopen.c:350
>  do_syscall_64+0x174/0x580 arch/x86/entry/syscall_64.c:94
>  </TASK>
> 
> Fixes: 853a0c25baf9 ("udf: Mark LVID buffer as uptodate before marking it dirty")
> Assisted-by: Gemini:gemini-3.1-pro-preview Gemini:gemini-3-flash-preview syzbot
> Reported-by: syzbot+0306b38d9ed6ef71467d@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=0306b38d9ed6ef71467d
> Link: https://syzkaller.appspot.com/ai_job?id=05f8e20f-f080-4c7f-a206-08dbc15cb4a1
> Signed-off-by: Aleksandr Nogikh <nogikh@google.com>

Thanks. I've added the patch to my tree.

								Honza

> 
> ---
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index cad2e15d6..7fa2e52cf 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -2040,6 +2040,17 @@ static int udf_load_vrs(struct super_block *sb, struct udf_options *uopt,
>  	return 0;
>  }
>  
> +static void udf_mark_buffer_dirty(struct buffer_head *bh)
> +{
> +	/*
> +	 * We set buffer uptodate unconditionally here to avoid spurious
> +	 * warnings from mark_buffer_dirty() when previous EIO has marked
> +	 * the buffer as !uptodate
> +	 */
> +	set_buffer_uptodate(bh);
> +	mark_buffer_dirty(bh);
> +}
> +
>  static void udf_finalize_lvid(struct logicalVolIntegrityDesc *lvid)
>  {
>  	struct timespec64 ts;
> @@ -2075,7 +2086,7 @@ static void udf_open_lvid(struct super_block *sb)
>  		UDF_SET_FLAG(sb, UDF_FLAG_INCONSISTENT);
>  
>  	udf_finalize_lvid(lvid);
> -	mark_buffer_dirty(bh);
> +	udf_mark_buffer_dirty(bh);
>  	sbi->s_lvid_dirty = 0;
>  	mutex_unlock(&sbi->s_alloc_mutex);
>  	/* Make opening of filesystem visible on the media immediately */
> @@ -2108,14 +2119,8 @@ static void udf_close_lvid(struct super_block *sb)
>  	if (!UDF_QUERY_FLAG(sb, UDF_FLAG_INCONSISTENT))
>  		lvid->integrityType = cpu_to_le32(LVID_INTEGRITY_TYPE_CLOSE);
>  
> -	/*
> -	 * We set buffer uptodate unconditionally here to avoid spurious
> -	 * warnings from mark_buffer_dirty() when previous EIO has marked
> -	 * the buffer as !uptodate
> -	 */
> -	set_buffer_uptodate(bh);
>  	udf_finalize_lvid(lvid);
> -	mark_buffer_dirty(bh);
> +	udf_mark_buffer_dirty(bh);
>  	sbi->s_lvid_dirty = 0;
>  	mutex_unlock(&sbi->s_alloc_mutex);
>  	/* Make closing of filesystem visible on the media immediately */
> @@ -2397,7 +2402,7 @@ static int udf_sync_fs(struct super_block *sb, int wait)
>  		 * Blockdevice will be synced later so we don't have to submit
>  		 * the buffer for IO
>  		 */
> -		mark_buffer_dirty(bh);
> +		udf_mark_buffer_dirty(bh);
>  		sbi->s_lvid_dirty = 0;
>  	}
>  	mutex_unlock(&sbi->s_alloc_mutex);
> 
> 
> base-commit: 8cd9520d35a6c38db6567e97dd93b1f11f185dc6
> -- 
> See https://goo.gle/syzbot-ai-patches for information about AI-generated patches.
> You can comment on the patch as usual, syzbot will try to address
> the comments and send a new version of the patch if necessary.
> syzbot engineers can be reached at syzkaller@googlegroups.com.
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] udf: Mark LVID buffer as uptodate before marking it dirty

[Jan Kara]

On Fri 23-12-11 20:39:17, Namjae Jeon wrote:
> 2011/12/23 Jan Kara <jack@suse.cz>:
> > When we hit EIO while writing LVID, the buffer uptodate bit is cleared.
> > This then results in an anoying warning from mark_buffer_dirty() when we
> > write the buffer again. So just set uptodate flag unconditionally.
> >
> > Signed-off-by: Jan Kara <jack@suse.cz>
> Reviewed-by: Namjae Jeon <linkinjeon@gmail.com>
> > ---
> Hi. Jan.
> looks good to me !!.
  Thanks for review!

									Honza

Re: [PATCH] udf: Mark LVID buffer as uptodate before marking it dirty

[Namjae Jeon]

2011/12/23 Jan Kara <jack@suse.cz>:
> When we hit EIO while writing LVID, the buffer uptodate bit is cleared.
> This then results in an anoying warning from mark_buffer_dirty() when we
> write the buffer again. So just set uptodate flag unconditionally.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
Reviewed-by: Namjae Jeon <linkinjeon@gmail.com>
> ---
Hi. Jan.
looks good to me !!.
Thanks.

[PATCH] udf: Mark LVID buffer as uptodate before marking it dirty

[Jan Kara]

When we hit EIO while writing LVID, the buffer uptodate bit is cleared.
This then results in an anoying warning from mark_buffer_dirty() when we
write the buffer again. So just set uptodate flag unconditionally.

Signed-off-by: Jan Kara <jack@suse.cz>
---
 fs/udf/super.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

  I carry this patch in my tree and will merge it with Linus in the next
merge window if noone objects.

diff --git a/fs/udf/super.c b/fs/udf/super.c
index e185253..87cb24a 100644
--- a/fs/udf/super.c
+++ b/fs/udf/super.c
@@ -1799,6 +1799,12 @@ static void udf_close_lvid(struct super_block *sb)
 				le16_to_cpu(lvid->descTag.descCRCLength)));
 
 	lvid->descTag.tagChecksum = udf_tag_checksum(&lvid->descTag);
+	/*
+	 * We set buffer uptodate unconditionally here to avoid spurious
+	 * warnings from mark_buffer_dirty() when previous EIO has marked
+	 * the buffer as !uptodate
+	 */
+	set_buffer_uptodate(bh);
 	mark_buffer_dirty(bh);
 	sbi->s_lvid_dirty = 0;
 	mutex_unlock(&sbi->s_alloc_mutex);
-- 
1.7.1