ext4: fix use-after-free in update_super_work when racing with umount
PATCH v3 1/1 ext4: fix use-after-free in update_super_work when racing with umount
From: Jiayuan Chen <[email protected]>
Commit b98535d09179 ("ext4: fix bug_on in start_this_handle during umount filesystem") moved ext4_unregister_sysfs() before flushing s_sb_upd_work to prevent new error work from being queued via /proc/fs/ext4/xx/mb_groups reads during unmount. However, this introduced a use-after-free because update_super_work calls ext4_notify_error_sysfs() -> sysfs_notify() which accesses the kobject's kernfs_node after it has been freed by kobject_del() in ext4_unregister_sysfs():
update_super_work ext4_put_super
----------------- --------------
ext4_unregister_sysfs(sb)
kobject_del(&sbi->s_kobj)
__kobject_del()
sysfs_remove_dir()
kobj->sd = NULL
sysfs_put(sd)
kernfs_put() // RCU free
ext4_notify_error_sysfs(sbi)
sysfs_notify(&sbi->s_kobj)
kn = kobj->sd // stale pointer
kernfs_get(kn) // UAF on freed kernfs_node
ext4_journal_destroy()
flush_work(&sbi->s_sb_upd_work)Instead of reordering the teardown sequence, fix this by making ext4_notify_error_sysfs() detect that sysfs has already been torn down by checking s_kobj.state_in_sysfs, and skipping the sysfs_notify() call in that case. A dedicated mutex (s_error_notify_mutex) serializes ext4_notify_error_sysfs() against kobject_del() in ext4_unregister_sysfs() to prevent TOCTOU races where the kobject could be deleted between the state_in_sysfs check and the sysfs_notify() call.
---
v2 -> v3: Fix issues reported by AI.
https://sashiko.dev/#/patchset/20260319073651.79209-1-jiayuan.chen%40linux.dev
v1 -> v2: Use state_in_sysfs instead of reordering the teardown sequence.
https://lore.kernel.org/linux-ext4/[email protected]/
--- fs/ext4/ext4.h | 1 + fs/ext4/super.c | 1 + fs/ext4/sysfs.c | 10 +++++++++- 3 files changed, 11 insertions(+), 1 deletion(-)
fs/ext4/ext4.h +1 -0
@@ -1574,6 +1574,7 @@ struct ext4_sb_info {struct proc_dir_entry *s_proc;struct kobject s_kobj;struct completion s_kobj_unregister; +struct mutex s_error_notify_mutex;/* protects sysfs_notify vs kobject_del */ struct super_block *s_sb;struct buffer_head *s_mmp_bh;
fs/ext4/super.c +1 -0
@@ -5397,6 +5397,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)timer_setup (&sbi->s_err_report, print_daily_error_info,0 );spin_lock_init (&sbi->s_error_lock); +mutex_init (&sbi->s_error_notify_mutex);INIT_WORK (&sbi->s_sb_upd_work, update_super_work); err =ext4_group_desc_init (sb, es, logical_sb_block, &first_not_zeroed);
fs/ext4/sysfs.c +9 -1
@@ -597,7 +597,10 @@ static const struct kobj_type ext4_feat_ktype = {void ext4_notify_error_sysfs (struct ext4_sb_info *sbi) { -sysfs_notify (&sbi->s_kobj,NULL ,"errors_count" ); +mutex_lock (&sbi->s_error_notify_mutex); +if (sbi->s_kobj.state_in_sysfs) +sysfs_notify (&sbi->s_kobj,NULL ,"errors_count" ); +mutex_unlock (&sbi->s_error_notify_mutex); }static struct kobject *ext4_root;
@@ -610,8 +613,10 @@ int ext4_register_sysfs(struct super_block *sb)int err;init_completion (&sbi->s_kobj_unregister); +mutex_lock (&sbi->s_error_notify_mutex); err =kobject_init_and_add (&sbi->s_kobj, &ext4_sb_ktype, ext4_root,"%s" , sb->s_id); +mutex_unlock (&sbi->s_error_notify_mutex);if (err) {kobject_put (&sbi->s_kobj);wait_for_completion (&sbi->s_kobj_unregister);
@@ -644,7 +649,10 @@ void ext4_unregister_sysfs(struct super_block *sb)if (sbi->s_proc)remove_proc_subtree (sb->s_id, ext4_proc_root); + +mutex_lock (&sbi->s_error_notify_mutex);kobject_del (&sbi->s_kobj); +mutex_unlock (&sbi->s_error_notify_mutex); }int __init ext4_init_sysfs (void )
Signature · 1 line
2.43.0
Quoted 34 lines · Jiayuan Chen
From: Jiayuan Chen <[email protected]>
Commit
b98535d09179("ext4: fixbug_oninstart_this_handleduring umount filesystem") movedext4_unregister_sysfs()before flushings_sb_upd_workto prevent new error work from being queued via/proc/fs/ext4/xx/mb_groupsreads during unmount. However, this introduced a use-after-free becauseupdate_super_workcallsext4_notify_error_sysfs()->sysfs_notify()which accesses the kobject'skernfs_nodeafter it has been freed bykobject_del()inext4_unregister_sysfs():
update_super_workext4_put_super
----------------- --------------ext4_unregister_sysfs(sb) kobject_del(&sbi->s_kobj) __kobject_del() sysfs_remove_dir() kobj->sd = NULL sysfs_put(sd) kernfs_put() // RCU free ext4_notify_error_sysfs(sbi) sysfs_notify(&sbi->s_kobj) kn = kobj->sd // stale pointer kernfs_get(kn) // UAF on freed kernfs_node ext4_journal_destroy() flush_work(&sbi->s_sb_upd_work)Instead of reordering the teardown sequence, fix this by making
ext4_notify_error_sysfs()detect that sysfs has already been torn down by checking s_kobj.state_in_sysfs, and skipping thesysfs_notify()call in that case. A dedicated mutex (s_error_notify_mutex) serializesext4_notify_error_sysfs()againstkobject_del()inext4_unregister_sysfs()to prevent TOCTOU races where the kobject could be deleted between thestate_in_sysfscheck and thesysfs_notify()call.
Quoted 4 lines
Fixes b98535d09179 ("ext4: fix bug_on in start_this_handle during umount filesystem")Cc Jiayuan Chen <[email protected]>Suggested-by Jan Kara <[email protected]>Signed-off-by Jiayuan Chen <[email protected]>
Looks good to me! Feel free to add:
Honza
Quoted 78 lines
---
v2 -> v3: Fix issues reported by AI.
https://sashiko.dev/#/patchset/20260319073651.79209-1-jiayuan.chen%40linux.devv1 -> v2: Use
state_in_sysfsinstead of reordering the teardown sequence.
https://lore.kernel.org/linux-ext4/[email protected]/--- fs/ext4/ext4.h | 1 + fs/ext4/super.c | 1 + fs/ext4/sysfs.c | 10 +++++++++- 3 files changed, 11 insertions(+), 1 deletion(-)fs/ext4/ext4.h +1
-0@@ -1574,6 +1574,7 @@ struct ext4_sb_info {struct proc_dir_entry *s_proc;struct kobject s_kobj;struct completion s_kobj_unregister; +struct mutex s_error_notify_mutex;/* protects sysfs_notify vs kobject_del */ struct super_block *s_sb;struct buffer_head *s_mmp_bh;fs/ext4/super.c +1
-0@@ -5397,6 +5397,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)timer_setup (&sbi->s_err_report, print_daily_error_info,0 );spin_lock_init (&sbi->s_error_lock); +mutex_init (&sbi->s_error_notify_mutex);INIT_WORK (&sbi->s_sb_upd_work, update_super_work); err =ext4_group_desc_init (sb, es, logical_sb_block, &first_not_zeroed);fs/ext4/sysfs.c +9
-1@@ -597,7 +597,10 @@ static const struct kobj_type ext4_feat_ktype = {void ext4_notify_error_sysfs (struct ext4_sb_info *sbi) { -sysfs_notify (&sbi->s_kobj,NULL ,"errors_count" ); +mutex_lock (&sbi->s_error_notify_mutex); +if (sbi->s_kobj.state_in_sysfs) +sysfs_notify (&sbi->s_kobj,NULL ,"errors_count" ); +mutex_unlock (&sbi->s_error_notify_mutex); }static struct kobject *ext4_root;@@ -610,8 +613,10 @@ int ext4_register_sysfs(struct super_block *sb)int err;init_completion (&sbi->s_kobj_unregister); +mutex_lock (&sbi->s_error_notify_mutex); err =kobject_init_and_add (&sbi->s_kobj, &ext4_sb_ktype, ext4_root,"%s" , sb->s_id); +mutex_unlock (&sbi->s_error_notify_mutex);if (err) {kobject_put (&sbi->s_kobj);wait_for_completion (&sbi->s_kobj_unregister);@@ -644,7 +649,10 @@ void ext4_unregister_sysfs(struct super_block *sb)if (sbi->s_proc)remove_proc_subtree (sb->s_id, ext4_proc_root); + +mutex_lock (&sbi->s_error_notify_mutex);kobject_del (&sbi->s_kobj); +mutex_unlock (&sbi->s_error_notify_mutex); }int __init ext4_init_sysfs (void )Signature · 1 line
2.43.0
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR
Quoted 12 lines · Jiayuan Chen
From: Jiayuan Chen <[email protected]>
Commit
b98535d09179("ext4: fixbug_oninstart_this_handleduring umount filesystem") movedext4_unregister_sysfs()before flushings_sb_upd_workto prevent new error work from being queued via/proc/fs/ext4/xx/mb_groupsreads during unmount. However, this introduced a use-after-free becauseupdate_super_workcallsext4_notify_error_sysfs()->sysfs_notify()which accesses the kobject'skernfs_nodeafter it has been freed bykobject_del()inext4_unregister_sysfs():
update_super_workext4_put_super
----------------- --------------
Quoted 13 lines
ext4_unregister_sysfs(sb)
kobject_del(&sbi->s_kobj)
__kobject_del()
sysfs_remove_dir()
kobj->sd = NULL
sysfs_put(sd)
kernfs_put() // RCU free
ext4_notify_error_sysfs(sbi)
sysfs_notify(&sbi->s_kobj)
kn = kobj->sd // stale pointer
kernfs_get(kn) // UAF on freed kernfs_node
ext4_journal_destroy()
flush_work(&sbi->s_sb_upd_work)Yes, the race between kobject_del() and sysfs_notify() -> kernfs_get() is real.
and sysfs_remove_dir() explicitely says:
* In general, kobject owner is responsible for ensuring removal
* doesn't race with other operations and sysfs doesn't provide any
* protection
Hence this patch which adds a new mutex lock to prevent the above race, looks good to me. So please feel free to add:
Quoted 9 lines · Jiayuan Chen
Commit
b98535d09179("ext4: fixbug_oninstart_this_handleduring umount filesystem") movedext4_unregister_sysfs()before flushings_sb_upd_workto prevent new error work from being queued via/proc/fs/ext4/xx/mb_groupsreads during unmount. However, this introduced a use-after-free becauseupdate_super_workcallsext4_notify_error_sysfs()->sysfs_notify()which accesses the kobject'skernfs_nodeafter it has been freed bykobject_del()inext4_unregister_sysfs():[...]
Applied, thanks!
[1/1] ext4: fix use-after-free in update_super_work when racing with umount
commit: d15e4b0a418537aafa56b2cb80d44add83e83697
Best regards,
Signature · 1 line
Theodore Ts'o <[email protected]>