Thread (5 messages) read the whole thread 5 messages, 3 authors, 2022-01-30

Re: [PATCH] pinctrl: bcm2835: Fix a few error paths

From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2022-01-28 16:28:06
Also in: linux-gpio, lkml

On Fri, Jan 28, 2022 at 08:12:02AM -0800, Florian Fainelli wrote:
On 1/28/2022 6:35 AM, Andy Shevchenko wrote:
quoted
On Thu, Jan 27, 2022 at 01:50:31PM -0800, Florian Fainelli wrote:
quoted
After commit 266423e60ea1 ("pinctrl: bcm2835: Change init order for gpio
hogs") a few error paths would not unwind properly the registration of
gpio ranges. Correct that by assigning a single error label and goto it
whenever we encounter a fatal error.
quoted
  1 file changed, 15 insertions(+), 8 deletions(-)
While this seems legit per se, my eyes caught this:

quoted
  	if (!girq->parents) {
-		pinctrl_remove_gpio_range(pc->pctl_dev, &pc->gpio_range);
-		return -ENOMEM;
+		err = -ENOMEM;
+		goto out_remove;
Non-devm....
quoted
  	}
  	if (is_7211) {
  		pc->wake_irq = devm_kcalloc(dev, BCM2835_NUM_IRQS,
  					    sizeof(*pc->wake_irq),
  					    GFP_KERNEL);
...followed by devm.

It means more ordering bugs in the ->remove() and error path are lurking
around. Can you double check and be sure that we do not have a case where
non-devm registration code followed by devm?
It seems to me like we are fine with the patch as is, because:

- girq->parents is allocated with devm
- pc->wake_irq is allocated with devm
- name is allocated with devm

and those are the only variables being allocated for which we also process
an error handling path.
Okay, thanks.

My worries that it might be the case when the GPIO ranges have been removed by
explicit call in ->remove() followed by some interrupt or so and oops or
misbehaviour because of that.

-- 
With Best Regards,
Andy Shevchenko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help