Re: [Xenomai-core] [PATCH] Catch multiple xnshadow_map attempts

2008-11-21 Thread Philippe Gerum
Jan Kiszka wrote:
 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.


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

 Jan
 
 ---
  ChangeLog |5 +
  ksrc/nucleus/shadow.c |5 +
  2 files changed, 10 insertions(+)
 
 Index: xenomai/ksrc/nucleus/shadow.c
 ===
 --- xenomai.orig/ksrc/nucleus/shadow.c
 +++ xenomai/ksrc/nucleus/shadow.c
 @@ -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
 ===
 --- xenomai.orig/ChangeLog
 +++ xenomai/ChangeLog
 @@ -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


-- 
Philippe.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] Catch multiple xnshadow_map attempts

2008-11-21 Thread Jan Kiszka
Philippe Gerum wrote:
 Jan Kiszka wrote:
 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.

 
 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

 
 Jan

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

 Index: xenomai/ksrc/nucleus/shadow.c
 ===
 --- xenomai.orig/ksrc/nucleus/shadow.c
 +++ xenomai/ksrc/nucleus/shadow.c
 @@ -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
 ===
 --- xenomai.orig/ChangeLog
 +++ xenomai/ChangeLog
 @@ -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
 
 


-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] Catch multiple xnshadow_map attempts

2008-11-21 Thread Philippe Gerum
Jan Kiszka wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 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.

 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:

--- ksrc/nucleus/shadow.c   (revision 4411)
+++ ksrc/nucleus/shadow.c   (working copy)
@@ -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;

 Jan
 
 Jan

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

 Index: xenomai/ksrc/nucleus/shadow.c
 ===
 --- xenomai.orig/ksrc/nucleus/shadow.c
 +++ xenomai/ksrc/nucleus/shadow.c
 @@ -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
 ===
 --- xenomai.orig/ChangeLog
 +++ xenomai/ChangeLog
 @@ -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

 
 


-- 
Philippe.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] Catch multiple xnshadow_map attempts

2008-11-21 Thread Philippe Gerum
Philippe Gerum wrote:
 Jan Kiszka wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 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.

 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:
 
 --- ksrc/nucleus/shadow.c (revision 4411)
 +++ ksrc/nucleus/shadow.c (working copy)
 @@ -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;
 

--- ksrc/nucleus/shadow.c   (revision 4411)
+++ ksrc/nucleus/shadow.c   (working copy)
@@ -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.
  *
+ * - -EBUSY is returned if the thread has already been mapped.
+ *
  * Environments:

-- 
Philippe.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] Catch multiple xnshadow_map attempts

2008-11-21 Thread Jan Kiszka
Philippe Gerum wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 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.

 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:

 --- ksrc/nucleus/shadow.c(revision 4411)
 +++ ksrc/nucleus/shadow.c(working copy)
 @@ -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;

 
 --- ksrc/nucleus/shadow.c (revision 4411)
 +++ ksrc/nucleus/shadow.c (working copy)
 @@ -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.
   *
 + * - -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). EBUSY should be dedicated to the case that the current
_Linux_task_ is already mapped (to some other xnthread object). I think
my patch is clearer in this respect.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] Catch multiple xnshadow_map attempts

2008-11-21 Thread Philippe Gerum
Jan Kiszka wrote:
 Philippe Gerum wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 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.

 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:

 --- ksrc/nucleus/shadow.c   (revision 4411)
 +++ ksrc/nucleus/shadow.c   (working copy)
 @@ -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;

 --- ksrc/nucleus/shadow.c(revision 4411)
 +++ ksrc/nucleus/shadow.c(working copy)
 @@ -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.
   *
 + * - -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
 _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.

 Jan
 


-- 
Philippe.

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core


Re: [Xenomai-core] [PATCH] Catch multiple xnshadow_map attempts

2008-11-21 Thread Jan Kiszka
Philippe Gerum wrote:
 Jan Kiszka wrote:
 Philippe Gerum wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 Philippe Gerum wrote:
 Jan Kiszka wrote:
 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.

 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:

 --- ksrc/nucleus/shadow.c  (revision 4411)
 +++ ksrc/nucleus/shadow.c  (working copy)
 @@ -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;

 --- ksrc/nucleus/shadow.c   (revision 4411)
 +++ ksrc/nucleus/shadow.c   (working copy)
 @@ -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.
   *
 + * - -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
 _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.

Well, OK. But then clarify in the doc that EBUSY is about both the
thread structure and the current Linux task.

Jan

-- 
Siemens AG, Corporate Technology, CT SE 2 ES-OS
Corporate Competence Center Embedded Linux

___
Xenomai-core mailing list
Xenomai-core@gna.org
https://mail.gna.org/listinfo/xenomai-core