Commit 59b57717fff8 ("blkcg: delay blkg destruction until after
writeback has finished") delayed call to blkcg_destroy_blkgs() to
cgwb_release_workfn(). However, it is done after a css_put() of blkcg
which may be the final put that causes the blkcg to be freed as RCU
read lock isn't held.
By adding a css_tryget() into blkcg_destroy_blkgs() and warning its
failure, the following stack trace was produced in a test system on
bootup.
[ 34.254240] RIP: 0010:blkcg_destroy_blkgs+0x16a/0x1a0
:
[ 34.339943] Call Trace:
[ 34.342395] <TASK>
[ 34.344510] blkcg_unpin_online+0x38/0x60
[ 34.348523] cgwb_release_workfn+0x6a/0x200
[ 34.352708] process_one_work+0x1e5/0x3b0
[ 34.356742] ? rescuer_thread+0x390/0x390
[ 34.360758] worker_thread+0x50/0x3a0
[ 34.364425] ? rescuer_thread+0x390/0x390
[ 34.368447] kthread+0xd9/0x100
[ 34.371592] ? kthread_complete_and_exit+0x20/0x20
[ 34.376386] ret_from_fork+0x22/0x30
[ 34.379982] </TASK>
This confirms that a potential UAF situation can happen.
Fix that by delaying the css_put() until after the blkcg_unpin_online()
call. Also use css_tryget() in blkcg_destroy_blkgs() and issue a warning
if css_tryget() fails with no RCU read lock held.
The reproducing system can no longer produce a warning with this patch.
All the runnable block/0* tests including block/027 were run successfully
without failure.
Fixes: 59b57717fff8 ("blkcg: delay blkg destruction until after writeback has finished")
Suggested-by: Michal Koutný <mkoutny@xxxxxxxx>
Reported-by: Yi Zhang <yi.zhang@xxxxxxxxxx>
Signed-off-by: Waiman Long <longman@xxxxxxxxxx>
---
block/blk-cgroup.c | 10 +++++++++-
mm/backing-dev.c | 8 ++++++--
2 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
index 57941d2a8ba3..904372bb96f1 100644
--- a/block/blk-cgroup.c
+++ b/block/blk-cgroup.c
@@ -1088,7 +1088,15 @@ static void blkcg_destroy_blkgs(struct blkcg *blkcg)
might_sleep();
- css_get(&blkcg->css);
+ /*
+ * blkcg_destroy_blkgs() shouldn't be called with all the blkcg
+ * references gone and rcu_read_lock not held.
+ */
+ if (!css_tryget(&blkcg->css)) {
+ WARN_ON_ONCE(!rcu_read_lock_held());
+ return;
+ }
+
spin_lock_irq(&blkcg->lock);
while (!hlist_empty(&blkcg->blkg_list)) {
struct blkcg_gq *blkg = hlist_entry(blkcg->blkg_list.first,
diff --git a/mm/backing-dev.c b/mm/backing-dev.c
index c30419a5e119..36f75b072325 100644
--- a/mm/backing-dev.c
+++ b/mm/backing-dev.c
@@ -390,11 +390,15 @@ static void cgwb_release_workfn(struct work_struct *work)
wb_shutdown(wb);
css_put(wb->memcg_css);
- css_put(wb->blkcg_css);
mutex_unlock(&wb->bdi->cgwb_release_mutex);
- /* triggers blkg destruction if no online users left */
+ /*
+ * Triggers blkg destruction if no online users left
+ * The final blkcg css_put() has to be done after blkcg_unpin_online()
+ * to avoid use-after-free.
+ */
blkcg_unpin_online(wb->blkcg_css);
+ css_put(wb->blkcg_css);
fprop_local_destroy_percpu(&wb->memcg_completions);
--
2.31.1
