Re: [PATCH sched_ext/for-7.4] sched_ext: Clear a sub-scheduler's caps before ops.sub_detach()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] cleanup: mixing goto-based error handling with guard() declarations in scx_bpf_sub_grant()
--

commit 289fe25628ccca2188e3c04a46e7f9601f2bbbe1
Author: Tejun Heo <tj@kernel.org>

sched_ext: Clear a sub-scheduler's caps before ops.sub_detach()

This commit clears every capability of a sub-scheduler before calling
ops.sub_detach(). This ensures the dying sub-scheduler cannot modify
state after the parent has cleaned it up.

> diff --git a/kernel/sched/ext/sub.c b/kernel/sched/ext/sub.c
> --- a/kernel/sched/ext/sub.c
> +++ b/kernel/sched/ext/sub.c

[ ... ]

> @@ -2321,6 +2355,12 @@ __bpf_kfunc s32 scx_bpf_sub_grant(u64 cgroup_id, u64 caps,
>  		scoped_guard (raw_spinlock, &pps->lock) {
>  			guard(raw_spinlock_nested)(&cps->lock);
>  
> +			/* the child is being disabled, see clear_all_caps() */
> +			if (unlikely(READ_ONCE(child->aborting))) {
> +				ret = -ENODEV;
> +				goto out;
> +			}

[Severity: Medium]
Does jumping out of the scoped_guard() and guard() scopes using a goto
violate the cleanup subsystem guidelines?

According to the cleanup subsystem documentation, the expectation is that
usage of goto and cleanup helpers should never be mixed in the same
function. This pattern creates confusing ownership semantics.

> +
>  			/*
>  			 * Narrow granted_cids to cids the parent holds every
>  			 * requested cap on. All-or-nothing per cid.
> @@ -2373,9 +2413,10 @@ __bpf_kfunc s32 scx_bpf_sub_grant(u64 cgroup_id, u64 caps,
>  		}
>  	}
>  
> +	ret = any_denied ? -EPERM : 0;
> +out:
>  	caps_updated_deliver(&to_deliver);
> -
> -	return any_denied ? -EPERM : 0;
> +	return ret;
>  }

[Severity: Medium]
Could this goto-based cleanup label be avoided to adhere to the scope-based
cleanup guidelines? Since this function now mixes goto cleanup with guards,
should all resources be converted to scope-based cleanups, or should the
goto be refactored?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/b46300eda6da03d56262fcdcbfc9b4fa@kernel.org?part=1