From 70caae813f683250ca8d3e8ce7a40bdd4efa7916 Mon Sep 17 00:00:00 2001 From: David Vernet Date: Wed, 14 Feb 2024 11:54:48 -0600 Subject: [PATCH 1/2] scx: Avoid memory leak of scx_root_kobj We're making two mistakes with respect to object lifecycle for scx_root_kobj: - We're not calling kobject_put() if kobject_init_and_add() returns an error code. According to the API for that function, we should be. - We weren't kobject_putt()'ing scx_root_kobj on the scx_ops_disable_workfn() path. We were calling kobject_del(), but this doesn't actually call the release callback under the hood. It just unlinks the kobject from the hierarchy. Address both of these. With these changes, we no longer trigger the memory leak detected by Syzkaller in https://github.com/sched-ext/sched_ext/issues/142. Without this patch, we see the following with memleak enabled: unreferenced object 0xffff8dd78a99f000 (size 64): comm "runner", pid 2383, jiffies 4294697813 (age 128.878s) hex dump (first 32 bytes): 78 17 5f be ff ff ff ff 08 f0 99 8a d7 8d ff ff x._............. 08 f0 99 8a d7 8d ff ff 00 00 00 00 00 00 00 00 ................ backtrace: [<0000000034bda486>] __kmem_cache_alloc_node+0x109/0x1f0 [<00000000a91612ee>] kmalloc_trace+0x29/0xa0 [<00000000bfe2a4b7>] bpf_scx_reg+0x135/0xd80 [<000000000e3a5728>] bpf_struct_ops_link_create+0xd7/0x130 [<00000000b14e1608>] __sys_bpf+0x2a9/0x530 [<000000003140300e>] __x64_sys_bpf+0x17/0x20 [<00000000fa410384>] do_syscall_64+0x4d/0xf0 [<000000008cbc0d51>] entry_SYSCALL_64_after_hwframe+0x6f/0x77 With it, we see no such entries. Fixes: e7a7781b485d4 ("scx: Add /sys/kernel/sched_ext interface") Signed-off-by: David Vernet --- kernel/sched/ext.c | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c index 4d78fb2eb453d..327fcdfb6b564 100644 --- a/kernel/sched/ext.c +++ b/kernel/sched/ext.c @@ -3395,7 +3395,14 @@ static void scx_ops_disable_workfn(struct kthread_work *work) SCX_CALL_OP(SCX_KF_UNLOCKED, exit, ei); cancel_delayed_work_sync(&scx_watchdog_work); + /* + * Delete the kobject from the hierarchy eagerly in addition to just + * dropping a reference. Otherwise, if the object is deleted + * asynchronously, sysfs could observe an object of the same name still + * in the hierarchy when another scheduler is loaded. + */ kobject_del(scx_root_kobj); + kobject_put(scx_root_kobj); scx_root_kobj = NULL; memset(&scx_ops, 0, sizeof(scx_ops)); @@ -3633,16 +3640,10 @@ static int scx_ops_enable(struct sched_ext_ops *ops) goto err_unlock; } - scx_exit_info = alloc_exit_info(); - if (!scx_exit_info) { - ret = -ENOMEM; - goto err; - } - scx_root_kobj = kzalloc(sizeof(*scx_root_kobj), GFP_KERNEL); if (!scx_root_kobj) { ret = -ENOMEM; - goto err; + goto err_unlock; } scx_root_kobj->kset = scx_kset; @@ -3650,6 +3651,12 @@ static int scx_ops_enable(struct sched_ext_ops *ops) if (ret < 0) goto err; + scx_exit_info = alloc_exit_info(); + if (!scx_exit_info) { + ret = -ENOMEM; + goto err_del; + } + /* * Set scx_ops, transition to PREPPING and clear exit info to arm the * disable path. Failure triggers full disabling from here on. @@ -3864,11 +3871,11 @@ static int scx_ops_enable(struct sched_ext_ops *ops) return 0; +err_del: + kobject_del(scx_root_kobj); err: - if (scx_root_kobj) { - kfree(scx_root_kobj); - scx_root_kobj = NULL; - } + kobject_put(scx_root_kobj); + scx_root_kobj = NULL; if (scx_exit_info) { free_exit_info(scx_exit_info); scx_exit_info = NULL; From 73f45644eda4e6fb62cc71c37cf133441f6bfca6 Mon Sep 17 00:00:00 2001 From: David Vernet Date: Wed, 14 Feb 2024 14:42:06 -0600 Subject: [PATCH 2/2] v6.7.4-scx2 Signed-off-by: David Vernet --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index c17a1f2fdf868..80b02a47aa2af 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ VERSION = 6 PATCHLEVEL = 7 SUBLEVEL = 4 -EXTRAVERSION = -scx1 +EXTRAVERSION = -scx2 NAME = Hurr durr I'ma ninja sloth # *DOCUMENTATION*