Re: [PATCH] cpumask: Fix crash on updating CPU enabled mask

[Jonathan Cameron]

On Thu,  8 Aug 2024 14:08:08 +1000
Gavin Shan <gshan@xxxxxxxxxx> wrote:

> The CPU enabled mask instead of the CPU possible mask should be used
> by set_cpu_enabled(). Otherwise, we run into crash due to write to
> the read-only CPU possible mask when vCPU is hot added on ARM64.
> 
>   (qemu) device_add host-arm-cpu,id=cpu1,socket-id=1
>   Unable to handle kernel write to read-only memory at virtual address ffff800080fa7190
>     :
>   Call trace:
>     register_cpu+0x1a4/0x2e8
>     arch_register_cpu+0x84/0xd8
>     acpi_processor_add+0x480/0x5b0
>     acpi_bus_attach+0x1c4/0x300
>     acpi_dev_for_one_check+0x3c/0x50
>     device_for_each_child+0x68/0xc8
>     acpi_dev_for_each_child+0x48/0x80
>     acpi_bus_attach+0x84/0x300
>     acpi_bus_scan+0x74/0x220
>     acpi_scan_rescan_bus+0x54/0x88
>     acpi_device_hotplug+0x208/0x478
>     acpi_hotplug_work_fn+0x2c/0x50
>     process_one_work+0x15c/0x3c0
>     worker_thread+0x2ec/0x400
>     kthread+0x120/0x130
>     ret_from_fork+0x10/0x20
> 
> Fix it by passing the CPU enabled mask instead of the CPU possible
> mask to set_cpu_enabled().
> 
> Fixes: 51c4767503d5 ("Merge tag 'bitmap-6.11-rc1' of https://github.com:/norov/linux";)
> Signed-off-by: Gavin Shan <gshan@xxxxxxxxxx>
Thanks Gavin!

We'd flagged the merge conflict but thought we'd chase it with a patch adding
set_cpu_enabled() as you have it fixed below.

I completely missed that Linus took a different path whilst resolving the conflict
to https://lore.kernel.org/all/20240701175051.0ef5d901@xxxxxxxxxxxxxxxx/
which was what Stephen did for linux-next (which would have needed a follow up to
use the new infrastructure.

Anyhow, all's well that ends well.

Reviewed-by: Jonathan Cameron <Jonathan.Cameron@xxxxxxxxxx>

> ---
>  include/linux/cpumask.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
> index 801a7e524113..53158de44b83 100644
> --- a/include/linux/cpumask.h
> +++ b/include/linux/cpumask.h
> @@ -1037,7 +1037,7 @@ void init_cpu_online(const struct cpumask *src);
>  	assign_bit(cpumask_check(cpu), cpumask_bits(mask), (val))
>  
>  #define set_cpu_possible(cpu, possible)	assign_cpu((cpu), &__cpu_possible_mask, (possible))
> -#define set_cpu_enabled(cpu, enabled)	assign_cpu((cpu), &__cpu_possible_mask, (enabled))
> +#define set_cpu_enabled(cpu, enabled)	assign_cpu((cpu), &__cpu_enabled_mask, (enabled))
>  #define set_cpu_present(cpu, present)	assign_cpu((cpu), &__cpu_present_mask, (present))
>  #define set_cpu_active(cpu, active)	assign_cpu((cpu), &__cpu_active_mask, (active))
>  #define set_cpu_dying(cpu, dying)	assign_cpu((cpu), &__cpu_dying_mask, (dying))