* [PATCH] irq/generic-chip: Fix memory leak of domain->name
@ 2017-09-27 12:28 Jeffy Chen
2017-09-27 13:29 ` Thomas Gleixner
0 siblings, 1 reply; 4+ messages in thread
From: Jeffy Chen @ 2017-09-27 12:28 UTC (permalink / raw)
To: linux-kernel; +Cc: Jeffy Chen, Thomas Gleixner
Free domain->name when IRQ_DOMAIN_NAME_ALLOCATED been set.
Fixes: d59f6617eef0 ("genirq: Allow fwnode to carry name information only")
Signed-off-by: Jeffy Chen <jeffy.chen@rock-chips.com>
---
kernel/irq/generic-chip.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
index f7086b78ad6e..174dae8ee7fe 100644
--- a/kernel/irq/generic-chip.c
+++ b/kernel/irq/generic-chip.c
@@ -322,6 +322,10 @@ int __irq_alloc_domain_generic_chips(struct irq_domain *d, int irqs_per_chip,
/* Calc pointer to the next generic chip */
tmp += sizeof(*gc) + num_ct * sizeof(struct irq_chip_type);
}
+ if (d->flags & IRQ_DOMAIN_NAME_ALLOCATED) {
+ kfree(d->name);
+ d->flags &= ~IRQ_DOMAIN_NAME_ALLOCATED;
+ }
d->name = name;
return 0;
}
--
2.11.0
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] irq/generic-chip: Fix memory leak of domain->name
2017-09-27 12:28 [PATCH] irq/generic-chip: Fix memory leak of domain->name Jeffy Chen
@ 2017-09-27 13:29 ` Thomas Gleixner
2017-09-28 3:18 ` jeffy
0 siblings, 1 reply; 4+ messages in thread
From: Thomas Gleixner @ 2017-09-27 13:29 UTC (permalink / raw)
To: Jeffy Chen; +Cc: linux-kernel
On Wed, 27 Sep 2017, Jeffy Chen wrote:
> Free domain->name when IRQ_DOMAIN_NAME_ALLOCATED been set.
I can see that from the patch, but you fail to explain what the problem
is.
It's actually more than just a memory leak. If the domain gets destroyed
then the domain free code would try to free d->name which might be a string
constant .....
> + if (d->flags & IRQ_DOMAIN_NAME_ALLOCATED) {
> + kfree(d->name);
> + d->flags &= ~IRQ_DOMAIN_NAME_ALLOCATED;
> + }
> d->name = name;
I don't think that this is the proper thing to do. There is no reason why
the domain should have the same name as the irq chip. So we rather should
do:
if (!d->name)
d->name = name;
Along with a proper comment.
Thanks,
tglx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] irq/generic-chip: Fix memory leak of domain->name
2017-09-27 13:29 ` Thomas Gleixner
@ 2017-09-28 3:18 ` jeffy
2017-09-28 4:36 ` jeffy
0 siblings, 1 reply; 4+ messages in thread
From: jeffy @ 2017-09-28 3:18 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linux-kernel
Hi Thomas,
Thanks for your review.
On 09/27/2017 09:29 PM, Thomas Gleixner wrote:
> On Wed, 27 Sep 2017, Jeffy Chen wrote:
>
>> Free domain->name when IRQ_DOMAIN_NAME_ALLOCATED been set.
>
> I can see that from the patch, but you fail to explain what the problem
> is.
>
> It's actually more than just a memory leak. If the domain gets destroyed
> then the domain free code would try to free d->name which might be a string
> constant .....
ok, i'll add that to commit msg.
>
>> + if (d->flags & IRQ_DOMAIN_NAME_ALLOCATED) {
>> + kfree(d->name);
>> + d->flags &= ~IRQ_DOMAIN_NAME_ALLOCATED;
>> + }
>> d->name = name;
>
> I don't think that this is the proper thing to do. There is no reason why
> the domain should have the same name as the irq chip. So we rather should
> do:
>
> if (!d->name)
> d->name = name;
>
> Along with a proper comment.
that is better, will do it in next version :)
>
> Thanks,
>
> tglx
>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] irq/generic-chip: Fix memory leak of domain->name
2017-09-28 3:18 ` jeffy
@ 2017-09-28 4:36 ` jeffy
0 siblings, 0 replies; 4+ messages in thread
From: jeffy @ 2017-09-28 4:36 UTC (permalink / raw)
To: Thomas Gleixner; +Cc: linux-kernel
Hi Thomas,
On 09/28/2017 11:18 AM, jeffy wrote:
>>
>> I don't think that this is the proper thing to do. There is no reason why
>> the domain should have the same name as the irq chip. So we rather should
>> do:
>>
>> if (!d->name)
>> d->name = name;
>>
>> Along with a proper comment.
> that is better, will do it in next version :)
it looks like the __irq_domain_add() would guarantee every domain has a
valid name, maybe we can just remove this?
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-09-28 4:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-27 12:28 [PATCH] irq/generic-chip: Fix memory leak of domain->name Jeffy Chen
2017-09-27 13:29 ` Thomas Gleixner
2017-09-28 3:18 ` jeffy
2017-09-28 4:36 ` jeffy
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.