All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Tejun Heo" <tj@kernel.org>
Cc: sched-ext@lists.linux.dev
Subject: Re: [PATCH v2 sched_ext/for-7.2] sched_ext: Add scx_task_iter_relock() and use it in scx_root_enable_workfn()
Date: Mon, 11 May 2026 22:41:52 +0000	[thread overview]
Message-ID: <20260511224153.953EAC2BCB0@smtp.kernel.org> (raw)
In-Reply-To: <20260510224511.2683323-1-tj@kernel.org>

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Dereferencing `p->comm` and `p->pid` in `scx_error()` after dropping the task reference and releasing locks causes a Use-After-Free.
--

commit 3693e9669076c05d1b5519c1c28733cc325c69fb
Author: Tejun Heo <tj@kernel.org>
sched_ext: Add scx_task_iter_relock() and use it in scx_root_enable_workfn()

This commit introduces scx_task_iter_relock() to re-acquire scx_tasks_lock
and optionally the task's runqueue lock. It applies this new function in
scx_root_enable_workfn() to properly manage locking during task
initialization and hash insertion.

> diff --git a/kernel/sched/ext.c b/kernel/sched/ext.c
> index 1d7f206941f0c..2238bd005dc7b 100644
> --- a/kernel/sched/ext.c
> +++ b/kernel/sched/ext.c

[ ... ]

> @@ -7135,6 +7153,9 @@ static void scx_root_enable_workfn(struct kthread_work *work)
>  		scx_task_iter_unlock(&sti);
>  
>  		ret = scx_init_task(sch, p, false);
> +
> +		scx_task_iter_relock(&sti, p);
> +
>  		if (ret) {
>  			put_task_struct(p);
>  			scx_task_iter_stop(&sti);

This regression wasn't introduced by this patch, but existed previously. Can
this cause a use-after-free when scx_error() is called?

Looking at the error path just below this diff chunk:

    if (ret) {
        put_task_struct(p);
        scx_task_iter_stop(&sti);
        scx_error(sch, "ops.init_task() failed (%d) for %s[%d]",
                  ret, p->comm, p->pid);

If put_task_struct() drops the last reference to the task, and
scx_task_iter_stop() drops the iterator locks (which unconditionally
re-enables local interrupts via raw_spin_unlock_irq()), could an RCU grace
period expire and physically free the task struct before scx_error() accesses
p->comm and p->pid?

Would it be safer to move the put_task_struct() call until after the
scx_error() formatting completes?

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

      reply	other threads:[~2026-05-11 22:41 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-10 22:23 [PATCH sched_ext/for-7.2] sched_ext: Add scx_task_iter_relock() and use it in scx_root_enable_workfn() Tejun Heo
2026-05-10 22:45 ` [PATCH v2 " Tejun Heo
2026-05-11 22:41   ` sashiko-bot [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260511224153.953EAC2BCB0@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=sashiko@lists.linux.dev \
    --cc=sched-ext@lists.linux.dev \
    --cc=tj@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.