mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
To: juri.lelli@redhat.com, peterz@infradead.org, mingo@redhat.com,
	vincent.guittot@linaro.org
Cc: linux-kernel@vger.kernel.org,
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
	tglx@kernel.org, will@kernel.org, boqun@kernel.org,
	dietmar.eggemann@arm.com, rostedt@goodmis.org,
	bsegall@google.com, mgorman@suse.de, vschneid@redhat.com,
	kprateek.nayak@amd.com, luca.abeni@santannapisa.it,
	mathieu.poirier@linaro.org, longman@redhat.com,
	ridong.chen@linux.dev, tj@kernel.org, hannes@cmpxchg.org,
	mkoutny@suse.com, shuah@kernel.org, bigeasy@linutronix.de,
	clrkwllms@kernel.org, parri.andrea@gmail.com,
	m.szyprowski@samsung.com, kayracizmeci@gmail.com,
	weihengqiuu@gmail.com, cgroups@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-rt-devel@lists.linux.dev,
	Sashiko <sashiko-bot@kernel.org>,
	stable@vger.kernel.org
Subject: [PATCH v5 01/29] sched: Fix cpuset_mutex double unlock in __sched_setscheduler()
Date: Sun,  8 Nov 2026 13:53:38 -0500	[thread overview]
Message-ID: <20261108185406.1549770-2-mathieu.desnoyers@efficios.com> (raw)
In-Reply-To: <20261108185406.1549770-1-mathieu.desnoyers@efficios.com>

From: Andrea Parri <parri.andrea@gmail.com>

A sched_setattr() with SCHED_FLAG_KEEP_POLICY on a SCHED_DEADLINE task
unlocks cpuset_mutex twice if the task's policy changes concurrently:

  WARNING: bad unlock balance detected!
  7.4.0-rc1 #1 Not tainted
  -------------------------------------
  repro/126 is trying to release lock (cpuset_mutex) at:
  [<ffffffff9a17cacc>] __sched_setscheduler+0x9cc/0xc40
  but there are no more locks to release!
  ...
  Call Trace:
   print_unlock_imbalance_bug.part.0+0xfb/0x110
   lock_release+0x286/0x2e0
   __mutex_unlock_slowpath+0x1a/0x400
   __sched_setscheduler+0x9cc/0xc40
   __x64_sys_sched_setattr+0xa3/0x1b0
   do_syscall_64+0x106/0x5f0
   entry_SYSCALL_64_after_hwframe+0x77/0x7f

__sched_setscheduler() samples p->policy before taking any lock, and
restarts from the recheck label if p->policy differs under the rq lock.
The restart drops cpuset_mutex but leaves cpuset_locked set, so a pass
that no longer needs the mutex still unlocks it on its way out:

  CPU 1                                CPU 2
  __sched_setscheduler(KEEP_POLICY)
    policy = p->policy /* DEADLINE */
                                       __sched_setscheduler(SCHED_OTHER)
                                         p->policy = SCHED_OTHER
    cpuset_locked = true
    cpuset_lock()
    rq = task_rq_lock(p)
    oldpolicy != p->policy
      cpuset_unlock()
      goto recheck
    /* no cpuset_lock() this time */
    ...
    if (cpuset_locked)
      cpuset_unlock()

Without CONFIG_DEBUG_MUTEXES, or CONFIG_DEBUG_RT_MUTEXES on PREEMPT_RT,
nothing warns. The second unlock still clears the owner field regardless
of who holds the mutex, so another task can lose cpuset_mutex inside its
critical section. Entering SCHED_DEADLINE needs CAP_SYS_NICE, but a
racing caller that owns the task does not need it if it samples a policy
other than SCHED_DEADLINE.

Clear cpuset_locked when dropping the mutex for the restart. This should be
safe because the flag is only tested on the paths leaving the next pass,
after that pass has set it again if it took the mutex.

The following program, run as root, reproduces the splat, which no longer
appears with this applied. On PREEMPT_RT it crashes the kernel within
seconds, which this also fixes:

  #define _GNU_SOURCE
  #include <pthread.h>
  #include <stdint.h>
  #include <sys/syscall.h>
  #include <unistd.h>

  struct attr {
      uint32_t size, policy;
      uint64_t flags;
      int32_t nice;
      uint32_t prio;
      uint64_t runtime, deadline, period;
  };

  static volatile int stop;
  static volatile pid_t target;

  static void set(struct attr a)
  {
      a.size = sizeof(a);
      syscall(SYS_sched_setattr, target, &a, 0);
  }

  static void *sleeper(void *arg)
  {
      target = syscall(SYS_gettid);
      while (!stop)
          usleep(1000);
      return arg;
  }

  static void *switcher(void *arg)
  {
      while (!stop) {
          /* SCHED_DEADLINE, 2us every 1s */
          set((struct attr){ .policy = 6, .runtime = 2000,
                             .deadline = 1000000000,
                             .period = 1000000000 });
          set((struct attr){ .policy = 0 }); /* SCHED_OTHER */
      }
      return arg;
  }

  static void *racer(void *arg)
  {
      /* KEEP_POLICY | KEEP_PARAMS */
      while (!stop)
          set((struct attr){ .flags = 0x18 });
      return arg;
  }

  int main(void)
  {
      pthread_t t[5];
      int i;

      pthread_create(&t[0], NULL, sleeper, NULL);
      while (!target)
          usleep(100);
      pthread_create(&t[1], NULL, switcher, NULL);
      for (i = 2; i < 5; i++)
          pthread_create(&t[i], NULL, racer, NULL);
      sleep(30);
      stop = 1;
      for (i = 0; i < 5; i++)
          pthread_join(t[i], NULL);
      return 0;
  }

An LLM helped find the PREEMPT_RT crash and write this patch and its test.

Fixes: 111cd11bbc54 ("sched/cpuset: Bring back cpuset_mutex")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://sashiko.dev/#/patchset/20261016062629.2102883-1-sh_def%40163.com
Signed-off-by: Andrea Parri <parri.andrea@gmail.com>
Cc: stable@vger.kernel.org
Reviewed-by: Waiman Long <longman@redhat.com>
Reviewed-by: Kayra Cizmeci <kayracizmeci@gmail.com>
Reviewed-by: Juri Lelli <juri.lelli@redhat.com>
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
---
 kernel/sched/syscalls.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/kernel/sched/syscalls.c b/kernel/sched/syscalls.c
index 45a127deeadd..2af550f92ac6 100644
--- a/kernel/sched/syscalls.c
+++ b/kernel/sched/syscalls.c
@@ -640,8 +640,10 @@ int __sched_setscheduler(struct task_struct *p,
 	if (unlikely(oldpolicy != -1 && oldpolicy != p->policy)) {
 		policy = oldpolicy = -1;
 		task_rq_unlock(rq, p, &rf);
-		if (cpuset_locked)
+		if (cpuset_locked) {
 			cpuset_unlock();
+			cpuset_locked = false;
+		}
 		goto recheck;
 	}
 
-- 
2.47.3


  reply	other threads:[~2026-11-08 18:54 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-08 18:53 [PATCH v5 00/29] sched/deadline: bandwidth accounting fixes and selftests Mathieu Desnoyers
2026-11-08 18:53 ` Mathieu Desnoyers [this message]
2026-11-08 18:53 ` [PATCH v5 02/29] sched: Refuse SCHED_FLAG_KEEP_PARAMS that would change the policy or the class Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 03/29] sched/deadline: Do not admit bandwidth for parameters that are kept Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 04/29] sched: Let a task boosted by a deadline waiter change its policy Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 05/29] sched/deadline: Do not divide by zero active CPUs of a root domain Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 06/29] sched: Take cpuset_mutex for a deadline policy change only with pi Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 07/29] sched/deadline: Do not lose a throttled task switched out and back Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 08/29] sched/deadline: Take a deadline boost from the waiter's rt_mutex node Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 09/29] sched/deadline: Reset the deadline and the runtime with the parameters Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 10/29] sched/deadline: Reset the donor's runtime at a deboost in the class Mathieu Desnoyers
2026-11-08 19:17   ` sashiko-bot
2026-11-08 18:53 ` [PATCH v5 11/29] sched/deadline: Release the bandwidth of a task that leaves the class asleep Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 12/29] sched/deadline: Reject SCHED_DEADLINE on exited tasks Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 13/29] sched/deadline: Keep departing bandwidth across a root domain rebuild Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 14/29] sched/deadline: Fix this_bw when a task is changed during a remote wakeup Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 15/29] sched/deadline: Rebuild the root domains' accounting from the task list Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 16/29] sched/deadline: Record the root domain holding a task's bandwidth Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 17/29] sched/deadline: Leave a dying task out of a rebuild of the accounting Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 18/29] sched/deadline: Move a task's bandwidth only where room was made Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 19/29] cgroup/cpuset: Wait for an attach before rebuilding the sched domains Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 20/29] cgroup/cpuset: Give back the bandwidth an attach did not move Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 21/29] cgroup/cpuset: Reserve the bandwidth in the root domain a task moves to Mathieu Desnoyers
2026-11-08 18:53 ` [PATCH v5 22/29] sched/deadline: Make a rebuild use the CPU recorded with a task's bandwidth Mathieu Desnoyers
2026-11-08 18:54 ` [PATCH v5 23/29] sched/deadline: Move a task's bandwidth when it must leave its CPU Mathieu Desnoyers
2026-11-08 18:54 ` [PATCH v5 24/29] sched/deadline: Cancel the replenishment timer with the parameters Mathieu Desnoyers
2026-11-08 18:54 ` [PATCH v5 25/29] sched/deadline: Count the bandwidth a boosted task is given Mathieu Desnoyers
2026-11-08 18:54 ` [PATCH v5 26/29] sched/debug: Show the deadline bandwidth of each runqueue Mathieu Desnoyers
2026-11-08 18:54 ` [PATCH v5 27/29] selftests/sched: Add a test of SCHED_DEADLINE policy round trips Mathieu Desnoyers
2026-11-08 18:54 ` [PATCH v5 28/29] selftests/sched: Test SCHED_DEADLINE bandwidth across a root domain rebuild Mathieu Desnoyers
2026-11-08 18:54 ` [PATCH v5 29/29] selftests/sched: Add a test of SCHED_DEADLINE bandwidth accounting Mathieu Desnoyers

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=20261108185406.1549770-2-mathieu.desnoyers@efficios.com \
    --to=mathieu.desnoyers@efficios.com \
    --cc=bigeasy@linutronix.de \
    --cc=boqun@kernel.org \
    --cc=bsegall@google.com \
    --cc=cgroups@vger.kernel.org \
    --cc=clrkwllms@kernel.org \
    --cc=dietmar.eggemann@arm.com \
    --cc=hannes@cmpxchg.org \
    --cc=juri.lelli@redhat.com \
    --cc=kayracizmeci@gmail.com \
    --cc=kprateek.nayak@amd.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=longman@redhat.com \
    --cc=luca.abeni@santannapisa.it \
    --cc=m.szyprowski@samsung.com \
    --cc=mathieu.poirier@linaro.org \
    --cc=mgorman@suse.de \
    --cc=mingo@redhat.com \
    --cc=mkoutny@suse.com \
    --cc=parri.andrea@gmail.com \
    --cc=peterz@infradead.org \
    --cc=ridong.chen@linux.dev \
    --cc=rostedt@goodmis.org \
    --cc=sashiko-bot@kernel.org \
    --cc=shuah@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@kernel.org \
    --cc=tj@kernel.org \
    --cc=vincent.guittot@linaro.org \
    --cc=vschneid@redhat.com \
    --cc=weihengqiuu@gmail.com \
    --cc=will@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®