Catch multiple xnshadow_map attempts

My customer managed to find another hidden door to Xenomai's hell:
Trying to create a Xenomai POSIX thread from within a native thread. A think the other way around would "work" as well. The precise scenario was native thread -> dlopen(libs that's linked against libpthread_rt) -> __init_posix_interface -> __wrap_pthread_setschedparam -> oops. That scenario revealed two more issues in fact.

Here is a patch to remove this hell gate by catching xnshadow_map invocations from withing already mapped shadow thread.

Jan

---
 ChangeLog             |    5 +++++
 ksrc/nucleus/shadow.c |    5 +++++
 2 files changed, 10 insertions(+)

Index: xenomai/ksrc/nucleus/shadow.c
===================================================================

ksrc/nucleus/shadow.c +5 -0

@@ -1334,6 +1334,8 @@ void xnshadow_exit(void)
  * - -EINVAL is returned if the thread control block does not bear the
  * XNSHADOW bit, or if the thread has already been mapped.
  *
+ * - -EBUSY is returned if the current Linux task is already mapped.
+ *
  * Environments:
  *
  * This service can be called from:

@@ -1351,6 +1353,9 @@ int xnshadow_map(xnthread_t *thread, xnc
 	unsigned muxid, magic;
 	int err;
 
+	if (xnshadow_thread(current))
+		return -EBUSY;
+
 	if (!xnthread_test_state(thread, XNSHADOW))
 		return -EINVAL;
 

Index: xenomai/ChangeLog
===================================================================

ChangeLog +5 -0

@@ -1,3 +1,8 @@
+2008-11-21  Jan Kiszka  <[email protected]>
+
+	* ksrc/nucleus/shadow.c (xnshadow_map): Reject already mapped
+	tasks with -EBUSY.
+
 2008-11-16  Philippe Gerum  <[email protected]>
 
 	* ksrc/arch/blackfin/patches: Upgrade to to

Quoted 7 lines · Jan Kiszka

My customer managed to find another hidden door to Xenomai's hell:
Trying to create a Xenomai POSIX thread from within a native thread. A think the other way around would "work" as well. The precise scenario was native thread -> dlopen(libs that's linked against libpthread_rt) -> __init_posix_interface -> __wrap_pthread_setschedparam -> oops. That scenario revealed two more issues in fact.

Quoted 3 lines

Here is a patch to remove this hell gate by catching xnshadow_map invocations from withing already mapped shadow thread.

Any reason why the XNMAPPED predicate did not work in your case?

Quoted 44 lines

Jan

---
 ChangeLog             |    5 +++++
 ksrc/nucleus/shadow.c |    5 +++++
 2 files changed, 10 insertions(+)

Index: xenomai/ksrc/nucleus/shadow.c
===================================================================

ksrc/nucleus/shadow.c +5 -0

@@ -1334,6 +1334,8 @@ void xnshadow_exit(void)
  * - -EINVAL is returned if the thread control block does not bear the
  * XNSHADOW bit, or if the thread has already been mapped.
  *
+ * - -EBUSY is returned if the current Linux task is already mapped.
+ *
  * Environments:
  *
  * This service can be called from:

@@ -1351,6 +1353,9 @@ int xnshadow_map(xnthread_t *thread, xnc
 	unsigned muxid, magic;
 	int err;
 
+	if (xnshadow_thread(current))
+		return -EBUSY;
+
 	if (!xnthread_test_state(thread, XNSHADOW))
 		return -EINVAL;
 

Index: xenomai/ChangeLog
===================================================================

ChangeLog +5 -0

@@ -1,3 +1,8 @@
+2008-11-21  Jan Kiszka  <[email protected]>
+
+	* ksrc/nucleus/shadow.c (xnshadow_map): Reject already mapped
+	tasks with -EBUSY.
+
 2008-11-16  Philippe Gerum  <[email protected]>
 
 	* ksrc/arch/blackfin/patches: Upgrade to to

Quoted 8 lines · Philippe Gerum

Quoted 7 lines · Jan Kiszka

My customer managed to find another hidden door to Xenomai's hell:
Trying to create a Xenomai POSIX thread from within a native thread. A think the other way around would "work" as well. The precise scenario was native thread -> dlopen(libs that's linked against libpthread_rt) -> __init_posix_interface -> __wrap_pthread_setschedparam -> oops. That scenario revealed two more issues in fact.

Quoted 4 lines

Quoted 3 lines

Here is a patch to remove this hell gate by catching xnshadow_map invocations from withing already mapped shadow thread.

Any reason why the XNMAPPED predicate did not work in your case?

Sorry, which one? Keep in mind that the thread thrown at the second xnshadow_map is a virgin one, just allocated from scratch.

Jan

Quoted 44 lines

Quoted 43 lines

Jan

---
 ChangeLog             |    5 +++++
 ksrc/nucleus/shadow.c |    5 +++++
 2 files changed, 10 insertions(+)

Index: xenomai/ksrc/nucleus/shadow.c
===================================================================

ksrc/nucleus/shadow.c +5 -0

@@ -1334,6 +1334,8 @@ void xnshadow_exit(void)
  * - -EINVAL is returned if the thread control block does not bear the
  * XNSHADOW bit, or if the thread has already been mapped.
  *
+ * - -EBUSY is returned if the current Linux task is already mapped.
+ *
  * Environments:
  *
  * This service can be called from:

@@ -1351,6 +1353,9 @@ int xnshadow_map(xnthread_t *thread, xnc
 	unsigned muxid, magic;
 	int err;
 
+	if (xnshadow_thread(current))
+		return -EBUSY;
+
 	if (!xnthread_test_state(thread, XNSHADOW))
 		return -EINVAL;
 

Index: xenomai/ChangeLog
===================================================================

ChangeLog +5 -0

@@ -1,3 +1,8 @@
+2008-11-21  Jan Kiszka  <[email protected]>
+
+	* ksrc/nucleus/shadow.c (xnshadow_map): Reject already mapped
+	tasks with -EBUSY.
+
 2008-11-16  Philippe Gerum  <[email protected]>
 
 	* ksrc/arch/blackfin/patches: Upgrade to to

Quoted 9 lines · Jan Kiszka

Quoted 8 lines · Philippe Gerum

Quoted 7 lines · Jan Kiszka

My customer managed to find another hidden door to Xenomai's hell:
Trying to create a Xenomai POSIX thread from within a native thread. A think the other way around would "work" as well. The precise scenario was native thread -> dlopen(libs that's linked against libpthread_rt) -> __init_posix_interface -> __wrap_pthread_setschedparam -> oops. That scenario revealed two more issues in fact.

Quoted 7 lines

Quoted 4 lines

Quoted 3 lines

Here is a patch to remove this hell gate by catching xnshadow_map invocations from withing already mapped shadow thread.

Any reason why the XNMAPPED predicate did not work in your case?

Sorry, which one? Keep in mind that the thread thrown at the second xnshadow_map is a virgin one, just allocated from scratch.

Ok, got it, it's the converse case, so let's deal for that:

nucleus/shadow.c +1 -1

@@ -1331,7 +1331,7 @@
  * case, the real-time mapping operation has failed globally, and no
  * Xenomai resource remains attached to it.
  *
- * - -EINVAL is returned if the thread control block does not bear the
+ * - -EBUSY is returned if the thread control block does not bear the
  * XNSHADOW bit, or if the thread has already been mapped.
  *
  * Environments:

Quoted 42 lines · Philippe Gerum

Quoted 9 lines · Jan Kiszka

Quoted 8 lines · Philippe Gerum

Quoted 7 lines · Jan Kiszka

My customer managed to find another hidden door to Xenomai's hell:
Trying to create a Xenomai POSIX thread from within a native thread. A think the other way around would "work" as well. The precise scenario was native thread -> dlopen(libs that's linked against libpthread_rt) -> __init_posix_interface -> __wrap_pthread_setschedparam -> oops. That scenario revealed two more issues in fact.

Quoted 7 lines

Quoted 4 lines

Quoted 3 lines

Here is a patch to remove this hell gate by catching xnshadow_map invocations from withing already mapped shadow thread.

Any reason why the XNMAPPED predicate did not work in your case?

Sorry, which one? Keep in mind that the thread thrown at the second xnshadow_map is a virgin one, just allocated from scratch.

Ok, got it, it's the converse case, so let's deal for that:

nucleus/shadow.c +3 -3

@@ -1331,7 +1331,7 @@
  * case, the real-time mapping operation has failed globally, and no
  * Xenomai resource remains attached to it.
  *
- * - -EINVAL is returned if the thread control block does not bear the
+ * - -EBUSY is returned if the thread control block does not bear the
  * XNSHADOW bit, or if the thread has already been mapped.
  *
  * Environments:

@@ -1354,8 +1354,8 @@
 	if (!xnthread_test_state(thread, XNSHADOW))
 		return -EINVAL;
 
-	if (xnthread_test_state(thread, XNMAPPED))
-		return -EINVAL;
+	if (xnshadow_thread(current) || xnthread_test_state(thread, XNMAPPED))
+		return -EBUSY;
 
 	if (!access_wok(u_mode, sizeof(*u_mode)))
 		return -EFAULT;

Quoted 51 lines · Philippe Gerum

Quoted 42 lines · Philippe Gerum

Quoted 9 lines · Jan Kiszka

Quoted 8 lines · Philippe Gerum

Quoted 7 lines · Jan Kiszka

My customer managed to find another hidden door to Xenomai's hell:
Trying to create a Xenomai POSIX thread from within a native thread. A think the other way around would "work" as well. The precise scenario was native thread -> dlopen(libs that's linked against libpthread_rt) -> __init_posix_interface -> __wrap_pthread_setschedparam -> oops. That scenario revealed two more issues in fact.

Quoted 7 lines

Quoted 4 lines

Quoted 3 lines

Here is a patch to remove this hell gate by catching xnshadow_map invocations from withing already mapped shadow thread.

Any reason why the XNMAPPED predicate did not work in your case?

Sorry, which one? Keep in mind that the thread thrown at the second xnshadow_map is a virgin one, just allocated from scratch.

Ok, got it, it's the converse case, so let's deal for that:

nucleus/shadow.c +3 -3

@@ -1331,7 +1331,7 @@
  * case, the real-time mapping operation has failed globally, and no
  * Xenomai resource remains attached to it.
  *
- * - -EINVAL is returned if the thread control block does not bear the
+ * - -EBUSY is returned if the thread control block does not bear the
  * XNSHADOW bit, or if the thread has already been mapped.
  *
  * Environments:

@@ -1354,8 +1354,8 @@
 	if (!xnthread_test_state(thread, XNSHADOW))
 		return -EINVAL;
 
-	if (xnthread_test_state(thread, XNMAPPED))
-		return -EINVAL;
+	if (xnshadow_thread(current) || xnthread_test_state(thread, XNMAPPED))
+		return -EBUSY;
 
 	if (!access_wok(u_mode, sizeof(*u_mode)))
 		return -EFAULT;

nucleus/shadow.c +1 -1

@@ -1332,8 +1332,10 @@
  * Xenomai resource remains attached to it.
  *
  * - -EINVAL is returned if the thread control block does not bear the
- * XNSHADOW bit, or if the thread has already been mapped.
+ * XNSHADOW bit.

nucleus/shadow.c line 1339

Quoted 4 lines

nucleus/shadow.c +2 -0

  *
+ * - -EBUSY is returned if the thread has already been mapped.
+ *
  * Environments:

The code now returns -EBUSY if someone passes down an already touched xnthread object - for me a clear case of EINVAL (the object should be a virgin one).

_Linux_task_ is already mapped (to some other xnthread object). I think my patch is clearer in this respect.

Jan

Quoted 51 lines · Jan Kiszka

Quoted 50 lines · Philippe Gerum

Quoted 42 lines · Philippe Gerum

Quoted 9 lines · Jan Kiszka

Quoted 8 lines · Philippe Gerum

Quoted 7 lines · Jan Kiszka

My customer managed to find another hidden door to Xenomai's hell:
Trying to create a Xenomai POSIX thread from within a native thread. A think the other way around would "work" as well. The precise scenario was native thread -> dlopen(libs that's linked against libpthread_rt) -> __init_posix_interface -> __wrap_pthread_setschedparam -> oops. That scenario revealed two more issues in fact.

Quoted 7 lines

Quoted 4 lines

Quoted 3 lines

Here is a patch to remove this hell gate by catching xnshadow_map invocations from withing already mapped shadow thread.

Any reason why the XNMAPPED predicate did not work in your case?

Sorry, which one? Keep in mind that the thread thrown at the second xnshadow_map is a virgin one, just allocated from scratch.

Ok, got it, it's the converse case, so let's deal for that:

nucleus/shadow.c +3 -3

@@ -1331,7 +1331,7 @@
  * case, the real-time mapping operation has failed globally, and no
  * Xenomai resource remains attached to it.
  *
- * - -EINVAL is returned if the thread control block does not bear the
+ * - -EBUSY is returned if the thread control block does not bear the
  * XNSHADOW bit, or if the thread has already been mapped.
  *
  * Environments:

@@ -1354,8 +1354,8 @@
 	if (!xnthread_test_state(thread, XNSHADOW))
 		return -EINVAL;
 
-	if (xnthread_test_state(thread, XNMAPPED))
-		return -EINVAL;
+	if (xnshadow_thread(current) || xnthread_test_state(thread, XNMAPPED))
+		return -EBUSY;
 
 	if (!access_wok(u_mode, sizeof(*u_mode)))
 		return -EFAULT;

nucleus/shadow.c +0 -1

@@ -1332,8 +1332,10 @@
  * Xenomai resource remains attached to it.
  *
  * - -EINVAL is returned if the thread control block does not bear the
- * XNSHADOW bit, or if the thread has already been mapped.

Quoted 8 lines

nucleus/shadow.c line 1339

Quoted 5 lines

nucleus/shadow.c +3 -0

+ * XNSHADOW bit.
  *
+ * - -EBUSY is returned if the thread has already been mapped.
+ *
  * Environments:

The code now returns -EBUSY if someone passes down an already touched xnthread object - for me a clear case of EINVAL (the object should be a virgin one).

That's precisely a case for EBUSY; someone is attempting to reuse a TCB.

EBUSY should be dedicated to the case that the current

Quoted 3 lines

_Linux_task_ is already mapped (to some other xnthread object). I think my patch is clearer in this respect.

My intent regarding XNMAPPED was precisely to catch that case on the xnthread struct, returning -EINVAL was a bad idea: this did not convey the right information. Your addition completes that test by checking the task_struct side as well. EINVAL should be kept for out-of-bound / badly specified input param; in the XNMAPPED case, the xnthread struct is ok, but in the wrong state. This is what EBUSY is supposed to say.