[PATCHSET sched_ext/for-7.1] sched_ext: Fix kobject and cgroup ref error handling in scx_alloc_and_add_sched()

[PATCHSET sched_ext/for-7.1] sched_ext: Fix kobject and cgroup ref error handling in scx_alloc_and_add_sched()

[Tejun Heo]

Hello,

Fix error handling bugs in scx_alloc_and_add_sched():

 0001 sched_ext: Fix cgroup double-put on sub-sched abort path
 0002 sched_ext: Use kobject_put() for kobject_init_and_add() failure in scx_alloc_and_add_sched()

Based on sched_ext/for-7.1 (f96bc0fa92be).

 kernel/sched/ext.c | 46 ++++++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 18 deletions(-)

--
tejun

[PATCH 1/2] sched_ext: Fix cgroup double-put on sub-sched abort path

[Tejun Heo]

The abort path in scx_sub_enable_workfn() fell through to out_put_cgrp,
double-putting the cgroup ref already owned by sch->cgrp. It also skipped
kthread_flush_work() needed to flush the disable path.

Relocate the abort block above err_unlock_and_disable so it falls through to
err_disable.

Fixes: ebeca1f930ea ("sched_ext: Introduce cgroup sub-sched support")
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 9202c6d7a771..2f70effcc4a6 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -7050,6 +7050,13 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
 	ret = 0;
 	goto out_unlock;
 
+out_put_cgrp:
+	cgroup_put(cgrp);
+out_unlock:
+	mutex_unlock(&scx_enable_mutex);
+	cmd->ret = ret;
+	return;
+
 abort:
 	put_task_struct(p);
 	scx_task_iter_stop(&sti);
@@ -7063,15 +7070,6 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
 		}
 	}
 	scx_task_iter_stop(&sti);
-	scx_cgroup_unlock();
-	percpu_up_write(&scx_fork_rwsem);
-out_put_cgrp:
-	cgroup_put(cgrp);
-out_unlock:
-	mutex_unlock(&scx_enable_mutex);
-	cmd->ret = ret;
-	return;
-
 err_unlock_and_disable:
 	/* we'll soon enter disable path, keep bypass on */
 	scx_cgroup_unlock();
-- 
2.53.0

Re: [PATCH 1/2] sched_ext: Fix cgroup double-put on sub-sched abort path

[Andrea Righi]

On Sun, Mar 15, 2026 at 07:43:27PM -1000, Tejun Heo wrote:
> The abort path in scx_sub_enable_workfn() fell through to out_put_cgrp,
> double-putting the cgroup ref already owned by sch->cgrp. It also skipped
> kthread_flush_work() needed to flush the disable path.
> 
> Relocate the abort block above err_unlock_and_disable so it falls through to
> err_disable.
> 
> Fixes: ebeca1f930ea ("sched_ext: Introduce cgroup sub-sched support")

Maybe we should point to commit 337ec00b1d9c ("sched_ext: Implement cgroup
sub-sched enabling and disabling")

That's where we added the real sub-sched enable/disable implementation and
the abort block.

Apart from that looks good.

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> Signed-off-by: Tejun Heo <tj@kernel.org>
> ---
>  kernel/sched/ext.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 9202c6d7a771..2f70effcc4a6 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -7050,6 +7050,13 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
>  	ret = 0;
>  	goto out_unlock;
>  
> +out_put_cgrp:
> +	cgroup_put(cgrp);
> +out_unlock:
> +	mutex_unlock(&scx_enable_mutex);
> +	cmd->ret = ret;
> +	return;
> +
>  abort:
>  	put_task_struct(p);
>  	scx_task_iter_stop(&sti);
> @@ -7063,15 +7070,6 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
>  		}
>  	}
>  	scx_task_iter_stop(&sti);
> -	scx_cgroup_unlock();
> -	percpu_up_write(&scx_fork_rwsem);
> -out_put_cgrp:
> -	cgroup_put(cgrp);
> -out_unlock:
> -	mutex_unlock(&scx_enable_mutex);
> -	cmd->ret = ret;
> -	return;
> -
>  err_unlock_and_disable:
>  	/* we'll soon enter disable path, keep bypass on */
>  	scx_cgroup_unlock();
> -- 
> 2.53.0
> 

Re: [PATCH 1/2] sched_ext: Fix cgroup double-put on sub-sched abort path

[Tejun Heo]

Hello,

> Tejun Heo (2):
>   sched_ext: Fix cgroup double-put on sub-sched abort path
>   sched_ext: Use kobject_put() for kobject_init_and_add() failure in
>     scx_alloc_and_add_sched()

Applied 1-2 to sched_ext/for-7.1 w/ the Fixes tag updated per your
suggestion.

Thanks.

--
tejun

[PATCH 2/2] sched_ext: Use kobject_put() for kobject_init_and_add() failure in scx_alloc_and_add_sched()

[Tejun Heo]

kobject_init_and_add() failure requires kobject_put() for proper cleanup, but
the error paths were using kfree(sch) possibly leaking the kobject name. The
kset_create_and_add() failure was already using kobject_put() correctly.

Switch the kobject_init_and_add() error paths to use kobject_put(). As the
release path puts the cgroup ref, make scx_alloc_and_add_sched() always
consume @cgrp via a new err_put_cgrp label at the bottom of the error chain
and update scx_sub_enable_workfn() accordingly.

Fixes: 17108735b47d ("sched_ext: Use dynamic allocation for scx_sched")
Reported-by: David Carlier <devnexen@gmail.com>
Link: https://lore.kernel.org/r/20260314134457.46216-1-devnexen@gmail.com
Signed-off-by: Tejun Heo <tj@kernel.org>
---
 kernel/sched/ext.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
index 2f70effcc4a6..b942918fa364 100644
--- a/kernel/sched/ext.c
+++ b/kernel/sched/ext.c
@@ -6353,6 +6353,10 @@ static struct scx_sched_pnode *alloc_pnode(struct scx_sched *sch, int node)
 	return pnode;
 }
 
+/*
+ * Allocate and initialize a new scx_sched. @cgrp's reference is always
+ * consumed whether the function succeeds or fails.
+ */
 static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
 						 struct cgroup *cgrp,
 						 struct scx_sched *parent)
@@ -6362,8 +6366,10 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
 	s32 node, cpu, ret, bypass_fail_cpu = nr_cpu_ids;
 
 	sch = kzalloc_flex(*sch, ancestors, level);
-	if (!sch)
-		return ERR_PTR(-ENOMEM);
+	if (!sch) {
+		ret = -ENOMEM;
+		goto err_put_cgrp;
+	}
 
 	sch->exit_info = alloc_exit_info(ops->exit_dump_len);
 	if (!sch->exit_info) {
@@ -6468,8 +6474,8 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
 		ret = kobject_init_and_add(&sch->kobj, &scx_ktype, NULL, "root");
 
 	if (ret < 0) {
-		kfree(sch->cgrp_path);
-		goto err_stop_helper;
+		kobject_put(&sch->kobj);
+		return ERR_PTR(ret);
 	}
 
 	if (ops->sub_attach) {
@@ -6479,11 +6485,12 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
 			return ERR_PTR(-ENOMEM);
 		}
 	}
-
 #else	/* CONFIG_EXT_SUB_SCHED */
 	ret = kobject_init_and_add(&sch->kobj, &scx_ktype, NULL, "root");
-	if (ret < 0)
-		goto err_stop_helper;
+	if (ret < 0) {
+		kobject_put(&sch->kobj);
+		return ERR_PTR(ret);
+	}
 #endif	/* CONFIG_EXT_SUB_SCHED */
 	return sch;
 
@@ -6506,6 +6513,8 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
 	free_exit_info(sch->exit_info);
 err_free_sch:
 	kfree(sch);
+err_put_cgrp:
+	cgroup_put(cgrp);
 	return ERR_PTR(ret);
 }
 
@@ -6577,6 +6586,7 @@ static void scx_root_enable_workfn(struct kthread_work *work)
 {
 	struct scx_enable_cmd *cmd = container_of(work, struct scx_enable_cmd, work);
 	struct sched_ext_ops *ops = cmd->ops;
+	struct cgroup *cgrp = root_cgroup();
 	struct scx_sched *sch;
 	struct scx_task_iter sti;
 	struct task_struct *p;
@@ -6593,7 +6603,8 @@ static void scx_root_enable_workfn(struct kthread_work *work)
 	if (ret)
 		goto err_unlock;
 
-	sch = scx_alloc_and_add_sched(ops, root_cgroup(), NULL);
+	cgroup_get(cgrp);
+	sch = scx_alloc_and_add_sched(ops, cgrp, NULL);
 	if (IS_ERR(sch)) {
 		ret = PTR_ERR(sch);
 		goto err_free_ksyncs;
@@ -6887,11 +6898,12 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
 	kobject_get(&parent->kobj);
 	raw_spin_unlock_irq(&scx_sched_lock);
 
+	/* scx_alloc_and_add_sched() consumes @cgrp whether it succeeds or not */
 	sch = scx_alloc_and_add_sched(ops, cgrp, parent);
 	kobject_put(&parent->kobj);
 	if (IS_ERR(sch)) {
 		ret = PTR_ERR(sch);
-		goto out_put_cgrp;
+		goto out_unlock;
 	}
 
 	ret = scx_link_sched(sch);
-- 
2.53.0

Re: [PATCH 2/2] sched_ext: Use kobject_put() for kobject_init_and_add() failure in scx_alloc_and_add_sched()

[Andrea Righi]

On Sun, Mar 15, 2026 at 07:43:28PM -1000, Tejun Heo wrote:
> kobject_init_and_add() failure requires kobject_put() for proper cleanup, but
> the error paths were using kfree(sch) possibly leaking the kobject name. The
> kset_create_and_add() failure was already using kobject_put() correctly.
> 
> Switch the kobject_init_and_add() error paths to use kobject_put(). As the
> release path puts the cgroup ref, make scx_alloc_and_add_sched() always
> consume @cgrp via a new err_put_cgrp label at the bottom of the error chain
> and update scx_sub_enable_workfn() accordingly.
> 
> Fixes: 17108735b47d ("sched_ext: Use dynamic allocation for scx_sched")
> Reported-by: David Carlier <devnexen@gmail.com>
> Link: https://lore.kernel.org/r/20260314134457.46216-1-devnexen@gmail.com
> Signed-off-by: Tejun Heo <tj@kernel.org>

Reviewed-by: Andrea Righi <arighi@nvidia.com>

Thanks,
-Andrea

> ---
>  kernel/sched/ext.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 2f70effcc4a6..b942918fa364 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c
> @@ -6353,6 +6353,10 @@ static struct scx_sched_pnode *alloc_pnode(struct scx_sched *sch, int node)
>  	return pnode;
>  }
>  
> +/*
> + * Allocate and initialize a new scx_sched. @cgrp's reference is always
> + * consumed whether the function succeeds or fails.
> + */
>  static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
>  						 struct cgroup *cgrp,
>  						 struct scx_sched *parent)
> @@ -6362,8 +6366,10 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
>  	s32 node, cpu, ret, bypass_fail_cpu = nr_cpu_ids;
>  
>  	sch = kzalloc_flex(*sch, ancestors, level);
> -	if (!sch)
> -		return ERR_PTR(-ENOMEM);
> +	if (!sch) {
> +		ret = -ENOMEM;
> +		goto err_put_cgrp;
> +	}
>  
>  	sch->exit_info = alloc_exit_info(ops->exit_dump_len);
>  	if (!sch->exit_info) {
> @@ -6468,8 +6474,8 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
>  		ret = kobject_init_and_add(&sch->kobj, &scx_ktype, NULL, "root");
>  
>  	if (ret < 0) {
> -		kfree(sch->cgrp_path);
> -		goto err_stop_helper;
> +		kobject_put(&sch->kobj);
> +		return ERR_PTR(ret);
>  	}
>  
>  	if (ops->sub_attach) {
> @@ -6479,11 +6485,12 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
>  			return ERR_PTR(-ENOMEM);
>  		}
>  	}
> -
>  #else	/* CONFIG_EXT_SUB_SCHED */
>  	ret = kobject_init_and_add(&sch->kobj, &scx_ktype, NULL, "root");
> -	if (ret < 0)
> -		goto err_stop_helper;
> +	if (ret < 0) {
> +		kobject_put(&sch->kobj);
> +		return ERR_PTR(ret);
> +	}
>  #endif	/* CONFIG_EXT_SUB_SCHED */
>  	return sch;
>  
> @@ -6506,6 +6513,8 @@ static struct scx_sched *scx_alloc_and_add_sched(struct sched_ext_ops *ops,
>  	free_exit_info(sch->exit_info);
>  err_free_sch:
>  	kfree(sch);
> +err_put_cgrp:
> +	cgroup_put(cgrp);
>  	return ERR_PTR(ret);
>  }
>  
> @@ -6577,6 +6586,7 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  {
>  	struct scx_enable_cmd *cmd = container_of(work, struct scx_enable_cmd, work);
>  	struct sched_ext_ops *ops = cmd->ops;
> +	struct cgroup *cgrp = root_cgroup();
>  	struct scx_sched *sch;
>  	struct scx_task_iter sti;
>  	struct task_struct *p;
> @@ -6593,7 +6603,8 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  	if (ret)
>  		goto err_unlock;
>  
> -	sch = scx_alloc_and_add_sched(ops, root_cgroup(), NULL);
> +	cgroup_get(cgrp);
> +	sch = scx_alloc_and_add_sched(ops, cgrp, NULL);
>  	if (IS_ERR(sch)) {
>  		ret = PTR_ERR(sch);
>  		goto err_free_ksyncs;
> @@ -6887,11 +6898,12 @@ static void scx_sub_enable_workfn(struct kthread_work *work)
>  	kobject_get(&parent->kobj);
>  	raw_spin_unlock_irq(&scx_sched_lock);
>  
> +	/* scx_alloc_and_add_sched() consumes @cgrp whether it succeeds or not */
>  	sch = scx_alloc_and_add_sched(ops, cgrp, parent);
>  	kobject_put(&parent->kobj);
>  	if (IS_ERR(sch)) {
>  		ret = PTR_ERR(sch);
> -		goto out_put_cgrp;
> +		goto out_unlock;
>  	}
>  
>  	ret = scx_link_sched(sch);
> -- 
> 2.53.0
>