Re: [Xenomai-core] [BUG] module usage counter of xenomai native corrupted (version 2.2.0 and 2.2.5)

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

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

Jan Kiszka wrote:
> Gilles Chanteperdrix wrote:
> 
>>Gilles Chanteperdrix wrote:
>>
>>>Jan Kiszka wrote:
>>>
>>>
>>>>Thomas Wiedemann wrote:
>>>>
>>>>
>>>>
>>>>>Hi,
>>>>>
>>>>>there seems to be a bug in rt_task_create(). When no more memory is
>>>>>available, the module usage counter of xeno_native is decremented. I
>>>>>guess it is not incremented before, however, so the counter gets 0 and
>>>>>wraps then to a negative number. It is therefore not possible to remove
>>>>>the module.
>>>>>
>>>>>I appended a small program to demonstrate this. It simply eats up all
>>>>>memory from xenomai by registering as much mutexes as possible,
>>>>>and then tries to execute rt_task_create(), which fails. When started
>>>>>again, the bug occurs at rt_task_shadow(), as the mutexes have never
>>>>>been deleted.
>>>>>Compile with  gcc -O2 -Wall `xeno-config --xeno-cflags` `xeno-config
>>>>>--xeno-ldflags` -lrtdm -lnative -o rttest rttest.c
>>>>>then simply run it, and watch the output of lsmod before and after.
>>>>>
>>>>>Tested with xenomai 2.2.{0,5} and linux 2.6.17.8, modules loaded:
>>>>>xeno_native and xeno_nucleus.
>>>>>
>>>>
>>>>Confirmed. Requires a closer look to find the leak path.
>>>
>>>Here is what happens: the task is created with the XNSHADOW bit, and
>>>destroyed before it was xnshadow_mapped, but the deletion hook calls
>>>xnshadow_unmap because the task has the XNSHADOW bit. And xnshadow_unmap
>>> decrements the module count.
>>
>>Here is an untested quick fix.
>>
>>
>>
>>------------------------------------------------------------------------
>>
>>Index: ksrc/nucleus/shadow.c
>>===================================================================
>>--- ksrc/nucleus/shadow.c	(révision 1930)
>>+++ ksrc/nucleus/shadow.c	(copie de travail)
>>@@ -888,6 +888,9 @@
>> 
>> 	p = xnthread_archtcb(thread)->user_task;	/* May be != current */
>> 
>>+	if (!xnshadow_thrptd(p))
>>+		return;
>>+
>> 	magic = xnthread_get_magic(thread);
>> 
>> 	for (muxid = 0; muxid < XENOMAI_MUX_NR; muxid++) {
> 
> 
> Nope, shows unwanted side effects, probably because xnshadow_thrptd is
> already NULL'ed in do_taskexit_event. Looks like it takes an extra flag, no?

Setting xnshadow_thrptd to NULL in do_taskexit_event does not seem to be
 that useful. Here comes version 2.

-- 
                                                 Gilles Chanteperdrix

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: xeno-avoid-unmapping-not-yet-mapped-shadow.2.diff --]
[-- Type: text/x-patch; name="xeno-avoid-unmapping-not-yet-mapped-shadow.2.diff", Size: 728 bytes --]

Index: ksrc/nucleus/shadow.c
===================================================================
--- ksrc/nucleus/shadow.c	(révision 1930)
+++ ksrc/nucleus/shadow.c	(copie de travail)
@@ -888,6 +888,9 @@
 
 	p = xnthread_archtcb(thread)->user_task;	/* May be != current */
 
+	if (!xnshadow_thrptd(p))
+		return;
+
 	magic = xnthread_get_magic(thread);
 
 	for (muxid = 0; muxid < XENOMAI_MUX_NR; muxid++) {
@@ -1639,8 +1642,6 @@
 		xnshadow_relax(0);
 
 	xnlock_get_irqsave(&nklock, s);
-	/* Prevent wakeup call from xnshadow_unmap(). */
-	xnshadow_thrptd(p) = NULL;
 	xnthread_archtcb(thread)->user_task = NULL;
 	/* xnpod_delete_thread() -> hook -> xnshadow_unmap(). */
 	xnpod_delete_thread(thread);