On Wed, 2019-04-17 at 10:44 +0800, Wen Yang wrote: > In stratix10_svc_init function, fw_np is obtained by calling > of_find_node_by_name(), np is obtained by calling > of_find_matching_node(), and the reference counts of those > two device_nodes, fw_np and np, are increased. > But when the function exits, only of_node_put is called on np, > and fw_np's reference count is leaked. > > Detected by coccinelle with the following warnings: > ./drivers/firmware/stratix10-svc.c:1020:2-8: ERROR: missing of_node_put; > acquired a node pointer with refcount incremented on line 1014, but without a > corresponding object release within this function. > ./drivers/firmware/stratix10-svc.c:1025:2-8: ERROR: missing of_node_put; > acquired a node pointer with refcount incremented on line 1014, but without a > corresponding object release within this function. > ./drivers/firmware/stratix10-svc.c:1027:1-7: ERROR: missing of_node_put; > acquired a node pointer with refcount incremented on line 1014, but without a > corresponding object release within this function. > > Signed-off-by: Wen Yang <[email protected]> > Cc: Greg Kroah-Hartman <[email protected]> > Cc: Alan Tull <[email protected]> > Cc: Richard Gong <[email protected]> > Cc: Nicolas Saenz Julienne <[email protected]> > Cc: [email protected] > --- > drivers/firmware/stratix10-svc.c | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/drivers/firmware/stratix10-svc.c b/drivers/firmware/stratix10- > svc.c > index 6e65148..482a6bd 100644 > --- a/drivers/firmware/stratix10-svc.c > +++ b/drivers/firmware/stratix10-svc.c > @@ -1016,15 +1016,21 @@ static int __init stratix10_svc_init(void) > return -ENODEV; > > np = of_find_matching_node(fw_np, stratix10_svc_drv_match);
Sorry but this patch isn't right, of_find_matching_node() will free the
reference to fw_np internally.
> - if (!np)
> - return -ENODEV;
> + if (!np) {
> + ret = -ENODEV;
> + goto out_put_fw_np;
> + }
>
> of_node_put(np);
> ret = of_platform_populate(fw_np, stratix10_svc_drv_match, NULL, NULL);
> if (ret)
> - return ret;
> + goto out_put_fw_np;
Consequently and assuming I'm not missing something, I think fw_np shouldn't be
used here as is.
Regards,
Nicolas
signature.asc
Description: This is a digitally signed message part
