[PATCH v2] gpio: mvebu: fix potential user-after-free on probe
Baruch Siach
baruch at tkos.co.il
Mon Nov 30 13:12:53 EST 2020
Hi Uwe,
(+ tglx)
On Mon, Nov 30 2020, Uwe Kleine-König wrote:
> On Mon, Nov 30, 2020 at 05:09:53PM +0200, Baruch Siach wrote:
>> When mvebu_pwm_probe() fails IRQ domain is not released. Goto the
>> err_domain label on failure to release IRQ domain.
>>
>> Fixes: 757642f9a584 ("gpio: mvebu: Add limited PWM support")
>> Reported-by: Andrew Lunn <andrew at lunn.ch>
>> Signed-off-by: Baruch Siach <baruch at tkos.co.il>
>> ---
>> v2: Don't leak pwm resources (Uwe Kleine-König)
>>
>> This is split out of the "gpio: mvebu: Armada 8K/7K PWM support" series.
>> I'll rebase the series v2 on top on this fix.
>> ---
>> drivers/gpio/gpio-mvebu.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpio/gpio-mvebu.c b/drivers/gpio/gpio-mvebu.c
>> index 433e2c3f3fd5..c53ed975a180 100644
>> --- a/drivers/gpio/gpio-mvebu.c
>> +++ b/drivers/gpio/gpio-mvebu.c
>> @@ -1255,8 +1255,11 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
>> }
>>
>> /* Some MVEBU SoCs have simple PWM support for GPIO lines */
>> - if (IS_ENABLED(CONFIG_PWM))
>> - return mvebu_pwm_probe(pdev, mvchip, id);
>> + if (IS_ENABLED(CONFIG_PWM)) {
>> + err = mvebu_pwm_probe(pdev, mvchip, id);
>> + if (err)
>> + goto err_domain;
>
> I only looked quickly, but I wonder if you need to undo
> irq_alloc_domain_generic_chips(), too?!
So it seems. __irq_alloc_domain_generic_chips() calls kzalloc() for the
gc field of irq_domain. But I could not find any code that releases this
allocation. These drivers call irq_alloc_domain_generic_chips(), but do
not release gc on failure:
drivers/irqchip/irq-ingenic-tcu.c
drivers/irqchip/irq-orion.c
drivers/irqchip/irq-renesas-irqc.c
drivers/irqchip/irq-sunxi-nmi.c
drivers/pinctrl/pinctrl-rockchip.c
Some of them apparently skip the cleanup because the system would be
unusable anyway. But most of them call irq_domain_remove() on failure.
Thomas, what is the right thing to do here? Should we just call
kfree(mvchip->domain->gc);
directly to release the allocation?
Thanks,
baruch
--
~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
More information about the linux-arm-kernel
mailing list