linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] cpumask: Fix crash on updating CPU enabled mask
@ 2024-08-08  4:08 Gavin Shan
  2024-08-08  8:41 ` Jonathan Cameron
  2024-08-08 17:35 ` Yury Norov
  0 siblings, 2 replies; 3+ messages in thread
From: Gavin Shan @ 2024-08-08  4:08 UTC (permalink / raw)
  To: linux-arm-kernel
  Cc: linux-kernel, yury.norov, linux, Jonathan.Cameron, salil.mehta,
	shan.gavin

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



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] cpumask: Fix crash on updating CPU enabled mask
  2024-08-08  4:08 [PATCH] cpumask: Fix crash on updating CPU enabled mask Gavin Shan
@ 2024-08-08  8:41 ` Jonathan Cameron
  2024-08-08 17:35 ` Yury Norov
  1 sibling, 0 replies; 3+ messages in thread
From: Jonathan Cameron @ 2024-08-08  8:41 UTC (permalink / raw)
  To: Gavin Shan
  Cc: linux-arm-kernel, linux-kernel, yury.norov, linux, salil.mehta,
	shan.gavin

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))



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] cpumask: Fix crash on updating CPU enabled mask
  2024-08-08  4:08 [PATCH] cpumask: Fix crash on updating CPU enabled mask Gavin Shan
  2024-08-08  8:41 ` Jonathan Cameron
@ 2024-08-08 17:35 ` Yury Norov
  1 sibling, 0 replies; 3+ messages in thread
From: Yury Norov @ 2024-08-08 17:35 UTC (permalink / raw)
  To: Gavin Shan
  Cc: linux-arm-kernel, linux-kernel, linux, Jonathan.Cameron,
	salil.mehta, shan.gavin

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



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2024-08-08 17:35 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-08  4:08 [PATCH] cpumask: Fix crash on updating CPU enabled mask Gavin Shan
2024-08-08  8:41 ` Jonathan Cameron
2024-08-08 17:35 ` Yury Norov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).