[Xenomai-core] Race in rpi_clear_remote?

[Jan Kiszka <jan.kiszka@domain.hid>]

Hi,

I'm meditating over an oops in rpi_clear_remote. NULL pointer deref, it
/seems/ like thread->rpi is invalid. Looking at the code, I wonder if
this could explain the bug:


static void rpi_clear_remote(struct xnthread *thread)
{
...
        rpi = thread->rpi;
        if (unlikely(rpi == NULL))
                return;

        xnlock_get_irqsave(&rpi->rpilock, s);

        /*
         * The RPI slot - if present - is always valid, and won't
         * change since the thread is resuming on this CPU and cannot
         * migrate under our feet. We may grab the remote slot lock
         * now.
         */
        xnsched_pop_rpi(thread);
        thread->rpi = NULL;

...

So we deref (xnsched_pop_rpi) and clear thread->rpi under rpilock, but
we check for it without any protection? Sounds racy. I think 'thread' is
not only pointing to the current thread but could refer to a foreign one
as well, right? Don't we need this:

diff --git a/ksrc/nucleus/shadow.c b/ksrc/nucleus/shadow.c
index 872c37f..1f995d6 100644
--- a/ksrc/nucleus/shadow.c
+++ b/ksrc/nucleus/shadow.c
@@ -331,6 +331,12 @@ static void rpi_clear_remote(struct xnthread *thread)
 
 	xnlock_get_irqsave(&rpi->rpilock, s);
 
+	/* Re-check under lock, someone may have cleared rpi by now. */
+	if (unlikely(thread->rpi == NULL)) {
+		xnlock_put_irqrestore(&rpi->rpilock, s);
+		return;
+	}
+
 	/*
 	 * The RPI slot - if present - is always valid, and won't
 	 * change since the thread is resuming on this CPU and cannot


Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux