Re: [PATCH] powerpc/mpic: Fix memory leak of mpic on error paths

[Jyoti Gupta <jyoti@linux.ibm.com>]

On 2026-09-29 13:02, Amit Machhiwal wrote:
> Hi Jyoti,
> 
> For some reason, you response to this thread is not appearing on the 
> list.  I
> think you mail client is messing up the "In-Reply-To" information in 
> the
> header.  Fix look into fixing it.
> 
> For now, I'll qoute your reply here with my further response.
> 
Hello Amit ,

I'm not sure why my reply did not reach the list,but it is visible at 
the link below:
https://lore.kernel.org/all/e3c17fc50f7897b08fbd67a120087467@linux.ibm.com/

let me know if this looks fine.

> On 2026/09/24 03:05 PM, Amit Machhiwal wrote:
>> On 2026/09/24 12:00 PM, Jyoti Gupta wrote:
>> > mpic_alloc() allocates struct mpic (and optionally the protected
>> > sources bitmap) but does not free them when mpic_setup_error_int() or
>> > irq_domain_create_linear() fails, leaking the memory.
>> >
>> > Initialise mpic to NULL and free the bitmap and the mpic in the common
>> > error path before dropping the device node reference. The register
>> > mappings are not released here.
>> >
>> > Signed-off-by: Jyoti Gupta <jyoti@linux.ibm.com>
>> 
>> I see a couple of problems with this patch:
>> 
>> 1. The patch does not apply.  Both "git apply" and "git am" fail on 
>> this patch.
>>    I think the context lines around the irq_domain_create_linear() 
>> call have
>>    tabs converted to spaces, which causes the context match to fail.
>> 
>>       ❯ git apply 
>> ./20260924_jyoti_powerpc_mpic_fix_memory_leak_of_mpic_on_error_paths.mbx 
>> --check --verbose
>>       Checking patch arch/powerpc/sysdev/mpic.c...
>>       error: while searching for:
>>       						 intvec_top,
>>       						 &mpic_host_ops, mpic);
>> 
>>       	/*
>>       	 * FIXME: The code leaks the MPIC object and mappings here; 
>> this
>>       	 * is very unlikely to fail but it ought to be fixed anyways.
>>       	 */
>>       	if (mpic->irqhost == NULL)
>>       		goto err_of_node_put;
>> 
>> 
>>       error: patch failed: arch/powerpc/sysdev/mpic.c:1488
>>       error: arch/powerpc/sysdev/mpic.c: patch does not apply
>> 
>>    Please regenerate with:
>> 
>>      git format-patch -1
>> 
>>    and verify with:
>> 
>>      git apply --check <patch>
>> 
>>    before resending.
>> 
>> 2. The mpic_setup_error_int() leak is not actually fixed.  The commit 
>> message
>>    says both mpic_setup_error_int() and irq_domain_create_linear() 
>> failure paths
>>    are fixed, but the former still does a bare return NULL:
>> 
>>      if (fsl_version >= 0x401) {
>>          ret = mpic_setup_error_int(mpic, intvec_top - 13);
>>          if (ret)
>>              return NULL;   /* <-- still leaks mpic and 
>> mpic->protected */
>>      }
>> 
>>    This needs to become:
>> 
>>          if (ret)
>>              goto err_of_node_put;
>> 
>> Thanks,
>> Amit
>> 
>> > ---
>> >  arch/powerpc/sysdev/mpic.c | 9 ++++-----
>> >  1 file changed, 4 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/arch/powerpc/sysdev/mpic.c b/arch/powerpc/sysdev/mpic.c
>> > index 6e8fa94985f3..e777b33f170d 100644
>> > --- a/arch/powerpc/sysdev/mpic.c
>> > +++ b/arch/powerpc/sysdev/mpic.c
>> > @@ -1221,7 +1221,7 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>> >  				const char *name)
>> >  {
>> >  	int i, psize, intvec_top;
>> > -	struct mpic *mpic;
>> > +	struct mpic *mpic = NULL;
>> >  	u32 greg_feature;
>> >  	const char *vers;
>> >  	const u32 *psrc;
>> > @@ -1488,10 +1488,6 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>> >  						 intvec_top,
>> >  						 &mpic_host_ops, mpic);
>> >
>> > -	/*
>> > -	 * FIXME: The code leaks the MPIC object and mappings here; this
>> > -	 * is very unlikely to fail but it ought to be fixed anyways.
>> > -	 */
>> >  	if (mpic->irqhost == NULL)
>> >  		goto err_of_node_put;
>> >
>> > @@ -1527,6 +1523,9 @@ struct mpic * __init mpic_alloc(struct device_node *node,
>> >  	return mpic;
>> >
>> >  err_of_node_put:
>> > +	if (mpic)
>> > +		bitmap_free(mpic->protected);
>> > +	kfree(mpic);
>> >  	of_node_put(node);
>> >  	return NULL;
>> >  }
>> > --
>> > 2.55.0
>> >
>> >
>> 
> 
> Response from Jyoti:
> 
>> The mpic_setup_error_int() leak is intentionally left to a separate 
>> patch
>> already under
> 
> If that's the case, you shouldn't have mentioned about this change in 
> the commit
> log of this patch.  The commit log needs to be updated accordingly if 
> that's
> what your intent is.

Agree,I'llfix it in v2.

>>  review:
>> "[PATCH] powerpc: Fix missing of_node_put() calls"
>> That patch converts the return NULL at line 1392 to goto 
>> err_of_node_put as
>> part of a
> 
> I could only find below:
> 
> https://lore.kernel.org/all/5cef10ab1610113fe162cec1dffd97f3@linux.ibm.com/
> 
> Sorry, but I don't think thats the right way to send a patch.  You seem 
> to have
> replied to the email which did not land into the mailing lists at all.

Yes, I made a mistake earlier by not keeping 
linuxppc-dev@lists.ozlabs.org
in the loop. I later replied to all with the list included, but I 
understand
that was not the right way to do it. I'll resend the patch as a fresh 
thread
in the proper format.

> Having
> said that, I think fixing this particular path in mpic_alloc() would 
> make more
> sense in the current patch itself.

sure we can do that too.

> Please look into fixing your mail client and send the patches/responses 
> in
> proper format.
> 

Got it.

Thanks,
Jyoti

> Thanks,
> Amit.
> 
>> broader of_node_put() fix series. This patch focuses only on the
>> irq_domain_create_linear()
>> failure path.
>> 
>> Thanks,
>> Jyoti