[PATCH] xfs: fix nofs context corruption in xfs_btree_split_worker

Yun Zhou posted 1 patch 4 days, 21 hours ago
fs/xfs/libxfs/xfs_btree.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] xfs: fix nofs context corruption in xfs_btree_split_worker
Posted by Yun Zhou 4 days, 21 hours ago
xfs_btree_split_worker() calls xfs_trans_set_context() on the shared
transaction, overwriting tp->t_pflags saved by the submitting thread.
This can cause the submitting thread's NOFS protection to be cleared
prematurely when the transaction is freed, risking deadlocks from
allocations recursing into fs_reclaim.

    Thread A (NOFS set)             Worker
    -------------------             ------
    xfs_trans_alloc()
      xfs_trans_set_context(tp)
        tp->t_pflags = 0  (*)
    ...
    xfs_btree_split()
      queue_work(split_worker)
                                    xfs_trans_set_context(tp)
                                      tp->t_pflags = NOFS  (clobbers!)
                                    __xfs_btree_split()
                                    xfs_trans_clear_context(tp)
      wait_for_completion()
    ...
    xfs_trans_free(tp)
      memalloc_nofs_restore(NOFS)
        -> clears Thread A's NOFS!

    (*) returns 0 because NOFS was already set by outer scope

Fix by using a local memalloc_nofs_save()/restore() in the worker
instead of touching the shared transaction state.

Reported-by: sashiko <sashiko@sashiko.dev>
Fixes: 756b1c343333 ("xfs: use current->journal_info for detecting transaction recursion")
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 fs/xfs/libxfs/xfs_btree.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 60ef7f08b1d3..1e4c43b7fa3d 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -3010,6 +3010,7 @@ xfs_btree_split_worker(
 						struct xfs_btree_split_args, work);
 	unsigned long		pflags;
 	unsigned long		new_pflags = 0;
+	unsigned int		nofs_flags;
 
 	/*
 	 * we are in a transaction context here, but may also be doing work
@@ -3021,12 +3022,12 @@ xfs_btree_split_worker(
 		new_pflags |= PF_MEMALLOC | PF_KSWAPD;
 
 	current_set_flags_nested(&pflags, new_pflags);
-	xfs_trans_set_context(args->cur->bc_tp);
+	nofs_flags = memalloc_nofs_save();
 
 	args->result = __xfs_btree_split(args->cur, args->level, args->ptrp,
 					 args->key, args->curp, args->stat);
 
-	xfs_trans_clear_context(args->cur->bc_tp);
+	memalloc_nofs_restore(nofs_flags);
 	current_restore_flags_nested(&pflags, new_pflags);
 
 	/*
-- 
2.43.0
Re: [PATCH] xfs: fix nofs context corruption in xfs_btree_split_worker
Posted by Christoph Hellwig 13 hours ago
Looks good for now:

Reviewed-by: Christoph Hellwig <hch@lst.de>
Re: [PATCH] xfs: fix nofs context corruption in xfs_btree_split_worker
Posted by Christoph Hellwig 4 days, 18 hours ago
On Mon, Jul 20, 2026 at 01:05:22PM +0800, Yun Zhou wrote:
>  	current_set_flags_nested(&pflags, new_pflags);
> -	xfs_trans_set_context(args->cur->bc_tp);
> +	nofs_flags = memalloc_nofs_save();

Note that the above is the only user of current_set_flags_nested.

>  	args->result = __xfs_btree_split(args->cur, args->level, args->ptrp,
>  					 args->key, args->curp, args->stat);
>  
> -	xfs_trans_clear_context(args->cur->bc_tp);
> +	memalloc_nofs_restore(nofs_flags);
>  	current_restore_flags_nested(&pflags, new_pflags);

and this is the only caller of current_restore_flags_nested.  Both
of which modify the task flags just like memalloc_nofs_save.

I think we'd be much better of just killing all these silly helpers
and do direct current->flags manipulations, which will both clarify
this code and fix the bug it caused.

Similarly xfs_trans_set_context / xfs_trans_set_context need to go
away as they were a part of this problem.  And to make this coherent,
it should be combined with your other flags series.
Re: [PATCH] xfs: fix nofs context corruption in xfs_btree_split_worker
Posted by Zhou, Yun 18 hours ago
Hi Christoph,

On 7/20/2026 4:35 PM, Christoph Hellwig wrote:
> On Mon, Jul 20, 2026 at 01:05:22PM +0800, Yun Zhou wrote:
>>        current_set_flags_nested(&pflags, new_pflags);
>> -     xfs_trans_set_context(args->cur->bc_tp);
>> +     nofs_flags = memalloc_nofs_save();
> 
> Note that the above is the only user of current_set_flags_nested.
> 
>>        args->result = __xfs_btree_split(args->cur, args->level, args->ptrp,
>>                                         args->key, args->curp, args->stat);
>>
>> -     xfs_trans_clear_context(args->cur->bc_tp);
>> +     memalloc_nofs_restore(nofs_flags);
>>        current_restore_flags_nested(&pflags, new_pflags);
> 
> and this is the only caller of current_restore_flags_nested.  Both
> of which modify the task flags just like memalloc_nofs_save.
> 
> I think we'd be much better of just killing all these silly helpers
> and do direct current->flags manipulations, which will both clarify
> this code and fix the bug it caused.
> 
> Similarly xfs_trans_set_context / xfs_trans_set_context need to go
> away as they were a part of this problem.  And to make this coherent,
> it should be combined with your other flags series.
> 

Thanks a lot.
I'd like to understand the scope you have in mind. Should the removal of 
xfs_trans_set/clear_context and current_set/restore_flags_nested be part 
of my nofs series, or is that something you'd prefer to handle separately?

BR,
Yun
Re: [PATCH] xfs: fix nofs context corruption in xfs_btree_split_worker
Posted by Christoph Hellwig 18 hours ago
On Fri, Jul 24, 2026 at 03:54:10PM +0800, Zhou, Yun wrote:
> Thanks a lot.
> I'd like to understand the scope you have in mind. Should the removal of
> xfs_trans_set/clear_context and current_set/restore_flags_nested be part of
> my nofs series, or is that something you'd prefer to handle separately?

I don't care too strongly.  Maybe we should a minimal fix for the bug
in ASAP, and then we just need to agree who cleans up after that.
Happy to do that myself if you don't have the time or aren't interested.
Re: [PATCH] xfs: fix nofs context corruption in xfs_btree_split_worker
Posted by Zhou, Yun 18 hours ago
On 7/24/2026 4:06 PM, Christoph Hellwig wrote:
> On Fri, Jul 24, 2026 at 03:54:10PM +0800, Zhou, Yun wrote:
>> Thanks a lot.
>> I'd like to understand the scope you have in mind. Should the removal of
>> xfs_trans_set/clear_context and current_set/restore_flags_nested be part of
>> my nofs series, or is that something you'd prefer to handle separately?
> 
> I don't care too strongly.  Maybe we should a minimal fix for the bug
> in ASAP, and then we just need to agree who cleans up after that.
> Happy to do that myself if you don't have the time or aren't interested.

Thanks. The patch I sent is already a minimal fix - it just replaces
xfs_trans_set/clear_context with memalloc_nofs_save/restore in the
worker. Could you take another look?

BR,
Yun