Re: [PATCH 2/6] irqchip/gic-v3-its: Fix memleak in its_probe_one()

[Marc Zyngier <maz@kernel.org>]

On Tue, 16 Jun 2026 02:39:10 +0100,
Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> 
> 在 2026/6/15 16:59:14, Marc Zyngier 写道:
> > On Mon, 15 Jun 2026 04:29:06 +0100,
> > Kemeng Shi <shikemeng@huaweicloud.com> wrote:
> >>
> >> Fix collection leak when its_init_domain() failed in its_probe_one().
> >>
> >> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> >> ---
> >>  drivers/irqchip/irq-gic-v3-its.c | 10 +++++++++-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c b/drivers/irqchip/irq-gic-v3-its.c
> >> index 2b7b546c43c8..df26ddc97ae2 100644
> >> --- a/drivers/irqchip/irq-gic-v3-its.c
> >> +++ b/drivers/irqchip/irq-gic-v3-its.c
> >> @@ -3032,6 +3032,12 @@ static int its_alloc_collections(struct its_node *its)
> >>  	return 0;
> >>  }
> >>  
> >> +static void its_free_collections(struct its_node *its)
> >> +{
> >> +	kfree(its->collections);
> >> +	its->collections = NULL;
> >> +}
> > 
> > Why do we need an extra helper for something that has a single calling
> > spot? Why is it important to set collections to NULL, given that we're
> > about to free the structure without even looking further?
> > 
> The extra helper is added for symmetry with its_alloc_collections(), keeping
> allocation/deallocation logic paired.

But there is zero logic here. None. it is a call to kfree(), just
that, and we don't need any additional abstraction.

> In this way, we need to only modified
> allocation/deallocation function if more member is added for collections.

Well, when we get there, we can always add an abstraction.

> Setting collections to NULL is a personal habit to quickly catch use-after-free
> of collections. Could drop this which is not that import.

I don't see the point of doing that.

	M.

-- 
Without deviation from the norm, progress is not possible.