[PATCH 0/2] x86/x2apic: fix conversion to CPU hotplug state machine

[PATCH 0/2] x86/x2apic: fix conversion to CPU hotplug state machine

[Luiz Capitulino]

The first patch fixes a real reproducible issue. The second one is
more theoretical. Please, check the paches for more details.

Luiz Capitulino (2):
  x86/x2apic: fix NULL pointer def during boot
  x86/x2apic: check return value on probe

 arch/x86/kernel/apic/x2apic_cluster.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.5.5

[PATCH 1/2] x86/x2apic: fix NULL pointer def during boot

[Luiz Capitulino]

Commit 6b2c28471, moved the allocation of cpus_in_cluster
to the x2apic_prepare_cpu() callback. However, it forgot
to move the cpumask_set_cpu() call that uses it.

This generates a NULL pointer dereference during boot
in machines with x2apic_mode=true.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 arch/x86/kernel/apic/x2apic_cluster.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 6368fa6..18b1704 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -165,6 +165,8 @@ int x2apic_prepare_cpu(unsigned int cpu)
 		return -ENOMEM;
 	}
 
+	cpumask_set_cpu(cpu, per_cpu(cpus_in_cluster, cpu));
+
 	return 0;
 }
 
@@ -185,12 +187,9 @@ int x2apic_dead_cpu(unsigned int this_cpu)
 
 static int x2apic_cluster_probe(void)
 {
-	int cpu = smp_processor_id();
-
 	if (!x2apic_mode)
 		return 0;
 
-	cpumask_set_cpu(cpu, per_cpu(cpus_in_cluster, cpu));
 	cpuhp_setup_state(CPUHP_X2APIC_PREPARE, "X2APIC_PREPARE",
 			  x2apic_prepare_cpu, x2apic_dead_cpu);
 	return 1;
-- 
2.5.5

Re: [PATCH 1/2] x86/x2apic: fix NULL pointer def during boot

[Davidlohr Bueso]

On Thu, 04 Aug 2016, Luiz Capitulino wrote:

>Commit 6b2c28471, moved the allocation of cpus_in_cluster
>to the x2apic_prepare_cpu() callback. However, it forgot
>to move the cpumask_set_cpu() call that uses it.
>
>This generates a NULL pointer dereference during boot
>in machines with x2apic_mode=true.
>
>Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>

Tested-by: Davidlohr Bueso <dbueso@suse.de>

Thanks, this at least fixes the issue for me.

[    0.666627] BUG: unable to handle kernel NULL pointer dereference at           (null)
[    0.684468] IP: [<ffffffff81058795>] x2apic_cluster_probe+0x35/0x70
[    0.698613] PGD 0
[    0.703346] Oops: 0002 [#1] SMP
[    0.710504] Modules linked in:
[    0.717543] CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.7.0-53-default+ #1
[    0.732956] Hardware name: Intel Corporation S2600WTT/S2600WTT, BIOS GRNDSDP1.86B.0030.R03.1405061547 05/06/2014
[    0.755841] task: ffff88017e34c040 task.stack: ffff88017e350000
[    0.769143] RIP: 0010:[<ffffffff81058795>]  [<ffffffff81058795>] x2apic_cluster_probe+0x35/0x70
[    0.788837] RSP: 0000:ffff88017e353e30  EFLAGS: 00010202
[    0.800795] RAX: 0000000000000000 RBX: ffffffff81f6f8b8 RCX: ffff88046ec00000
[    0.816787] RDX: 0000000000000000 RSI: 0000000000000002 RDI: 0000000000000246
[    0.832779] RBP: ffff88017e353e30 R08: 00000000000001af R09: ffff8800000bca60
[    0.848770] R10: 00000000000000a0 R11: 0000000000000050 R12: 0000000000002000
[    0.864759] R13: 000000000000a0f8 R14: 000000000000008f R15: 0000000000000090
[    0.880751] FS:  0000000000000000(0000) GS:ffff88046ec00000(0000) knlGS:0000000000000000
[    0.899030] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    0.911951] CR2: 0000000000000000 CR3: 0000000001c06000 CR4: 00000000001406f0
[    0.927944] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[    0.943935] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[    0.959925] Stack:
[    0.964587]  ffff88017e353e48 ffffffff81d9629d 000000000000a0f0 ffff88017e353e80
[    0.981620]  ffffffff81d8fc9a ffffffff81f4d008 ffff88017e34cb80 ffffffff81c96ce0
[    0.998648]  0000000000018480 ffff88017e34c040 ffff88017e353f38 ffffffff81d7b220
[    1.015680] Call Trace:
[    1.021304]  [<ffffffff81d9629d>] default_setup_apic_routing+0x28/0x69
[    1.035952]  [<ffffffff81d8fc9a>] native_smp_prepare_cpus+0x223/0x2d2
[    1.050408]  [<ffffffff81d7b220>] kernel_init_freeable+0xc9/0x226
[    1.064099]  [<ffffffff8163cdfe>] kernel_init+0xe/0x110
[    1.075867]  [<ffffffff8164a1ff>] ret_from_fork+0x1f/0x40
[    1.088015]  [<ffffffff8163cdf0>] ? rest_init+0x80/0x80
[    1.099780] Code: 00 31 c0 65 8b 15 9c 19 fb 7e 85 c9 75 01 c3 48 63 ca 55 48 c7 c0 10 d7 00 00 48 8b 0c cd a0 e6 d3 81 89 d2 48 89 e5 48 8b 04 08 <f0> 48 0f ab 10 49 c7 c0 d0 87 05 81 48 c7 c1 10 86 05 81 ba 01
[    1.146707] RIP  [<ffffffff81058795>] x2apic_cluster_probe+0x35/0x70
[    1.161043]  RSP <ffff88017e353e30>
[    1.168967] CR2: 0000000000000000

[PATCH 1/2] x86/x2apic: fix NULL pointer def during boot

[Igor Mammedov]

Fixes crash at boot for me.

Small nit wrt subj

s/def/deref/

[PATCH 1/2] x86/x2apic: fix NULL pointer def during boot

[Igor Mammedov]

Fixes crash at boot for me.

Small nit wrt subj

s/def/deref/

[PATCH 2/2] x86/x2apic: check return value on probe

[Luiz Capitulino]

cpuhp_setup_state() can fail. If it does, we have to
return 0 to upper layers.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 arch/x86/kernel/apic/x2apic_cluster.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c
index 18b1704..eb4e459 100644
--- a/arch/x86/kernel/apic/x2apic_cluster.c
+++ b/arch/x86/kernel/apic/x2apic_cluster.c
@@ -187,12 +187,14 @@ int x2apic_dead_cpu(unsigned int this_cpu)
 
 static int x2apic_cluster_probe(void)
 {
+	int ret;
+
 	if (!x2apic_mode)
 		return 0;
 
-	cpuhp_setup_state(CPUHP_X2APIC_PREPARE, "X2APIC_PREPARE",
+	ret = cpuhp_setup_state(CPUHP_X2APIC_PREPARE, "X2APIC_PREPARE",
 			  x2apic_prepare_cpu, x2apic_dead_cpu);
-	return 1;
+	return ret < 0 ? 0 : 1;
 }
 
 static const struct cpumask *x2apic_cluster_target_cpus(void)
-- 
2.5.5

Re: [PATCH 0/2] x86/x2apic: fix conversion to CPU hotplug state machine

[Luiz Capitulino]

On Thu,  4 Aug 2016 10:55:58 -0400
Luiz Capitulino <lcapitulino@redhat.com> wrote:

> The first patch fixes a real reproducible issue. The second one is
> more theoretical. Please, check the paches for more details.

Ping?

Just making sure this is not lost. Latest Linus tree doesn't
boot on my machines without patch 1/2.