Re: [RFC PATCH] sched: irq: cpu-hotplug race vs set_cpus_allowed_ptr()

[Sebastian Andrzej Siewior]

Hello :)

On 2026-09-07 10:58:25 [+0200], To syzbot wrote:
> syzbot reported a race of assigning a CPU affinity during CPU hotplug
> operation.
> After the affinity of an IRQ has been changed, the IRQ-core sets
> IRQTF_AFFINITY and wakes the relevant interrupt threads which need to
> adjust their affinity mask. The thread will then invoke
> set_cpus_allowed_ptr() to update the mask. Based on the new mask a CPU
> is chosen on which the thread should run.
> 
> It is verified that this CPU is online however there is no guarantee
> that this CPU remains online while affine_move_task() is moving _this_
> task. Since the current task requests the migration the stopper/
> migration_cpu_stop() is involved at which point the task pauses for a
> while. If the CPU goes offline (or is no longer cpu_online_mask) then
> __migrate_task() (due to is_cpu_allowed() reject) will return the rq of
> the CPU on which the task is currently running (which does not match
> it's task_struct::cpus_mask).
> 
> The aftermath:
> The IRQ-thread's CPU and task's cpus_mask do not match. A
> migrate_disable()-> schedule() will change task_struct::cpus_ptr
> ensuring that the following migrate_enable() will update the task to the
> requested affinity mask. At this point, affine_move_task() expects
> task_struct::migration_pending set but it is NULL because noone
> requested an affinity change while the task was in migrate-disable
> section.
> 
> I see two ways of fixing this:
> - Holding the cpus_read_lock while set_cpus_allowed_ptr() is invoked.
>   This ensure that the CPU remains in the cpu_online_mask while the
>   migration task moving the task over. Should the mask be already
>   invalid, then it is rejected otherwise the operaton completes.
>   Maybe we should also check if cpus_read_lock is held during the
>   invocation of set_cpus_allowed_ptr() so we don't get this problem from
>   other callers.

So sashiko complains about deadlock of cpus_write_lock() in the CPU down
path and calling irq_migrate_all_off_this_cpu() vs and
irq_thread_check_affinity() doing cpus_read_lock().
As far as I see, there is no deadlock because the mask is updated, the
tread is woken, the tread blocks with cpus_read_lock() until the CPU is
down.
But there is nothing that waits for the interrupt thread to complete,
there is no synchronize_irq() invocation which would be required for the
deadlock.

> - Should __migrate_task() fail to return the requested rq make sure its
>   CPU is part task_struct::cpus_mask. This ensures that the current CPU
>   is still part of mask avoiding a possible push by migrate_enable().
>   sched_class::set_cpus_allowed did not see this mask. It might be a bit
>   inconsistent and feels a bit like select_fallback_rq() without the
>   printk.

Here sashiko complains:
| If __migrate_task() fails (e.g., due to a concurrent CPU offline event), this
| fallback code forcibly modifies p->cpus_mask with cpumask_set_cpu(). This
| skips the required p->sched_class->set_cpus_allowed() callback and fails to
| update p->nr_cpus_allowed.
| Can this bypass global EDF real-time invariants for SCHED_DEADLINE (as
| set_cpus_allowed_dl() is skipped), and allow unprivileged tasks to escape
| affinity boundaries, giving them access to CPUs outside their cpusets?

We did do_set_cpus_allowed() for CPU X and can't migrate there now. We
also don't know if the invocation was successful.

Looking at this, it is not possible hold the CPU-hotplug lock and not
every caller holds it. This seems to be limited to migrate-disable.
Should this happen without it, then the during wake-up the scheduler
will just print "process … no longer affine to cpu" pick a CPU and be
done with it.
Maybe set the bit followed by do_set_cpus_allowed()?

> Both changes are implemented to illustrate, one is enough.
> I can reproduce this back on v6.8, therefore I assume we have this since
> day #1 of migrate-disable.
> 
> Reported-by: syzbot+6835d5c11145e4f77057@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://lore.kernel.org/all/6a9919ac.94649fcc.25487e.0005.GAE@xxxxxxxxxx/
> Signed-off-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx>
> ---
>  kernel/irq/manage.c | 6 ++++--
>  kernel/sched/core.c | 2 ++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 57eff26fa646a..75b6798c1d97c 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -8,6 +8,7 @@
>  
>  #define pr_fmt(fmt) "genirq: " fmt
>  
> +#include <linux/cpuhplock.h>
>  #include <linux/irq.h>
>  #include <linux/kthread.h>
>  #include <linux/module.h>
> @@ -1044,8 +1045,9 @@ static void irq_thread_check_affinity(struct irq_desc *desc, struct irqaction *a
>  		m = irq_data_get_effective_affinity_mask(&desc->irq_data);
>  		cpumask_copy(mask, m);
>  	}
> -
> -	set_cpus_allowed_ptr(current, mask);
> +	scoped_guard(cpus_read_lock) {
> +		set_cpus_allowed_ptr(current, mask);
> +	}
>  	free_cpumask_var(mask);
>  }
>  #else
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index b998ef6b87af4..b090a374fa728 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -2674,6 +2674,8 @@ static int migration_cpu_stop(void *data)
>  		if (task_on_rq_queued(p)) {
>  			update_rq_clock(rq);
>  			rq = __migrate_task(rq, &rf, p, arg->dest_cpu);
> +			if (rq != cpu_rq(arg->dest_cpu))
> +				cpumask_set_cpu(rq->cpu, &p->cpus_mask);
>  		} else {
>  			p->wake_cpu = arg->dest_cpu;
>  		}

Sebastian