[Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[M. Koehrer]

[-- Attachment #1.1: Type: text/plain, Size: 2460 bytes --]

Hi all,

I am just trying out the interrupt mechanism of Xenomai.
For that, I have create a kernel module that uses rt_intr_create (from the native skin) to
create an IRQ handler. So far that looks fine.
However, I noticed, that I have to pass a non-NULL argument  name to rt_intr_create().
Otherwise, cat /proc/xenomai/irq crashes with a kernel oops.
I think this is a bug as the API documentation allows the usage of a NULL name in rt_intr_create.
Probably, the zero pointer will not be checked in the proc reading function.
I am using 2.6.19.1 on a Pentium 4 (UP) with Xenomai 2.3.

I have enclosed a minimum kernel module that leads to a kernel oops to see the effect.

Here is the kernel oops:
---------- BEGIN ----------
BUG: unable to handle kernel NULL pointer dereference at virtual address 0000000
0
 printing eip:
*pde = 00000000
Oops: 0000 [#1]
PREEMPT
Modules linked in: irqtest e1000
CPU:    0
EIP:    0060:[<c0144daf>]    Not tainted VLI
EFLAGS: 00010046   (2.6.19.1 #5)
EIP is at xnintr_irq_proc+0x8c/0xcd
eax: d881a02a   ebx: d881a021   ecx: 00000580   edx: e097770c
esi: 00000000   edi: d881a02a   ebp: 00000000   esp: d9443ee4
ds: 007b   es: 007b   ss: 0068
Process cat (pid: 2480, ti=d9442000 task=c15c6030 task.ti=d9442000)
Stack: d881a015 0000000b 00000580 d881a02a c0261afb d881a015 d881a021 0000000b
       00000580 d881a000 c0145836 0000000b d881a021 00000000 d9443fa4 00000400
       d881a000 00000400 c01bced0 d881a000 d9443f50 00000000 00000400 d9443f54
Call Trace:
 [<c0261afb>] sprintf+0x2b/0x2f
 [<c0145836>] irq_read_proc+0x85/0xea
 [<c01bced0>] proc_file_read+0x11c/0x24c
 [<c0185fc7>] vfs_read+0xa0/0x170
 [<c0186407>] sys_read+0x4b/0x71
 [<c0103146>] syscall_call+0x7/0xb
 =======================
Code: 83 e5 01 c1 e1 07 8b 91 8c e5 4c c0 89 de 85 d2 74 34 be 3f 27 3e c0 89 df
 ac aa 84 c0 75 fa 8d 43 09 89 44 24 0c 8b 72 18 89 c7 <ac> aa 84 c0 75 fa 8b 52
 18 31 c0 83 c9 ff 89 d7 f2 ae f7 d1 49
EIP: [<c0144daf>] xnintr_irq_proc+0x8c/0xcd SS:ESP 0068:d9443ee4
--------- END ---------------

Regards

Mathias



-- 
Mathias Koehrer
mathias_koehrer@domain.hid


Viel oder wenig? Schnell oder langsam? Unbegrenzt surfen + telefonieren
ohne Zeit- und Volumenbegrenzung? DAS TOP ANGEBOT JETZT bei Arcor: günstig
und schnell mit DSL - das All-Inclusive-Paket für clevere Doppel-Sparer,
nur  44,85 €  inkl. DSL- und ISDN-Grundgebühr!
http://www.arcor.de/rd/emf-dsl-2

[-- Attachment #2: irqtest.tgz --]
[-- Type: application/octet-stream, Size: 731 bytes --]

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Jan Kiszka]

[-- Attachment #1.1: Type: text/plain, Size: 1072 bytes --]

M. Koehrer wrote:
> Hi all,
> 
> I am just trying out the interrupt mechanism of Xenomai.
> For that, I have create a kernel module that uses rt_intr_create (from the native skin) to
> create an IRQ handler. So far that looks fine.
> However, I noticed, that I have to pass a non-NULL argument  name to rt_intr_create().
> Otherwise, cat /proc/xenomai/irq crashes with a kernel oops.
> I think this is a bug as the API documentation allows the usage of a NULL name in rt_intr_create.
> Probably, the zero pointer will not be checked in the proc reading function.
> I am using 2.6.19.1 on a Pentium 4 (UP) with Xenomai 2.3.
> 
> I have enclosed a minimum kernel module that leads to a kernel oops to see the effect.
> 

Untested, but this patch should fix the issue - also for RTDM drivers
passing a NULL name to rtdm_irq_request (which was not forbidden so far
too). Nevertheless, passing a name is recommended to ease the system
diagnosis. Maybe the native doc should be changed in this regard too (I
already did so for RTDM in 2.3 and trunk).

Jan

[-- Attachment #1.2: xnintr-handle-NULL-name.patch --]
[-- Type: text/plain, Size: 1293 bytes --]

Index: ChangeLog
===================================================================
--- ChangeLog	(revision 2056)
+++ ChangeLog	(working copy)
@@ -1,3 +1,8 @@
+2007-01-10  Jan Kiszka  <jan.kiszka@domain.hid>
+
+	* ksrc/nucleus/intr.c (xnintr_init): Always set xnintr::name to
+	non-NULL content.
+
 2007-01-04  Philippe Gerum  <rpm@xenomai.org>
 
 	* include/nucleus/timebase.h (xntbase_convert): Add time base
Index: ksrc/nucleus/intr.c
===================================================================
--- ksrc/nucleus/intr.c	(revision 2056)
+++ ksrc/nucleus/intr.c	(working copy)
@@ -490,7 +490,7 @@ int xnintr_mount(void) { return 0; }
  * therefore it must be allocated in permanent memory.
  *
  * @param name An ASCII string standing for the symbolic name of the
- * interrupt object.
+ * interrupt object or NULL ("<unknown>" will be applied then).
  *
  * @param irq The hardware interrupt channel associated with the
  * interrupt object. This value is architecture-dependent. An
@@ -541,7 +541,7 @@ int xnintr_init(xnintr_t *intr,
 	intr->isr = isr;
 	intr->iack = iack;
 	intr->cookie = NULL;
-	intr->name = name;
+	intr->name = name ? : "<unknown>";
 	intr->flags = flags;
 	intr->unhandled = 0;
 	memset(&intr->stat, 0, sizeof(intr->stat));

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Dmitry Adamushko]

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

Hi,

I suppose, one more thing was missed. intr_reg.patch is to address it.

intr_swap.patch - to be safe with xnintr_t::name in all cases. In
particular, the reported crash should disappear without other
additional patches.


-- 
Best regards,
Dmitry Adamushko

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: intr_reg.patch --]
[-- Type: text/x-patch; name="intr_reg.patch", Size: 1137 bytes --]

--- ./ksrc/skins/native/intr.c	2006-06-15 14:15:44.000000000 +0200
+++ ./ksrc/skins/native/intr-new.c	2007-01-10 14:45:26.320565000 +0100
@@ -223,6 +223,7 @@ static xnpnode_t __intr_pnode = {
  *
  * - Kernel module initialization/cleanup code
  * - Kernel-based task
+ * - User-space task
  *
  * Rescheduling: possible.
  *
@@ -262,11 +263,21 @@ int rt_intr_create(RT_INTR *intr,
 	/* <!> Since xnregister_enter() may reschedule, only register
 	   complete objects, so that the registry cannot return handles to
 	   half-baked objects... */
+	if (!err && name) {
+		xnpnode_t *pnode = &__intr_pnode;
 
-	if (!err)
-		err =
-		    xnregistry_enter(intr->name, intr, &intr->handle,
-				     &__intr_pnode);
+		if (!*name) {
+			/* Since this is an anonymous object (empty name on entry)
+			 * from user-space, it gets registered under an unique
+			 * internal name but is not exported through /proc. */
+			xnobject_create_name(intr->name, sizeof(intr->name),
+				(void *)intr);
+			pnode = NULL;
+		}
+
+		err = xnregistry_enter(intr->name, intr, &intr->handle, pnode);
+	}	
+	
 #endif /* CONFIG_XENO_OPT_REGISTRY */
 
 	if (err)

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: intr_swap.patch --]
[-- Type: text/x-patch; name="intr_swap.patch", Size: 542 bytes --]

--- ./ksrc/skins/native/intr-new.c	2007-01-10 14:45:26.320565000 +0100
+++ ./ksrc/skins/native/intr-new2.c	2007-01-10 14:46:08.319442000 +0100
@@ -242,8 +242,8 @@ int rt_intr_create(RT_INTR *intr,
 	if (xnpod_asynch_p())
 		return -EPERM;
 
-	xnintr_init(&intr->intr_base, name, irq, isr, iack, mode);
 	xnobject_copy_name(intr->name, name);
+	xnintr_init(&intr->intr_base, intr->name, irq, isr, iack, mode);
 #if defined(__KERNEL__) && defined(CONFIG_XENO_OPT_PERVASIVE)
 	xnsynch_init(&intr->synch_base, XNSYNCH_PRIO);
 	intr->pending = 0;

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Jan Kiszka]

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

Dmitry Adamushko wrote:
> Hi,
> 
> I suppose, one more thing was missed. intr_reg.patch is to address it.
> 
> intr_swap.patch - to be safe with xnintr_t::name in all cases. In
> particular, the reported crash should disappear without other
> additional patches.
> 
> 
> 
> ------------------------------------------------------------------------
> 
> --- ./ksrc/skins/native/intr.c	2006-06-15 14:15:44.000000000 +0200
> +++ ./ksrc/skins/native/intr-new.c	2007-01-10 14:45:26.320565000 +0100
> @@ -223,6 +223,7 @@ static xnpnode_t __intr_pnode = {
>   *
>   * - Kernel module initialization/cleanup code
>   * - Kernel-based task
> + * - User-space task
>   *
>   * Rescheduling: possible.
>   *
> @@ -262,11 +263,21 @@ int rt_intr_create(RT_INTR *intr,
>  	/* <!> Since xnregister_enter() may reschedule, only register
>  	   complete objects, so that the registry cannot return handles to
>  	   half-baked objects... */
> +	if (!err && name) {
> +		xnpnode_t *pnode = &__intr_pnode;
>  
> -	if (!err)
> -		err =
> -		    xnregistry_enter(intr->name, intr, &intr->handle,
> -				     &__intr_pnode);
> +		if (!*name) {
> +			/* Since this is an anonymous object (empty name on entry)

Isn't a NULL name representing an anonymous object as well?

> +			 * from user-space, it gets registered under an unique
> +			 * internal name but is not exported through /proc. */
> +			xnobject_create_name(intr->name, sizeof(intr->name),
> +				(void *)intr);
> +			pnode = NULL;
> +		}
> +
> +		err = xnregistry_enter(intr->name, intr, &intr->handle, pnode);
> +	}	
> +	
>  #endif /* CONFIG_XENO_OPT_REGISTRY */
>  
>  	if (err)
> 
> 
> ------------------------------------------------------------------------
> 
> --- ./ksrc/skins/native/intr-new.c	2007-01-10 14:45:26.320565000 +0100
> +++ ./ksrc/skins/native/intr-new2.c	2007-01-10 14:46:08.319442000 +0100
> @@ -242,8 +242,8 @@ int rt_intr_create(RT_INTR *intr,
>  	if (xnpod_asynch_p())
>  		return -EPERM;
>  
> -	xnintr_init(&intr->intr_base, name, irq, isr, iack, mode);
>  	xnobject_copy_name(intr->name, name);
> +	xnintr_init(&intr->intr_base, intr->name, irq, isr, iack, mode);

This will set xnintr_t::name to "" if name is NULL - intentionally?

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads

[M. Koehrer]

Hi,

one more comment on this issue:
Regarding to the API documentation it is not required to use a static allocated
string for the name parameter
(API docu: "When non-NULL and non-empty, this string is copied to a safe place into the descriptor").

However, when I use a name variable that is created on the stack (using sprintf), I see
that the /proc/xenomai/registry/native/interrupts/xxx  name is fine, however the name in 
/proc/xenomai/irq is not valid.
I.e. I have to use a static name variable to pass to rt_inter_create() as the name pointer
seems to be used without a copy action in /proc/xenomai/irq.


Regards

Mathias
> > Hi,
> > 
> > I suppose, one more thing was missed. intr_reg.patch is to address it.
> > 
> > intr_swap.patch - to be safe with xnintr_t::name in all cases. In
> > particular, the reported crash should disappear without other
> > additional patches.
> > 
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > --- ./ksrc/skins/native/intr.c	2006-06-15 14:15:44.000000000 +0200
> > +++ ./ksrc/skins/native/intr-new.c	2007-01-10 14:45:26.320565000 +0100
> > @@ -223,6 +223,7 @@ static xnpnode_t __intr_pnode = {
> >   *
> >   * - Kernel module initialization/cleanup code
> >   * - Kernel-based task
> > + * - User-space task
> >   *
> >   * Rescheduling: possible.
> >   *
> > @@ -262,11 +263,21 @@ int rt_intr_create(RT_INTR *intr,
> >  	/* <!> Since xnregister_enter() may reschedule, only register
> >  	   complete objects, so that the registry cannot return handles to
> >  	   half-baked objects... */
> > +	if (!err && name) {
> > +		xnpnode_t *pnode = &__intr_pnode;
> >  
> > -	if (!err)
> > -		err =
> > -		    xnregistry_enter(intr->name, intr, &intr->handle,
> > -				     &__intr_pnode);
> > +		if (!*name) {
> > +			/* Since this is an anonymous object (empty name on entry)
> 
> Isn't a NULL name representing an anonymous object as well?
> 
> > +			 * from user-space, it gets registered under an unique
> > +			 * internal name but is not exported through /proc. */
> > +			xnobject_create_name(intr->name, sizeof(intr->name),
> > +				(void *)intr);
> > +			pnode = NULL;
> > +		}
> > +
> > +		err = xnregistry_enter(intr->name, intr, &intr->handle, pnode);
> > +	}	
> > +	
> >  #endif /* CONFIG_XENO_OPT_REGISTRY */
> >  
> >  	if (err)
> > 
> > 
> > ------------------------------------------------------------------------
> > 
> > --- ./ksrc/skins/native/intr-new.c	2007-01-10 14:45:26.320565000 +0100
> > +++ ./ksrc/skins/native/intr-new2.c	2007-01-10 14:46:08.319442000 +0100
> > @@ -242,8 +242,8 @@ int rt_intr_create(RT_INTR *intr,
> >  	if (xnpod_asynch_p())
> >  		return -EPERM;
> >  
> > -	xnintr_init(&intr->intr_base, name, irq, isr, iack, mode);
> >  	xnobject_copy_name(intr->name, name);
> > +	xnintr_init(&intr->intr_base, intr->name, irq, isr, iack, mode);
> 
> This will set xnintr_t::name to "" if name is NULL - intentionally?
> 
> Jan
> 
> 

-- 
Mathias Koehrer
mathias_koehrer@domain.hid


Viel oder wenig? Schnell oder langsam? Unbegrenzt surfen + telefonieren
ohne Zeit- und Volumenbegrenzung? DAS TOP ANGEBOT JETZT bei Arcor: günstig
und schnell mit DSL - das All-Inclusive-Paket für clevere Doppel-Sparer,
nur  44,85 €  inkl. DSL- und ISDN-Grundgebühr!
http://www.arcor.de/rd/emf-dsl-2

Re: Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads

[Dmitry Adamushko]

On 10/01/07, M. Koehrer <mathias_koehrer@domain.hid> wrote:
> Hi,
>
> one more comment on this issue:
> Regarding to the API documentation it is not required to use a static allocated
> string for the name parameter
> (API docu: "When non-NULL and non-empty, this string is copied to a safe place into the descriptor").
>
> However, when I use a name variable that is created on the stack (using sprintf), I see
> that the /proc/xenomai/registry/native/interrupts/xxx  name is fine, however the name in
> /proc/xenomai/irq is not valid.
> I.e. I have to use a static name variable to pass to rt_inter_create() as the name pointer
> seems to be used without a copy action in /proc/xenomai/irq.

You didn't apply the patches I sent, did you?

The second patch (intr_swap.patch) should address this issue.


-- 
Best regards,
Dmitry Adamushko

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Dmitry Adamushko]

> > +     if (!err && name) {
> > +             xnpnode_t *pnode = &__intr_pnode;
> >
> > -     if (!err)
> > -             err =
> > -                 xnregistry_enter(intr->name, intr, &intr->handle,
> > -                                  &__intr_pnode);
> > +             if (!*name) {
> > +                     /* Since this is an anonymous object (empty name on entry)
>
> Isn't a NULL name representing an anonymous object as well?

Anonymous objects are supported only for user-space clients.
We agreed on it with Philippe when similar code was introduced for
other objects. In this case, name is not NULL but "\0" on entry of
native::rt_object_create (object = mutex,event,etc.).

Now, don't ask me why the same is not applicable for the kernel-side
objects as I'll have to think about it (can't recall right now) and
may overheat. Likely, just some political issues. Yes, we want to make
life of kernel-side developers a bit more difficult. Namely, want an
object to be exported via registry? Give a valid name upon its
creation.

> > -     xnintr_init(&intr->intr_base, name, irq, isr, iack, mode);
> >       xnobject_copy_name(intr->name, name);
> > +     xnintr_init(&intr->intr_base, intr->name, irq, isr, iack, mode);
>
> This will set xnintr_t::name to "" if name is NULL - intentionally?

xnintr_t::name becomes a reference to RT_INTR::name. The later one
exists as long as the former anyway so there shoudn't be a broken
reference.

Just in case xnintr::name is used somewhere else, aside of xnintr_irq_proc().

>
> Jan
>


-- 
Best regards,
Dmitry Adamushko

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Dmitry Adamushko]

On 10/01/07, Dmitry Adamushko <dmitry.adamushko@domain.hid> wrote:
> > > +     if (!err && name) {
> > > +             xnpnode_t *pnode = &__intr_pnode;
> > >
> > > -     if (!err)
> > > -             err =
> > > -                 xnregistry_enter(intr->name, intr, &intr->handle,
> > > -                                  &__intr_pnode);
> > > +             if (!*name) {
> > > +                     /* Since this is an anonymous object (empty name on entry)
> >
> > Isn't a NULL name representing an anonymous object as well?
>
> Anonymous objects are supported only for user-space clients.
> We agreed on it with Philippe when similar code was introduced for
> other objects. In this case, name is not NULL but "\0" on entry of
> native::rt_object_create (object = mutex,event,etc.).
>
> Now, don't ask me why the same is not applicable for the kernel-side
> objects as I'll have to think about it (can't recall right now) and
> may overheat. Likely, just some political issues. Yes, we want to make
> life of kernel-side developers a bit more difficult. Namely, want an
> object to be exported via registry? Give a valid name upon its
> creation.
>

Although, intr objects would be an exception. As their names are
visible not only through the register but also through
/proc/xenomai/irq. On the other hand, showing something like 81234560
(an address of the object - if we use xnobject_create_name) instead
wouldn't be of any great avail?
Ah, at least one would see how many "anonymous" kernel-side handlers
are attached to the line.


-- 
Best regards,
Dmitry Adamushko

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Jan Kiszka]

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

Dmitry Adamushko wrote:
>> > +     if (!err && name) {
>> > +             xnpnode_t *pnode = &__intr_pnode;
>> >
>> > -     if (!err)
>> > -             err =
>> > -                 xnregistry_enter(intr->name, intr, &intr->handle,
>> > -                                  &__intr_pnode);
>> > +             if (!*name) {
>> > +                     /* Since this is an anonymous object (empty
>> name on entry)
>>
>> Isn't a NULL name representing an anonymous object as well?
> 
> Anonymous objects are supported only for user-space clients.
> We agreed on it with Philippe when similar code was introduced for
> other objects. In this case, name is not NULL but "\0" on entry of
> native::rt_object_create (object = mutex,event,etc.).

OK, I see.

> 
> Now, don't ask me why the same is not applicable for the kernel-side
> objects as I'll have to think about it (can't recall right now) and
> may overheat. Likely, just some political issues. Yes, we want to make
> life of kernel-side developers a bit more difficult. Namely, want an
> object to be exported via registry? Give a valid name upon its
> creation.
> 
>> > -     xnintr_init(&intr->intr_base, name, irq, isr, iack, mode);
>> >       xnobject_copy_name(intr->name, name);
>> > +     xnintr_init(&intr->intr_base, intr->name, irq, isr, iack, mode);
>>
>> This will set xnintr_t::name to "" if name is NULL - intentionally?
> 
> xnintr_t::name becomes a reference to RT_INTR::name. The later one
> exists as long as the former anyway so there shoudn't be a broken
> reference.

Yes, this specifically addresses Mathias' concern about passed names
that are temporary variables.

It just leaves xnintr_t::name with an emptry string instead of "unknown"
in some cases. Either xnintr_init needs to take care of this, or we
should catch that case already here. I think empty strings in
/proc/xenomai/irq are bad in case of shared IRQ lists.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Dmitry Adamushko]

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

> >>
> >> This will set xnintr_t::name to "" if name is NULL - intentionally?
> >
> > xnintr_t::name becomes a reference to RT_INTR::name. The later one
> > exists as long as the former anyway so there shoudn't be a broken
> > reference.
>
> Yes, this specifically addresses Mathias' concern about passed names
> that are temporary variables.
>
> It just leaves xnintr_t::name with an emptry string instead of "unknown"
> in some cases. Either xnintr_init needs to take care of this, or we
> should catch that case already here. I think empty strings in
> /proc/xenomai/irq are bad in case of shared IRQ lists.
>

Yes, this is what I've mentioned as a possible exception in my previous mail.

This patch should address it (although, it makes a code flow a bit
obscure but not too much :)


> Jan
>


-- 
Best regards,
Dmitry Adamushko

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: intr_3.patch --]
[-- Type: text/x-patch; name="intr_3.patch", Size: 737 bytes --]

--- ./ksrc/skins/native/intr-new2.c	2007-01-10 14:46:08.319442000 +0100
+++ ./ksrc/skins/native/intr-new3.c	2007-01-10 15:51:22.933976000 +0100
@@ -242,7 +242,14 @@ int rt_intr_create(RT_INTR *intr,
 	if (xnpod_asynch_p())
 		return -EPERM;
 
-	xnobject_copy_name(intr->name, name);
+	if (name)
+		xnobject_copy_name(intr->name, name);
+	else
+		/* Kernel-side "anonymous" objects (name == NULL) get unique names.
+		 * Nevertheless, they will not be exported via the registry. */
+		xnobject_create_name(intr->name, sizeof(intr->name),
+			(void *)intr);
+
 	xnintr_init(&intr->intr_base, intr->name, irq, isr, iack, mode);
 #if defined(__KERNEL__) && defined(CONFIG_XENO_OPT_PERVASIVE)
 	xnsynch_init(&intr->synch_base, XNSYNCH_PRIO);

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Jan Kiszka]

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

Dmitry Adamushko wrote:
>> >>
>> >> This will set xnintr_t::name to "" if name is NULL - intentionally?
>> >
>> > xnintr_t::name becomes a reference to RT_INTR::name. The later one
>> > exists as long as the former anyway so there shoudn't be a broken
>> > reference.
>>
>> Yes, this specifically addresses Mathias' concern about passed names
>> that are temporary variables.
>>
>> It just leaves xnintr_t::name with an emptry string instead of "unknown"
>> in some cases. Either xnintr_init needs to take care of this, or we
>> should catch that case already here. I think empty strings in
>> /proc/xenomai/irq are bad in case of shared IRQ lists.
>>
> 
> Yes, this is what I've mentioned as a possible exception in my previous
> mail.
> 
> This patch should address it (although, it makes a code flow a bit
> obscure but not too much :)
> 

I'm fine with it.

Mind bundling all the snippets to one complete patch so that people
don't have to collect the pieces?

Thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Dmitry Adamushko]

> I'm fine with it.
>
> Mind bundling all the snippets to one complete patch so that people
> don't have to collect the pieces?

Attached. Keep in mind that although it's a simple patch I,
personally, haven't tested it (even compilation-wise).
Anyone who would like to give it a try (Mathias?) is wellcome.


>
> Thanks,
> Jan
>

-- 
Best regards,
Dmitry Adamushko

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Dmitry Adamushko]

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

>
> Attached. Keep in mind that although it's a simple patch I,
> personally, haven't tested it (even compilation-wise).
> Anyone who would like to give it a try (Mathias?) is wellcome.

Now finally attached.

-- 
Best regards,
Dmitry Adamushko

[-- Attachment #2: intr.patch --]
[-- Type: text/x-patch, Size: 1813 bytes --]

--- ./ksrc/skins/native/intr.c	2006-06-15 14:15:44.000000000 +0200
+++ ./ksrc/skins/native/intr-new3.c	2007-01-10 15:51:22.933976000 +0100
@@ -223,6 +223,7 @@ static xnpnode_t __intr_pnode = {
  *
  * - Kernel module initialization/cleanup code
  * - Kernel-based task
+ * - User-space task
  *
  * Rescheduling: possible.
  *
@@ -241,8 +242,15 @@ int rt_intr_create(RT_INTR *intr,
 	if (xnpod_asynch_p())
 		return -EPERM;
 
-	xnintr_init(&intr->intr_base, name, irq, isr, iack, mode);
-	xnobject_copy_name(intr->name, name);
+	if (name)
+		xnobject_copy_name(intr->name, name);
+	else
+		/* Kernel-side "anonymous" objects (name == NULL) get unique names.
+		 * Nevertheless, they will not be exported via the registry. */
+		xnobject_create_name(intr->name, sizeof(intr->name),
+			(void *)intr);
+
+	xnintr_init(&intr->intr_base, intr->name, irq, isr, iack, mode);
 #if defined(__KERNEL__) && defined(CONFIG_XENO_OPT_PERVASIVE)
 	xnsynch_init(&intr->synch_base, XNSYNCH_PRIO);
 	intr->pending = 0;
@@ -262,11 +270,21 @@ int rt_intr_create(RT_INTR *intr,
 	/* <!> Since xnregister_enter() may reschedule, only register
 	   complete objects, so that the registry cannot return handles to
 	   half-baked objects... */
+	if (!err && name) {
+		xnpnode_t *pnode = &__intr_pnode;
 
-	if (!err)
-		err =
-		    xnregistry_enter(intr->name, intr, &intr->handle,
-				     &__intr_pnode);
+		if (!*name) {
+			/* Since this is an anonymous object (empty name on entry)
+			 * from user-space, it gets registered under an unique
+			 * internal name but is not exported through /proc. */
+			xnobject_create_name(intr->name, sizeof(intr->name),
+				(void *)intr);
+			pnode = NULL;
+		}
+
+		err = xnregistry_enter(intr->name, intr, &intr->handle, pnode);
+	}	
+	
 #endif /* CONFIG_XENO_OPT_REGISTRY */
 
 	if (err)

Re: Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[M. Koehrer]

Hi Dmitry,

I have tested the patch.
Compiling works fine.

Then, I ran the test I have sent to the list.
Now, cat /proc/xenomai/irq works.
I get the output:
--------------
IRQ         CPU0
 11:           0         e0977700
216:         174         [timer]
226:           0         [virtual]
--------------
which seems to be ok.

Then I modified my kernel module to pass a string that is build on the stack.
This works fine as well.

I.e. everything is fixed! Great work.
Thanks for the fast response time!

Best regards

Mathias
> >
> > Attached. Keep in mind that although it's a simple patch I,
> > personally, haven't tested it (even compilation-wise).
> > Anyone who would like to give it a try (Mathias?) is wellcome.
> 
> Now finally attached.
> 
> -- 
> Best regards,
> Dmitry Adamushko
> 

-- 
Mathias Koehrer
mathias_koehrer@domain.hid


Viel oder wenig? Schnell oder langsam? Unbegrenzt surfen + telefonieren
ohne Zeit- und Volumenbegrenzung? DAS TOP ANGEBOT JETZT bei Arcor: günstig
und schnell mit DSL - das All-Inclusive-Paket für clevere Doppel-Sparer,
nur  44,85 €  inkl. DSL- und ISDN-Grundgebühr!
http://www.arcor.de/rd/emf-dsl-2

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Philippe Gerum]

On Wed, 2007-01-10 at 16:59 +0100, Dmitry Adamushko wrote:
> >
> > Attached. Keep in mind that although it's a simple patch I,
> > personally, haven't tested it (even compilation-wise).
> > Anyone who would like to give it a try (Mathias?) is wellcome.
> 
> Now finally attached.
> 

Merged, thanks.

> _______________________________________________
> Xenomai-help mailing list
> Xenomai-help@domain.hid
> https://mail.gna.org/listinfo/xenomai-help
-- 
Philippe.

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Jan Kiszka]

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

Philippe Gerum wrote:
> On Wed, 2007-01-10 at 16:59 +0100, Dmitry Adamushko wrote:
>>> Attached. Keep in mind that although it's a simple patch I,
>>> personally, haven't tested it (even compilation-wise).
>>> Anyone who would like to give it a try (Mathias?) is wellcome.
>> Now finally attached.
>>
> 
> Merged, thanks.

Please also apply my patch to cover RTDM drivers passing NULL (though
this is discouraged -- now).

Thanks,
Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 250 bytes --]

Re: [Xenomai-help] Xenomai Bug: rt_intr_create with NULL-name leads to Kernel oops in /proc/xenomai/irq

[Philippe Gerum]

On Thu, 2007-01-11 at 21:49 +0100, Jan Kiszka wrote:
> Philippe Gerum wrote:
> > On Wed, 2007-01-10 at 16:59 +0100, Dmitry Adamushko wrote:
> >>> Attached. Keep in mind that although it's a simple patch I,
> >>> personally, haven't tested it (even compilation-wise).
> >>> Anyone who would like to give it a try (Mathias?) is wellcome.
> >> Now finally attached.
> >>
> > 
> > Merged, thanks.
> 
> Please also apply my patch to cover RTDM drivers passing NULL (though
> this is discouraged -- now).
> 

Ok, merged. Thanks.

> Thanks,
> Jan
> 
-- 
Philippe.