Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux
1
fork

Configure Feed

Select the types of activity you want to include in your feed.

writeback: Fix use after free in inode_switch_wbs_work_fn()

inode_switch_wbs_work_fn() has a loop like:

wb_get(new_wb);
while (1) {
list = llist_del_all(&new_wb->switch_wbs_ctxs);
/* Nothing to do? */
if (!list)
break;
... process the items ...
}

Now adding of items to the list looks like:

wb_queue_isw()
if (llist_add(&isw->list, &wb->switch_wbs_ctxs))
queue_work(isw_wq, &wb->switch_work);

Because inode_switch_wbs_work_fn() loops when processing isw items, it
can happen that wb->switch_work is pending while wb->switch_wbs_ctxs is
empty. This is a problem because in that case wb can get freed (no isw
items -> no wb reference) while the work is still pending causing
use-after-free issues.

We cannot just fix this by cancelling work when freeing wb because that
could still trigger problematic 0 -> 1 transitions on wb refcount due to
wb_get() in inode_switch_wbs_work_fn(). It could be all handled with
more careful code but that seems unnecessarily complex so let's avoid
that until it is proven that the looping actually brings practical
benefit. Just remove the loop from inode_switch_wbs_work_fn() instead.
That way when wb_queue_isw() queues work, we are guaranteed we have
added the first item to wb->switch_wbs_ctxs and nobody is going to
remove it (and drop the wb reference it holds) until the queued work
runs.

Fixes: e1b849cfa6b6 ("writeback: Avoid contention on wb->list_lock when switching inodes")
CC: stable@vger.kernel.org
Signed-off-by: Jan Kara <jack@suse.cz>
Link: https://patch.msgid.link/20260413093618.17244-2-jack@suse.cz
Acked-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Christian Brauner <brauner@kernel.org>

authored by

Jan Kara and committed by
Christian Brauner
6689f01d c03ce417

+19 -17
+19 -17
fs/fs-writeback.c
··· 570 570 struct inode_switch_wbs_context *isw, *next_isw; 571 571 struct llist_node *list; 572 572 573 + list = llist_del_all(&new_wb->switch_wbs_ctxs); 573 574 /* 574 - * Grab out reference to wb so that it cannot get freed under us 575 + * Nothing to do? That would be a problem as references held by isw 576 + * items protect wb from freeing... 577 + */ 578 + if (WARN_ON_ONCE(!list)) 579 + return; 580 + 581 + /* 582 + * Grab our reference to wb so that it cannot get freed under us 575 583 * after we process all the isw items. 576 584 */ 577 585 wb_get(new_wb); 578 - while (1) { 579 - list = llist_del_all(&new_wb->switch_wbs_ctxs); 580 - /* Nothing to do? */ 581 - if (!list) 582 - break; 583 - /* 584 - * In addition to synchronizing among switchers, I_WB_SWITCH 585 - * tells the RCU protected stat update paths to grab the i_page 586 - * lock so that stat transfer can synchronize against them. 587 - * Let's continue after I_WB_SWITCH is guaranteed to be 588 - * visible. 589 - */ 590 - synchronize_rcu(); 586 + /* 587 + * In addition to synchronizing among switchers, I_WB_SWITCH 588 + * tells the RCU protected stat update paths to grab the i_page 589 + * lock so that stat transfer can synchronize against them. 590 + * Let's continue after I_WB_SWITCH is guaranteed to be 591 + * visible. 592 + */ 593 + synchronize_rcu(); 591 594 592 - llist_for_each_entry_safe(isw, next_isw, list, list) 593 - process_inode_switch_wbs(new_wb, isw); 594 - } 595 + llist_for_each_entry_safe(isw, next_isw, list, list) 596 + process_inode_switch_wbs(new_wb, isw); 595 597 wb_put(new_wb); 596 598 } 597 599