Re: [Xenomai-core] [Xenomai-git] Jan Kiszka : nucleus: Fix race between gatekeeper and thread deletion

[Gilles Chanteperdrix <gilles.chanteperdrix@xenomai.org>]

On 07/11/2011 10:06 PM, Jan Kiszka wrote:
> On 2011-07-11 22:02, Gilles Chanteperdrix wrote:
>> On 07/11/2011 09:59 PM, Jan Kiszka wrote:
>>> On 2011-07-11 21:51, Gilles Chanteperdrix wrote:
>>>> On 07/11/2011 09:16 PM, Jan Kiszka wrote:
>>>>> On 2011-07-11 21:10, Jan Kiszka wrote:
>>>>>> On 2011-07-11 20:53, Gilles Chanteperdrix wrote:
>>>>>>> On 07/08/2011 06:29 PM, GIT version control wrote:
>>>>>>>> @@ -2528,6 +2534,22 @@ static inline void do_taskexit_event(struct task_struct *p)
>>>>>>>>  	magic = xnthread_get_magic(thread);
>>>>>>>>  
>>>>>>>>  	xnlock_get_irqsave(&nklock, s);
>>>>>>>> +
>>>>>>>> +	gksched = thread->gksched;
>>>>>>>> +	if (gksched) {
>>>>>>>> +		xnlock_put_irqrestore(&nklock, s);
>>>>>>>
>>>>>>> Are we sure irqs are on here? Are you sure that what is needed is not an
>>>>>>> xnlock_clear_irqon?
>>>>>>
>>>>>> We are in the context of do_exit. Not only IRQs are on, also preemption.
>>>>>> And surely no nklock is held.
>>>>>>
>>>>>>> Furthermore, I do not understand how we
>>>>>>> "synchronize" with the gatekeeper, how is the gatekeeper garanteed to
>>>>>>> wait for this assignment?
>>>>>>
>>>>>> The gatekeeper holds the gksync token while it's active. We request it,
>>>>>> thus we wait for the gatekeeper to become idle again. While it is idle,
>>>>>> we reset the queued reference - but I just realized that this may tramp
>>>>>> on other tasks' values. I need to add a check that the value to be
>>>>>> null'ified is actually still ours.
>>>>>
>>>>> Thinking again, that's actually not a problem: gktarget is only needed
>>>>> while gksync is zero - but then we won't get hold of it anyway and,
>>>>> thus, can't cause any damage.
>>>>
>>>> Well, you make it look like it does not work. From what I understand,
>>>> what you want is to set gktarget to null if a task being hardened is
>>>> destroyed. But by waiting for the semaphore, you actually wait for the
>>>> harden to be complete, so setting to NULL is useless. Or am I missing
>>>> something else?
>>>
>>> Setting to NULL is probably unneeded but still better than rely on the
>>> gatekeeper never waking up spuriously and then dereferencing a stale
>>> pointer.
>>>
>>> The key element of this fix is waitng on gksync, thus on the completion
>>> of the non-RT part of the hardening. Actually, this part usually fails
>>> as the target task received a termination signal at this point.
>>
>> Yes, but since you wait on the completion of the hardening, the test
>> if (target &&...) in the gatekeeper code will always be true, because at
>> this point the cleanup code will still be waiting for the semaphore.
> 
> Yes, except we will ever wake up the gatekeeper later on without an
> updated gktarget, ie. spuriously. Better safe than sorry, this is hairy
> code anyway (hopefully obsolete one day).

The gatekeeper is not woken up by posting the semaphore, the gatekeeper
is woken up by the thread which is going to be hardened (and this thread
is the one which waits for the semaphore).

> 
> Jan
> 


-- 
                                                                Gilles.