Re: [PATCH] iommu/ipmmu-vmsa: Avoid leak OF node on error

[Robin Murphy]

On 2022-05-23 12:54, Johan Hovold wrote:
> On Mon, May 23, 2022 at 11:11:45AM +0000, cgel.zte@xxxxxxxxx wrote:
> > From: Minghao Chi <chi.minghao@xxxxxxxxxx>
> >
> > The OF node should be put before returning error in ipmmu_init(),
> > otherwise node's refcount will be leaked.
> >
> > Reported-by: Zeal Robot <zealci@xxxxxxxxxx>
> > Signed-off-by: Minghao Chi <chi.minghao@xxxxxxxxxx>
> > ---
> >   drivers/iommu/ipmmu-vmsa.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/iommu/ipmmu-vmsa.c b/drivers/iommu/ipmmu-vmsa.c
> > index 8fdb84b3642b..f6440b106f46 100644
> > --- a/drivers/iommu/ipmmu-vmsa.c
> > +++ b/drivers/iommu/ipmmu-vmsa.c
> > @@ -1179,11 +1179,10 @@ static int __init ipmmu_init(void)
> >   		return 0;
> >   
> >   	np = of_find_matching_node(NULL, ipmmu_of_ids);
> > +	of_node_put(np);
> >   	if (!np)
> >   		return 0;
> >   
> > -	of_node_put(np);
> > -
> >   	ret = platform_driver_register(&ipmmu_driver);
> >   	if (ret < 0)
> >   		return ret;
>
> NAK

Indeed. How exactly can we hold a refcount on NULL, let alone leak it?

Static checkers are great for flagging up code that *might* have issues, 
but please actually *look* at the code and apply some thought before 
sending a patch.

Robin.