linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work()
@ 2025-07-08 11:15 libaokun
  2025-07-09 13:59 ` Jan Kara
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: libaokun @ 2025-07-08 11:15 UTC (permalink / raw)
  To: linux-ext4
  Cc: tytso, adilger.kernel, jack, linux-kernel, yi.zhang, yangerkun,
	libaokun1, libaokun

From: Baokun Li <libaokun1@huawei.com>

In ext4_io_end_defer_completion(), check if io_end->list_vec is empty to
avoid adding an io_end that requires no conversion to the
i_rsv_conversion_list, which in turn prevents starting an unnecessary
worker. An ext4_emergency_state() check is also added to avoid attempting
to abort the journal in an emergency state.

Additionally, ext4_put_io_end_defer() is refactored to call
ext4_io_end_defer_completion() directly instead of being open-coded.
This also prevents starting an unnecessary worker when EXT4_IO_END_FAILED
is set but data_err=abort is not enabled.

This ensures that the check in ext4_put_io_end_defer() is consistent with
the check in ext4_end_bio(). Otherwise, we might add an io_end to the
i_rsv_conversion_list and then call ext4_finish_bio(), after which the
inode could be freed before ext4_end_io_rsv_work() is called, triggering
a use-after-free issue.

Fixes: ce51afb8cc5e ("ext4: abort journal on data writeback failure if in data_err=abort mode")
Signed-off-by: Baokun Li <libaokun1@huawei.com>
---
 fs/ext4/page-io.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
index 179e54f3a3b6..3d8b0f6d2dea 100644
--- a/fs/ext4/page-io.c
+++ b/fs/ext4/page-io.c
@@ -236,10 +236,12 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
 
 static bool ext4_io_end_defer_completion(ext4_io_end_t *io_end)
 {
-	if (io_end->flag & EXT4_IO_END_UNWRITTEN)
+	if (io_end->flag & EXT4_IO_END_UNWRITTEN &&
+	    !list_empty(&io_end->list_vec))
 		return true;
 	if (test_opt(io_end->inode->i_sb, DATA_ERR_ABORT) &&
-	    io_end->flag & EXT4_IO_END_FAILED)
+	    io_end->flag & EXT4_IO_END_FAILED &&
+	    !ext4_emergency_state(io_end->inode->i_sb))
 		return true;
 	return false;
 }
@@ -256,6 +258,7 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end)
 	WARN_ON(!(io_end->flag & EXT4_IO_END_DEFER_COMPLETION));
 	WARN_ON(io_end->flag & EXT4_IO_END_UNWRITTEN &&
 		!io_end->handle && sbi->s_journal);
+	WARN_ON(!io_end->bio);
 
 	spin_lock_irqsave(&ei->i_completed_io_lock, flags);
 	wq = sbi->rsv_conversion_wq;
@@ -318,12 +321,9 @@ ext4_io_end_t *ext4_init_io_end(struct inode *inode, gfp_t flags)
 void ext4_put_io_end_defer(ext4_io_end_t *io_end)
 {
 	if (refcount_dec_and_test(&io_end->count)) {
-		if (io_end->flag & EXT4_IO_END_FAILED ||
-		    (io_end->flag & EXT4_IO_END_UNWRITTEN &&
-		     !list_empty(&io_end->list_vec))) {
-			ext4_add_complete_io(io_end);
-			return;
-		}
+		if (ext4_io_end_defer_completion(io_end))
+			return ext4_add_complete_io(io_end);
+
 		ext4_release_io_end(io_end);
 	}
 }
-- 
2.46.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work()
  2025-07-08 11:15 [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work() libaokun
@ 2025-07-09 13:59 ` Jan Kara
  2025-07-10  8:30 ` Zhang Yi
  2025-07-19 21:45 ` Theodore Ts'o
  2 siblings, 0 replies; 4+ messages in thread
From: Jan Kara @ 2025-07-09 13:59 UTC (permalink / raw)
  To: libaokun
  Cc: linux-ext4, tytso, adilger.kernel, jack, linux-kernel, yi.zhang,
	yangerkun, libaokun1

On Tue 08-07-25 19:15:04, libaokun@huaweicloud.com wrote:
> From: Baokun Li <libaokun1@huawei.com>
> 
> In ext4_io_end_defer_completion(), check if io_end->list_vec is empty to
> avoid adding an io_end that requires no conversion to the
> i_rsv_conversion_list, which in turn prevents starting an unnecessary
> worker. An ext4_emergency_state() check is also added to avoid attempting
> to abort the journal in an emergency state.
> 
> Additionally, ext4_put_io_end_defer() is refactored to call
> ext4_io_end_defer_completion() directly instead of being open-coded.
> This also prevents starting an unnecessary worker when EXT4_IO_END_FAILED
> is set but data_err=abort is not enabled.
> 
> This ensures that the check in ext4_put_io_end_defer() is consistent with
> the check in ext4_end_bio(). Otherwise, we might add an io_end to the
> i_rsv_conversion_list and then call ext4_finish_bio(), after which the
> inode could be freed before ext4_end_io_rsv_work() is called, triggering
> a use-after-free issue.
> 
> Fixes: ce51afb8cc5e ("ext4: abort journal on data writeback failure if in data_err=abort mode")
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/page-io.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 179e54f3a3b6..3d8b0f6d2dea 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -236,10 +236,12 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
>  
>  static bool ext4_io_end_defer_completion(ext4_io_end_t *io_end)
>  {
> -	if (io_end->flag & EXT4_IO_END_UNWRITTEN)
> +	if (io_end->flag & EXT4_IO_END_UNWRITTEN &&
> +	    !list_empty(&io_end->list_vec))
>  		return true;
>  	if (test_opt(io_end->inode->i_sb, DATA_ERR_ABORT) &&
> -	    io_end->flag & EXT4_IO_END_FAILED)
> +	    io_end->flag & EXT4_IO_END_FAILED &&
> +	    !ext4_emergency_state(io_end->inode->i_sb))
>  		return true;
>  	return false;
>  }
> @@ -256,6 +258,7 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end)
>  	WARN_ON(!(io_end->flag & EXT4_IO_END_DEFER_COMPLETION));
>  	WARN_ON(io_end->flag & EXT4_IO_END_UNWRITTEN &&
>  		!io_end->handle && sbi->s_journal);
> +	WARN_ON(!io_end->bio);
>  
>  	spin_lock_irqsave(&ei->i_completed_io_lock, flags);
>  	wq = sbi->rsv_conversion_wq;
> @@ -318,12 +321,9 @@ ext4_io_end_t *ext4_init_io_end(struct inode *inode, gfp_t flags)
>  void ext4_put_io_end_defer(ext4_io_end_t *io_end)
>  {
>  	if (refcount_dec_and_test(&io_end->count)) {
> -		if (io_end->flag & EXT4_IO_END_FAILED ||
> -		    (io_end->flag & EXT4_IO_END_UNWRITTEN &&
> -		     !list_empty(&io_end->list_vec))) {
> -			ext4_add_complete_io(io_end);
> -			return;
> -		}
> +		if (ext4_io_end_defer_completion(io_end))
> +			return ext4_add_complete_io(io_end);
> +
>  		ext4_release_io_end(io_end);
>  	}
>  }
> -- 
> 2.46.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work()
  2025-07-08 11:15 [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work() libaokun
  2025-07-09 13:59 ` Jan Kara
@ 2025-07-10  8:30 ` Zhang Yi
  2025-07-19 21:45 ` Theodore Ts'o
  2 siblings, 0 replies; 4+ messages in thread
From: Zhang Yi @ 2025-07-10  8:30 UTC (permalink / raw)
  To: libaokun, linux-ext4
  Cc: tytso, adilger.kernel, jack, linux-kernel, yangerkun, libaokun1

On 2025/7/8 19:15, libaokun@huaweicloud.com wrote:
> From: Baokun Li <libaokun1@huawei.com>
> 
> In ext4_io_end_defer_completion(), check if io_end->list_vec is empty to
> avoid adding an io_end that requires no conversion to the
> i_rsv_conversion_list, which in turn prevents starting an unnecessary
> worker. An ext4_emergency_state() check is also added to avoid attempting
> to abort the journal in an emergency state.
> 
> Additionally, ext4_put_io_end_defer() is refactored to call
> ext4_io_end_defer_completion() directly instead of being open-coded.
> This also prevents starting an unnecessary worker when EXT4_IO_END_FAILED
> is set but data_err=abort is not enabled.
> 
> This ensures that the check in ext4_put_io_end_defer() is consistent with
> the check in ext4_end_bio(). Otherwise, we might add an io_end to the
> i_rsv_conversion_list and then call ext4_finish_bio(), after which the
> inode could be freed before ext4_end_io_rsv_work() is called, triggering
> a use-after-free issue.
> 
> Fixes: ce51afb8cc5e ("ext4: abort journal on data writeback failure if in data_err=abort mode")
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good to me.

Reviewed-by: Zhang Yi <yi.zhang@huawei.com>

> ---
>  fs/ext4/page-io.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ext4/page-io.c b/fs/ext4/page-io.c
> index 179e54f3a3b6..3d8b0f6d2dea 100644
> --- a/fs/ext4/page-io.c
> +++ b/fs/ext4/page-io.c
> @@ -236,10 +236,12 @@ static void dump_completed_IO(struct inode *inode, struct list_head *head)
>  
>  static bool ext4_io_end_defer_completion(ext4_io_end_t *io_end)
>  {
> -	if (io_end->flag & EXT4_IO_END_UNWRITTEN)
> +	if (io_end->flag & EXT4_IO_END_UNWRITTEN &&
> +	    !list_empty(&io_end->list_vec))
>  		return true;
>  	if (test_opt(io_end->inode->i_sb, DATA_ERR_ABORT) &&
> -	    io_end->flag & EXT4_IO_END_FAILED)
> +	    io_end->flag & EXT4_IO_END_FAILED &&
> +	    !ext4_emergency_state(io_end->inode->i_sb))
>  		return true;
>  	return false;
>  }
> @@ -256,6 +258,7 @@ static void ext4_add_complete_io(ext4_io_end_t *io_end)
>  	WARN_ON(!(io_end->flag & EXT4_IO_END_DEFER_COMPLETION));
>  	WARN_ON(io_end->flag & EXT4_IO_END_UNWRITTEN &&
>  		!io_end->handle && sbi->s_journal);
> +	WARN_ON(!io_end->bio);
>  
>  	spin_lock_irqsave(&ei->i_completed_io_lock, flags);
>  	wq = sbi->rsv_conversion_wq;
> @@ -318,12 +321,9 @@ ext4_io_end_t *ext4_init_io_end(struct inode *inode, gfp_t flags)
>  void ext4_put_io_end_defer(ext4_io_end_t *io_end)
>  {
>  	if (refcount_dec_and_test(&io_end->count)) {
> -		if (io_end->flag & EXT4_IO_END_FAILED ||
> -		    (io_end->flag & EXT4_IO_END_UNWRITTEN &&
> -		     !list_empty(&io_end->list_vec))) {
> -			ext4_add_complete_io(io_end);
> -			return;
> -		}
> +		if (ext4_io_end_defer_completion(io_end))
> +			return ext4_add_complete_io(io_end);
> +
>  		ext4_release_io_end(io_end);
>  	}
>  }


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work()
  2025-07-08 11:15 [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work() libaokun
  2025-07-09 13:59 ` Jan Kara
  2025-07-10  8:30 ` Zhang Yi
@ 2025-07-19 21:45 ` Theodore Ts'o
  2 siblings, 0 replies; 4+ messages in thread
From: Theodore Ts'o @ 2025-07-19 21:45 UTC (permalink / raw)
  To: linux-ext4, libaokun
  Cc: Theodore Ts'o, adilger.kernel, jack, linux-kernel, yi.zhang,
	yangerkun, libaokun1


On Tue, 08 Jul 2025 19:15:04 +0800, libaokun@huaweicloud.com wrote:
> In ext4_io_end_defer_completion(), check if io_end->list_vec is empty to
> avoid adding an io_end that requires no conversion to the
> i_rsv_conversion_list, which in turn prevents starting an unnecessary
> worker. An ext4_emergency_state() check is also added to avoid attempting
> to abort the journal in an emergency state.
> 
> Additionally, ext4_put_io_end_defer() is refactored to call
> ext4_io_end_defer_completion() directly instead of being open-coded.
> This also prevents starting an unnecessary worker when EXT4_IO_END_FAILED
> is set but data_err=abort is not enabled.
> 
> [...]

Applied, thanks!

[1/1] ext4: fix inode use after free in ext4_end_io_rsv_work()
      commit: c678bdc998754589cea2e6afab9401d7d8312ac4

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-07-19 21:46 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-08 11:15 [PATCH] ext4: fix inode use after free in ext4_end_io_rsv_work() libaokun
2025-07-09 13:59 ` Jan Kara
2025-07-10  8:30 ` Zhang Yi
2025-07-19 21:45 ` Theodore Ts'o

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).