Re: [PATCH 22/23] cgroup/cpuset: Prevent offline_disabled CPUs from being used in isolated partition

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Unconditional NULL pointer dereference causes boot-time kernel panic on architectures with late CPU registration or systems with non-present possible CPUs.
- [Medium] Caching the `offline_disabled` state at early boot misses dynamic runtime updates, allowing illegal CPUs to enter isolated partitions.
- [Low] Buffer truncation for the isolated partition error message when multiple CPUs have `offline_disabled` set.
--

commit b13420f1f2cd136dd5780ea125eec8631214ef49
Author: Waiman Long <longman@redhat.com>

cgroup/cpuset: Prevent offline_disabled CPUs from being used in isolated partition

This commit introduces a check to prevent CPUs with the offline_disabled
flag set from being added to isolated cpuset partitions. It initializes
a cpumask during late boot to track these CPUs and checks against it
when partition states are updated.

> diff --git a/kernel/cgroup/cpuset.c b/kernel/cgroup/cpuset.c
> index 5f6b4e67748f2..f3af8ef6c64ee 100644
> --- a/kernel/cgroup/cpuset.c
> +++ b/kernel/cgroup/cpuset.c

[ ... ]

> @@ -164,6 +167,12 @@ static cpumask_var_t	isolated_mirq_cpus;	/* T */
>  static bool		boot_nohz_le_domain __ro_after_init;
>  static bool		boot_mirq_le_domain __ro_after_init;
>  
> +/*
> + * Cpumask of CPUs with offline_disabled set
> + * The cpumask is effectively __ro_after_init.
> + */
> +static cpumask_var_t	offline_disabled_cpus;

Is it safe to assume this mask is effectively ro_after_init? On some
architectures like ARM64, the offline_disabled state can be set dynamically
at runtime, such as when an asymmetric 32-bit EL0 CPU is brought online.
Does caching this state at boot miss these dynamic updates, potentially
allowing dynamically restricted CPUs to enter isolated partitions later on?

[ ... ]

> @@ -4188,6 +4240,39 @@ void __init cpuset_init_smp(void)
>  	BUG_ON(!cpuset_migrate_mm_wq);
>  }
>  
> +/**
> + * cpuset_init_late - initialize the list of CPUs with offline_disabled set
> + *
> + * Description: Initialize a cpumask with CPUs that have the offline_disabled
> + *		bit set. It is done in a separate initcall as cpuset_init_smp()
> + *		is called before driver_init() where the CPU devices will be
> + *		set up.
> + */
> +static int __init cpuset_init_late(void)
> +{
> +	int cpu;
> +
> +	if (!tick_nohz_full_enabled())
> +		return 0;
> +	/*
> +	 * Iterate all the possible CPUs to see which one has offline disabled.
> +	 */
> +	for_each_possible_cpu(cpu) {
> +		if (get_cpu_device(cpu)->offline_disabled)

Can get_cpu_device() return NULL here? Because this is a pure_initcall,
it runs before topology_init() on architectures like PowerPC, MIPS, and
SPARC, meaning the CPU devices might not be registered yet. 

Furthermore, on systems with physical CPU hotplug where possible_cpus exceeds
present_cpus, the non-present possible CPUs might also return NULL. Could
this lead to a NULL pointer dereference during early boot?

> +			__cpumask_set_cpu(cpu, offline_disabled_cpus);
> +	}
> +	if (!cpumask_empty(offline_disabled_cpus)) {
> +		char buf[128];
> +
> +		snprintf(buf, sizeof(buf),
> +			 "CPU %*pbl with offline disabled not allowed in isolated partition",
> +			 cpumask_pr_args(offline_disabled_cpus));

This isn't a bug, but could the 128-byte buffer silently truncate if there
are multiple non-contiguous offline-disabled CPUs? The static text takes up
59 bytes, leaving 68 bytes for the cpumask string. If the formatted string is
longer, the resulting error message surfaced to userspace via perr_strings
might be incomplete.

> +		perr_strings[PERR_OL_DISABLED] = kstrdup(buf, GFP_KERNEL);
> +	}
> +	return 0;
> +}
> +pure_initcall(cpuset_init_late);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260421030351.281436-1-longman@redhat.com?part=22