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
[email protected]
https://mail.gna.org/listinfo/xenomai-core
