| Message ID | 20240112064107.137384-1-chentao@kylinos.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vfio/platform: Remove unnecessary free in vfio_set_trigger | expand |
On Fri, 12 Jan 2024 14:41:07 +0800 Kunwu Chan <chentao@kylinos.cn> wrote: > commit 57f972e2b341 ("vfio/platform: trigger an interrupt via eventfd") > add 'name' as member for vfio_platform_irq,it's initialed by kasprintf, > so there is no need to free it before initializing. What?! Just look at the call path where vfio_set_trigger() is called with a valid fd and existing trigger. This change would leak irq->name as it's reallocated via kasprintf(). Thanks, Alex > Fixes: 57f972e2b341 ("vfio/platform: trigger an interrupt via eventfd") > Signed-off-by: Kunwu Chan <chentao@kylinos.cn> > --- > drivers/vfio/platform/vfio_platform_irq.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c > index 61a1bfb68ac7..5e3fd1926366 100644 > --- a/drivers/vfio/platform/vfio_platform_irq.c > +++ b/drivers/vfio/platform/vfio_platform_irq.c > @@ -179,7 +179,6 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, > if (irq->trigger) { > irq_clear_status_flags(irq->hwirq, IRQ_NOAUTOEN); > free_irq(irq->hwirq, irq); > - kfree(irq->name); > eventfd_ctx_put(irq->trigger); > irq->trigger = NULL; > }
On 2024/1/12 23:34, Alex Williamson wrote: > On Fri, 12 Jan 2024 14:41:07 +0800 > Kunwu Chan <chentao@kylinos.cn> wrote: > >> commit 57f972e2b341 ("vfio/platform: trigger an interrupt via eventfd") >> add 'name' as member for vfio_platform_irq,it's initialed by kasprintf, >> so there is no need to free it before initializing. > > What?! Just look at the call path where vfio_set_trigger() is called > with a valid fd and existing trigger. This change would leak irq->name > as it's reallocated via kasprintf(). Thanks, > > Alex > >> Fixes: 57f972e2b341 ("vfio/platform: trigger an interrupt via eventfd") >> Signed-off-by: Kunwu Chan <chentao@kylinos.cn> >> --- >> drivers/vfio/platform/vfio_platform_irq.c | 1 - >> 1 file changed, 1 deletion(-) >> >> diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c >> index 61a1bfb68ac7..5e3fd1926366 100644 >> --- a/drivers/vfio/platform/vfio_platform_irq.c >> +++ b/drivers/vfio/platform/vfio_platform_irq.c >> @@ -179,7 +179,6 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, >> if (irq->trigger) { >> irq_clear_status_flags(irq->hwirq, IRQ_NOAUTOEN); >> free_irq(irq->hwirq, irq); >> - kfree(irq->name); >> eventfd_ctx_put(irq->trigger); >> irq->trigger = NULL; >> } > Thanks for your reply. It's my bad.I misunderstood. Sorry to bother you.
diff --git a/drivers/vfio/platform/vfio_platform_irq.c b/drivers/vfio/platform/vfio_platform_irq.c index 61a1bfb68ac7..5e3fd1926366 100644 --- a/drivers/vfio/platform/vfio_platform_irq.c +++ b/drivers/vfio/platform/vfio_platform_irq.c @@ -179,7 +179,6 @@ static int vfio_set_trigger(struct vfio_platform_device *vdev, int index, if (irq->trigger) { irq_clear_status_flags(irq->hwirq, IRQ_NOAUTOEN); free_irq(irq->hwirq, irq); - kfree(irq->name); eventfd_ctx_put(irq->trigger); irq->trigger = NULL; }
commit 57f972e2b341 ("vfio/platform: trigger an interrupt via eventfd") add 'name' as member for vfio_platform_irq,it's initialed by kasprintf, so there is no need to free it before initializing. Fixes: 57f972e2b341 ("vfio/platform: trigger an interrupt via eventfd") Signed-off-by: Kunwu Chan <chentao@kylinos.cn> --- drivers/vfio/platform/vfio_platform_irq.c | 1 - 1 file changed, 1 deletion(-)