Jan Kiszka wrote:
> Philippe Gerum wrote:
>> Philippe Gerum wrote:
>>> Atsushi-san,
>>>
>>> Atsushi Katagiri wrote:
>>>> Yes, I actually encountered this bug and my Linux was crashed by NULL 
>>>> pointer dereference.
>>>>
>>>> I think this is a very simple bug.
>>>> It happens "everytime" we open /proc/xenomai/stat,
>>>> because the last iter->nentries++; (line 466) surely reaches the value of 
>>>> the count,
>>>> and the next iteration, line 449, surely overwrites zero on out of the 
>>>> kmalloced area.
>>>>
>>> Please try this fix instead:
>>>
>> Actually, this one is better:
>>
>> --- ksrc/nucleus/module.c    (revision 4074)
>> +++ ksrc/nucleus/module.c    (working copy)
>> @@ -440,13 +440,13 @@
>>      /* Iterate over all IRQ numbers, ... */
>>      for (irq = 0; irq < XNARCH_NR_IRQS; irq++) {
>>              xnintr_t *prev = NULL;
>> -            int cpu = 0;
>> +            int cpu = 0, _cpu;
>>              int err;
>>
>>              /* ...over all shared IRQs on all CPUs */
>>              while (1) {
>>                      stat_info = &iter->stat_info[iter->nentries];
>> -                    stat_info->cpu = cpu;
>> +                    _cpu = cpu;
>>
>>                      err = xnintr_query(irq, &cpu, &prev, intr_rev,
>>                                         stat_info->name,
>> @@ -458,6 +458,7 @@
>>                      if (err)
>>                              break; /* line unused or end of chain */
>>
>> +                    stat_info->cpu = _cpu;
>>                      stat_info->pid = 0;
>>                      stat_info->state =  0;
>>                      stat_info->ssw = 0;
>>
> 
> Good catch. But for the sake of telling variable names I would suggest:
> 
> Index: ksrc/nucleus/module.c
> ===================================================================
> --- ksrc/nucleus/module.c     (Revision 4094)
> +++ ksrc/nucleus/module.c     (Arbeitskopie)
> @@ -444,15 +444,15 @@ static int stat_seq_open(struct inode *i
>       /* Iterate over all IRQ numbers, ... */
>       for (irq = 0; irq < XNARCH_NR_IRQS; irq++) {
>               xnintr_t *prev = NULL;
> -             int cpu = 0, _cpu;
> -             int err;
> +             int next_cpu = 0;
> +             int cpu, err;
>  
>               /* ...over all shared IRQs on all CPUs */
>               while (1) {
>                       stat_info = &iter->stat_info[iter->nentries];
> -                     _cpu = cpu;
> +                     cpu = next_cpu;
>  
> -                     err = xnintr_query(irq, &cpu, &prev, intr_rev,
> +                     err = xnintr_query(irq, &next_cpu, &prev, intr_rev,
>                                          stat_info->name,
>                                          &stat_info->csw,
>                                          &stat_info->exectime,
> @@ -462,7 +462,7 @@ static int stat_seq_open(struct inode *i
>                       if (err)
>                               break; /* line unused or end of chain */
>  
> -                     stat_info->cpu = _cpu;
> +                     stat_info->cpu = cpu;
>                       stat_info->pid = 0;
>                       stat_info->state =  0;
>                       stat_info->ssw = 0;
> 

This would be even better with a proper iterator construct.

-- 
Philippe.

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

Reply via email to