Re: [PATCH] hw/intc: Fix incorrect calculation of core in liointc_read() and liointc_write()

[Philippe Mathieu-Daudé]

On 11/3/20 3:05 PM, AlexChen wrote:
> On 2020/11/3 17:53, Jiaxun Yang wrote:
>>
>>
>> 閸︼拷 2020/11/3 17:32, AlexChen 閸愭瑩浜�:
>>> According to the loongson spec
>>> (http://www.loongson.cn/uploadfile/cpu/3B1500/Loongson_3B1500_cpu_user_1.pdf)
>>> and the macro definition(#define R_PERCORE_ISR(x) (0x40 + 0x8 * x)), we know
>>> that the ISR size of per CORE is 8, so here we need to divide
>>> (addr - R_PERCORE_ISR(0)) by 8, not 4.
>> Hi Alex
>>
>> Thanks!
>>
>> That was my fault.. Per Core ISA is rarely used by kernel..

No board in QEMU use this device. So we are safe =)

>>
>> Reviewed-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
>>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Btw:
>> How can you discover this by robot?
>> Huawei owns real artifical intelligence technology lol :-閿涳拷
>>
>>
> 
> Thanks for your review.
> EulerRobot is a virtualization software quality automation project that
> integrates some tools and test suites such as gcc/clang make test, qemu ut,
> qtest, coccinelle scripts and avocado-vt.
> The code checking tool found there was a potential array out of bounds at
> 'r = p->per_core_isr[core]', since 'core' may be 7 which is bigger than
> 'per_core_isr' array size 3.
> So we found this bug.
> 
> Thanks,
> Alex
> 
>> - Jiaxun
>>> Signed-off-by: Alex Chen <alex.chen@huawei.com>
>>> ---
>>>   hw/intc/loongson_liointc.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/hw/intc/loongson_liointc.c b/hw/intc/loongson_liointc.c
>>> index 30fb375b72..fbbfb57ee9 100644
>>> --- a/hw/intc/loongson_liointc.c
>>> +++ b/hw/intc/loongson_liointc.c
>>> @@ -130,7 +130,7 @@ liointc_read(void *opaque, hwaddr addr, unsigned int 
>>> size)
>>>
>>>       if (addr >= R_PERCORE_ISR(0) &&
>>>           addr < R_PERCORE_ISR(NUM_CORES)) {
>>> -        int core = (addr - R_PERCORE_ISR(0)) / 4;
>>> +        int core = (addr - R_PERCORE_ISR(0)) / 8;
>>>           r = p->per_core_isr[core];
>>>           goto out;
>>>       }
>>> @@ -173,7 +173,7 @@ liointc_write(void *opaque, hwaddr addr,
>>>
>>>       if (addr >= R_PERCORE_ISR(0) &&
>>>           addr < R_PERCORE_ISR(NUM_CORES)) {
>>> -        int core = (addr - R_PERCORE_ISR(0)) / 4;
>>> +        int core = (addr - R_PERCORE_ISR(0)) / 8;
>>>           p->per_core_isr[core] = value;
>>>           goto out;
>>>       }
>> .
>>
> 
>