linux-kernel - Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <97d33f58-6a15-3a94-f9c0-42ebd48cf562@loongson.cn>
Date:   Wed, 3 Aug 2022 19:58:54 +0800
From:   Jianmin Lv <lvjianmin@...ngson.cn>
To:     Huacai Chen <chenhuacai@...il.com>, Marc Zyngier <maz@...nel.org>
Cc:     Huacai Chen <chenhuacai@...ngson.cn>,
        Thomas Gleixner <tglx@...utronix.de>,
        loongarch@...ts.linux.dev, LKML <linux-kernel@...r.kernel.org>,
        Xuefeng Li <lixuefeng@...ngson.cn>,
        Jiaxun Yang <jiaxun.yang@...goat.com>
Subject: Re: [PATCH 1/2] irqchip/loongson-eiointc: Check hwirq overflow



On 2022/8/3 下午3:33, Huacai Chen wrote:
> Hi, Jianmin,
> 
> On Wed, Aug 3, 2022 at 3:20 PM Marc Zyngier <maz@...nel.org> wrote:
>>
>> On Wed, 03 Aug 2022 05:27:27 +0100,
>> Huacai Chen <chenhuacai@...ngson.cn> wrote:
>>>
>>> Check hwirq overflow when allocate irq in eiointc domain.
>>>
>>> Signed-off-by: Huacai Chen <chenhuacai@...ngson.cn>
>>> ---
>>>   drivers/irqchip/irq-loongson-eiointc.c | 7 +++++--
>>>   1 file changed, 5 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/irqchip/irq-loongson-eiointc.c b/drivers/irqchip/irq-loongson-eiointc.c
>>> index 80d8ca6f2d46..f8060e58ee06 100644
>>> --- a/drivers/irqchip/irq-loongson-eiointc.c
>>> +++ b/drivers/irqchip/irq-loongson-eiointc.c
>>> @@ -241,8 +241,11 @@ static int eiointc_domain_alloc(struct irq_domain *domain, unsigned int virq,
>>>        struct eiointc *priv = domain->host_data;
>>>
>>>        ret = irq_domain_translate_onecell(domain, arg, &hwirq, &type);
>>> -     if (ret)
>>> -             return ret;
>>> +     if (ret < 0)
>>> +             return -EINVAL;
>>> +
>>> +     if (hwirq >= IOCSR_EXTIOI_VECTOR_NUM)
>>> +             return -EINVAL;
>>
>> How can this happen? Also, you're allocating a *range*. Surely the
>> upper boundary should matter too?
> Do you know the exact reason? Please give some information, thanks.
> 

In our internal repo, we don't have middle domain in pch-msi driver, so 
no check for hwirq as in alloc of middle domain. When hwirq is assigned 
failed(negtive value), the wrong hwirq will be passed to parent 
domain(eio domain)'s alloc, so we add check in eio domain's alloc there.


But here, it seems that the check is unnecessary, because in pch-msi driver:

static int pch_msi_middle_domain_alloc(struct irq_domain *domain,
                                            unsigned int virq,
                                            unsigned int nr_irqs, void 
*args)
{
         struct pch_msi_data *priv = domain->host_data;
         int hwirq, err, i;


         hwirq = pch_msi_allocate_hwirq(priv, nr_irqs);
         if (hwirq < 0)
                 return hwirq;


         for (i = 0; i < nr_irqs; i++) {
                 err = pch_msi_parent_domain_alloc(domain, virq + i, 
hwirq + i);
                 [...]


If pch_msi_allocate_hwirq failed, pch_msi_middle_domain_alloc will 
return, and pch_msi_parent_domain_alloc(will call eio domain's alloc) 
will not be called.


>>
>> And for the umpteenth time, please add a cover letter when sending
>> multiple patches. This is a hard requirement for me.
> OK, I will add a cover letter, even for simple fix patches. Sorry.
> 
> Huacai
>>
>> Thanks,
>>
>>          M.
>>
>> --
>> Without deviation from the norm, progress is not possible.