Re: [PATCH 3/5] gpio: mvebu: add pwm support for Armada 8K/7K

From: Baruch Siach <baruch@tkos.co.il>
To: Andrew Lunn <andrew@lunn.ch>
Cc: linux-pwm@vger.kernel.org,
	"Sascha Hauer" <s.hauer@pengutronix.de>,
	"Jason Cooper" <jason@lakedaemon.net>,
	linux-gpio@vger.kernel.org,
	"Linus Walleij" <linus.walleij@linaro.org>,
	"Chris Packham" <chris.packham@alliedtelesis.co.nz>,
	"Bartosz Golaszewski" <bgolaszewski@baylibre.com>,
	"Thierry Reding" <thierry.reding@gmail.com>,
	"Thomas Petazzoni" <thomas.petazzoni@bootlin.com>,
	"Uwe Kleine-König" <u.kleine-koenig@pengutronix.de>,
	"Ralph Sennhauser" <ralph.sennhauser@gmail.com>,
	"Lee Jones" <lee.jones@linaro.org>,
	"Gregory Clement" <gregory.clement@bootlin.com>,
	linux-arm-kernel@lists.infradead.org,
	"Sebastian Hesselbarth" <sebastian.hesselbarth@gmail.com>
Subject: Re: [PATCH 3/5] gpio: mvebu: add pwm support for Armada 8K/7K
Date: Thu, 19 Nov 2020 15:47:18 +0200	[thread overview]
Message-ID: <877dqhfogp.fsf@tarshish> (raw)
In-Reply-To: <20201119133429.GS1804098@lunn.ch>

Hi Andrew,

On Thu, Nov 19 2020, Andrew Lunn wrote:
>> >> @@ -1200,6 +1235,13 @@ static int mvebu_gpio_probe(struct platform_device *pdev)
>> >>  
>> >>  	devm_gpiochip_add_data(&pdev->dev, &mvchip->chip, mvchip);
>> >>  
>> >> +	/* Some MVEBU SoCs have simple PWM support for GPIO lines */
>> >> +	if (IS_ENABLED(CONFIG_PWM)) {
>> >> +		err = mvebu_pwm_probe(pdev, mvchip, id);
>> >> +		if (err)
>> >> +			return err;
>> >> +	}
>> >> +
>> >
>> > The existing error handling looks odd here. Why is there no goto
>> > err_domain when probing the PWMs fails? I wonder if this a bug from me
>> > from a long time again?
>> 
>> What would you release under the err_domain label? As far as I can see
>> all resources are allocated using devres, and released automatically on
>> failure exit.
>
> The IRQ domain is still registers. So once the memory is automatically
> freed, don't we have a potential use after free?

This patch moves PWM registration before IRQ domain registration for
another reason as mentioned in the commit log. So this might
incidentally fix the bug.

Would you prefer a separate patch for that with a 'Fixes:
757642f9a584e8' tag?

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel