Messages in this thread | | | Date | Wed, 1 Mar 2023 10:29:19 +0100 | Subject | Re: [PATCH] of: property: Add missing of_node_get() in parse_interrupt() | From | Jean-Jacques Hiblot <> |
| |
On 28/02/2023 20:07, Saravana Kannan wrote: > On Tue, Feb 28, 2023 at 9:40 AM Jean-Jacques Hiblot > <jjhiblot@traphandler.com> wrote: >> From: Jean Jacques Hiblot <jjhiblot@traphandler.com> >> >> As all the other parsers do, parse_interrupt() must increase the refcount >> of the device_node. Otherwise the refcount is decremented every time >> parse_interrupt() is called on this node, leading to a potential >> use-after-free. >> >> This is a regression introduced by commit f265f06af194 ("of: property: >> Fix fw_devlink handling of interrupts/interrupts-extended"). The reason is >> that of_irq_parse_one() does not increase the refcount while the previously >> used of_irq_find_parent() does. > Thanks for catching the issue Jean! > > This feels like a bug in of_irq_parse_one() to me. It's returning a > reference to a node without doing a of_node_get() on it. > > Rob, Marc, Do you agree?
Sarvana,
it looks like you're right. The bug seems to be in of_irq_parse_one().
It doesn't behave in the same way for "interrupts-extended" where it does a get() and 'interrupts" where it doesn't.
So please ignore this patch.
Thanks
> > Jean, > > If they agree, can you please fix of_irq_parse_one() and add a > of_node_put() to existing callers (if they aren't already doing a > put()). > > Thanks, > Saravana > >> Fixes: f265f06af194 ("of: property: Fix fw_devlink handling of interrupts/interrupts-extended") >> Signed-off-by: Jean Jacques Hiblot <jjhiblot@traphandler.com> >> --- >> drivers/of/property.c | 5 ++++- >> 1 file changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/of/property.c b/drivers/of/property.c >> index 134cfc980b70b..1f23bcb765c4e 100644 >> --- a/drivers/of/property.c >> +++ b/drivers/of/property.c >> @@ -1380,7 +1380,10 @@ static struct device_node *parse_interrupts(struct device_node *np, >> strcmp(prop_name, "interrupts-extended")) >> return NULL; >> >> - return of_irq_parse_one(np, index, &sup_args) ? NULL : sup_args.np; >> + if (of_irq_parse_one(np, index, &sup_args)) >> + return NULL; >> + >> + return of_node_get(sup_args.np); >> } >> >> static const struct supplier_bindings of_supplier_bindings[] = { >> -- >> 2.25.1 >>
| |