fs: Fix jfs_fsync() Sleeping in Invalid Context

Commit Message

Purva Yeshi March 25, 2025, 5:33 p.m. UTC

Bug detected by Syzbot:
BUG: sleeping function called from invalid context in jfs_fsync

generic_write_sync() is called within dio_complete(), which can be
triggered by dio_bio_end_aio(), a bio completion handler running in
SoftIRQ context. Since fsync() can sleep, executing it in SoftIRQ
causes an invalid context bug.

Fix this by deferring generic_write_sync() when dio_complete() is
triggered from dio_bio_end_aio(). Modify the completion path to
ensure fsync() is not executed in an atomic context, maintaining
proper synchronization and preventing unexpected failures.

Reported-by: [e-mail]
Closes: https://syzkaller.appspot.com/bug?extid=219127d0a3bce650e1b6
Tested-by: [e-mail]
Fixes: 5955102c9984 ("wrappers for ->i_mutex access")
Signed-off-by: Purva Yeshi <[e-mail]>
---
V1 - https://lore.kernel.org/all/[e-mail]/
V2 - Fix invalid jfs_fsync() invocation by deferring generic_write_sync()
in dio_complete().

 fs/direct-io.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

Comments

Matthew Wilcox (Oracle) March 25, 2025, 6:16 p.m. UTC | #1

On Tue, Mar 25, 2025 at 11:03:36PM +0530, Purva Yeshi wrote:
> +++ b/fs/direct-io.c
> @@ -356,13 +356,9 @@ static void dio_bio_end_aio(struct bio *bio)
>  			defer_completion = dio->defer_completion ||
>  					   (dio_op == REQ_OP_WRITE &&
>  					    dio->inode->i_mapping->nrpages);
> -		if (defer_completion) {
> -			INIT_WORK(&dio->complete_work, dio_aio_complete_work);
> -			queue_work(dio->inode->i_sb->s_dio_done_wq,
> -				   &dio->complete_work);
> -		} else {
> -			dio_complete(dio, 0, DIO_COMPLETE_ASYNC);
> -		}
> +
> +		INIT_WORK(&dio->complete_work, dio_aio_complete_work);
> +		queue_work(dio->inode->i_sb->s_dio_done_wq, &dio->complete_work);

This patch is definitely wrong.  If it were the right thing to do, then
since defer_completion is now un-read, we should stop calculating it.

I'm not sure what the right solution is; should we simply do:

                        defer_completion = dio->defer_completion ||
+					   in_atomic() ||
                                           (dio_op == REQ_OP_WRITE &&
                                            dio->inode->i_mapping->nrpages);

I'm kind of surprised this problem hasn't cropped up before now ...

Patch

diff --git a/fs/direct-io.c b/fs/direct-io.c
index 03d381377ae1..2ae832e7c57b 100644
--- a/fs/direct-io.c
+++ b/fs/direct-io.c
@@ -356,13 +356,9 @@  static void dio_bio_end_aio(struct bio *bio)
 			defer_completion = dio->defer_completion ||
 					   (dio_op == REQ_OP_WRITE &&
 					    dio->inode->i_mapping->nrpages);
-		if (defer_completion) {
-			INIT_WORK(&dio->complete_work, dio_aio_complete_work);
-			queue_work(dio->inode->i_sb->s_dio_done_wq,
-				   &dio->complete_work);
-		} else {
-			dio_complete(dio, 0, DIO_COMPLETE_ASYNC);
-		}
+
+		INIT_WORK(&dio->complete_work, dio_aio_complete_work);
+		queue_work(dio->inode->i_sb->s_dio_done_wq, &dio->complete_work);
 	}
 }