[Xenomai-core] [PATCH] HAL-x86: Fix race in timer request and release path

[Xenomai-core] [PATCH] HAL-x86: Fix race in timer request and release path

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 3044 bytes --]

By taking over a tick device from Linux, we install a set-next-tick
handler for Linux that could be called shortly after, eventually with
such a small delay that it will trigger the Xenomai tick IRQ before we
installed its corresponding handler. This will cause a NULL pointer oops
which has been observed at least once here over a KVM environment (where
race windows tend to widen a lot).

Plug the race by installing the tick IRQ handler first, then starting
to take over tick devices. Vice versa on release.

Signed-off-by: Jan Kiszka <jan.kiszka@domain.hid>
---

 ksrc/arch/x86/hal-common.c |   44 +++++++++++++++++++++++---------------------
 1 files changed, 23 insertions(+), 21 deletions(-)

diff --git a/ksrc/arch/x86/hal-common.c b/ksrc/arch/x86/hal-common.c
index 2b04dcf..4d818ad 100644
--- a/ksrc/arch/x86/hal-common.c
+++ b/ksrc/arch/x86/hal-common.c
@@ -107,18 +107,24 @@ int rthal_timer_request(
 	int cpu)
 {
 	unsigned long tmfreq;
-	int tickval, err;
+	int tickval, err, res;
+
+	if (cpu_timers_requested == 0) {
+		err = rthal_irq_request(RTHAL_APIC_TIMER_IPI,
+					(rthal_irq_handler_t) tick_handler,
+					NULL, NULL);
+		if (err)
+			return err;
+	}
 
 	/* This code works both for UP+LAPIC and SMP configurations. */
 
 #ifdef __IPIPE_FEATURE_REQUEST_TICKDEV
-	int res = ipipe_request_tickdev("lapic", mode_emul, tick_emul, cpu,
-					&tmfreq);
+	res = ipipe_request_tickdev("lapic", mode_emul, tick_emul, cpu,
+				    &tmfreq);
 #else
-	int res = ipipe_request_tickdev("lapic",
-					(compat_emumode_t)mode_emul,
-					(compat_emutick_t)tick_emul,
-					cpu);
+	res = ipipe_request_tickdev("lapic", (compat_emumode_t)mode_emul,
+				    (compat_emutick_t)tick_emul, cpu);
 	tmfreq = RTHAL_COMPAT_TIMERFREQ;
 #endif
 
@@ -141,9 +147,12 @@ int rthal_timer_request(
 		break;
 
 	case CLOCK_EVT_MODE_SHUTDOWN:
-		return -ENODEV;
+		res = -ENODEV;
+		/* fall through */
 
 	default:
+		if (cpu_timers_requested == 0)
+			rthal_irq_release(RTHAL_APIC_TIMER_IPI);
 		return res;
 	}
 	rthal_ktimer_saved_mode = res;
@@ -155,19 +164,12 @@ int rthal_timer_request(
 	 * The rest of the initialization should only be performed
 	 * once by a single CPU.
 	 */
-	if (cpu_timers_requested++ > 0)
-		goto out;
-
-	err = rthal_irq_request(RTHAL_APIC_TIMER_IPI,
-				(rthal_irq_handler_t) tick_handler, NULL, NULL);
-
-	if (err)
-		return err;
+	if (cpu_timers_requested++ == 0) {
+		rthal_timer_set_oneshot(1);
 
-	rthal_timer_set_oneshot(1);
+		rthal_nmi_init(&rthal_latency_above_max);
+	}
 
-	rthal_nmi_init(&rthal_latency_above_max);
-out:
 	return tickval;
 }
 
@@ -270,12 +272,12 @@ void rthal_timer_release(int cpu)
 
 	rthal_nmi_release();
 
-	rthal_irq_release(RTHAL_APIC_TIMER_IPI);
-
 	if (rthal_ktimer_saved_mode == KTIMER_MODE_PERIODIC)
 		rthal_timer_set_periodic();
 	else if (rthal_ktimer_saved_mode == KTIMER_MODE_ONESHOT)
 		rthal_timer_set_oneshot(0);
+
+	rthal_irq_release(RTHAL_APIC_TIMER_IPI);
 }
 
 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]