Skip to content

Commit f87904c

Browse files
khazhykakpm00
authored andcommitted
writeback: avoid use-after-free after removing device
When a disk is removed, bdi_unregister gets called to stop further writeback and wait for associated delayed work to complete. However, wb_inode_writeback_end() may schedule bandwidth estimation dwork after this has completed, which can result in the timer attempting to access the just freed bdi_writeback. Fix this by checking if the bdi_writeback is alive, similar to when scheduling writeback work. Since this requires wb->work_lock, and wb_inode_writeback_end() may get called from interrupt, switch wb->work_lock to an irqsafe lock. Link: https://lkml.kernel.org/r/[email protected] Fixes: 45a2966 ("writeback: fix bandwidth estimate for spiky workload") Signed-off-by: Khazhismel Kumykov <[email protected]> Reviewed-by: Jan Kara <[email protected]> Cc: Michael Stapelberg <[email protected]> Cc: Wu Fengguang <[email protected]> Cc: Alexander Viro <[email protected]> Cc: <[email protected]> Signed-off-by: Andrew Morton <[email protected]>
1 parent 9dfb3b8 commit f87904c

File tree

3 files changed

+16
-12
lines changed

3 files changed

+16
-12
lines changed

fs/fs-writeback.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -134,10 +134,10 @@ static bool inode_io_list_move_locked(struct inode *inode,
134134

135135
static void wb_wakeup(struct bdi_writeback *wb)
136136
{
137-
spin_lock_bh(&wb->work_lock);
137+
spin_lock_irq(&wb->work_lock);
138138
if (test_bit(WB_registered, &wb->state))
139139
mod_delayed_work(bdi_wq, &wb->dwork, 0);
140-
spin_unlock_bh(&wb->work_lock);
140+
spin_unlock_irq(&wb->work_lock);
141141
}
142142

143143
static void finish_writeback_work(struct bdi_writeback *wb,
@@ -164,15 +164,15 @@ static void wb_queue_work(struct bdi_writeback *wb,
164164
if (work->done)
165165
atomic_inc(&work->done->cnt);
166166

167-
spin_lock_bh(&wb->work_lock);
167+
spin_lock_irq(&wb->work_lock);
168168

169169
if (test_bit(WB_registered, &wb->state)) {
170170
list_add_tail(&work->list, &wb->work_list);
171171
mod_delayed_work(bdi_wq, &wb->dwork, 0);
172172
} else
173173
finish_writeback_work(wb, work);
174174

175-
spin_unlock_bh(&wb->work_lock);
175+
spin_unlock_irq(&wb->work_lock);
176176
}
177177

178178
/**
@@ -2082,13 +2082,13 @@ static struct wb_writeback_work *get_next_work_item(struct bdi_writeback *wb)
20822082
{
20832083
struct wb_writeback_work *work = NULL;
20842084

2085-
spin_lock_bh(&wb->work_lock);
2085+
spin_lock_irq(&wb->work_lock);
20862086
if (!list_empty(&wb->work_list)) {
20872087
work = list_entry(wb->work_list.next,
20882088
struct wb_writeback_work, list);
20892089
list_del_init(&work->list);
20902090
}
2091-
spin_unlock_bh(&wb->work_lock);
2091+
spin_unlock_irq(&wb->work_lock);
20922092
return work;
20932093
}
20942094

mm/backing-dev.c

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -260,10 +260,10 @@ void wb_wakeup_delayed(struct bdi_writeback *wb)
260260
unsigned long timeout;
261261

262262
timeout = msecs_to_jiffies(dirty_writeback_interval * 10);
263-
spin_lock_bh(&wb->work_lock);
263+
spin_lock_irq(&wb->work_lock);
264264
if (test_bit(WB_registered, &wb->state))
265265
queue_delayed_work(bdi_wq, &wb->dwork, timeout);
266-
spin_unlock_bh(&wb->work_lock);
266+
spin_unlock_irq(&wb->work_lock);
267267
}
268268

269269
static void wb_update_bandwidth_workfn(struct work_struct *work)
@@ -334,12 +334,12 @@ static void cgwb_remove_from_bdi_list(struct bdi_writeback *wb);
334334
static void wb_shutdown(struct bdi_writeback *wb)
335335
{
336336
/* Make sure nobody queues further work */
337-
spin_lock_bh(&wb->work_lock);
337+
spin_lock_irq(&wb->work_lock);
338338
if (!test_and_clear_bit(WB_registered, &wb->state)) {
339-
spin_unlock_bh(&wb->work_lock);
339+
spin_unlock_irq(&wb->work_lock);
340340
return;
341341
}
342-
spin_unlock_bh(&wb->work_lock);
342+
spin_unlock_irq(&wb->work_lock);
343343

344344
cgwb_remove_from_bdi_list(wb);
345345
/*

mm/page-writeback.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2892,6 +2892,7 @@ static void wb_inode_writeback_start(struct bdi_writeback *wb)
28922892

28932893
static void wb_inode_writeback_end(struct bdi_writeback *wb)
28942894
{
2895+
unsigned long flags;
28952896
atomic_dec(&wb->writeback_inodes);
28962897
/*
28972898
* Make sure estimate of writeback throughput gets updated after
@@ -2900,7 +2901,10 @@ static void wb_inode_writeback_end(struct bdi_writeback *wb)
29002901
* that if multiple inodes end writeback at a similar time, they get
29012902
* batched into one bandwidth update.
29022903
*/
2903-
queue_delayed_work(bdi_wq, &wb->bw_dwork, BANDWIDTH_INTERVAL);
2904+
spin_lock_irqsave(&wb->work_lock, flags);
2905+
if (test_bit(WB_registered, &wb->state))
2906+
queue_delayed_work(bdi_wq, &wb->bw_dwork, BANDWIDTH_INTERVAL);
2907+
spin_unlock_irqrestore(&wb->work_lock, flags);
29042908
}
29052909

29062910
bool __folio_end_writeback(struct folio *folio)

0 commit comments

Comments
 (0)