| Message ID | 20240808040808.647316-1-gshan@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | cpumask: Fix crash on updating CPU enabled mask | expand |
On Thu, 8 Aug 2024 14:08:08 +1000 Gavin Shan <gshan@redhat.com> 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@redhat.com> 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@canb.auug.org.au/ 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@huawei.com> > --- > 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))
On Thu, Aug 08, 2024 at 02:08:08PM +1000, Gavin Shan 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@redhat.com> > --- > 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)) > -- > 2.45.2 Thanks Gavin. I'll send a pull request withing this week. Thanks, Yury
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))
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@redhat.com> --- include/linux/cpumask.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)