Message ID | 20250215054431.55747-4-xueshuai@linux.alibaba.com (mailing list archive) |
---|---|
State | New |
Headers | show |
Series | dmaengine: idxd: fix memory leak in error handling path | expand |
Hi, Shuai, On 2/14/25 21:44, Shuai Xue wrote: > Memory allocated for groups is not freed if an error occurs during > idxd_setup_groups(). To fix it, free the allocated memory in the reverse > order of allocation before exiting the function in case of an error. > > Fixes: defe49f96012 ("dmaengine: idxd: fix group conf_dev lifetime") > Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> > Reviewed-by: Dave Jiang <dave.jiang@intel.com> > --- > drivers/dma/idxd/init.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c > index 4e47075c5bef..a2da68e6144d 100644 > --- a/drivers/dma/idxd/init.c > +++ b/drivers/dma/idxd/init.c > @@ -328,6 +328,7 @@ static int idxd_setup_groups(struct idxd_device *idxd) > rc = dev_set_name(conf_dev, "group%d.%d", idxd->id, group->id); > if (rc < 0) { > put_device(conf_dev); > + kfree(group); > goto err; > } > > @@ -352,7 +353,10 @@ static int idxd_setup_groups(struct idxd_device *idxd) > while (--i >= 0) { > group = idxd->groups[i]; > put_device(group_confdev(group)); > + kfree(group); > } > + kfree(idxd->groups); > + What happens to the memory areas previously allocated for wqs and engines after idxd_setup_groups() fails? They need to be freed as well, but currently they are not. Maybe a separate patch cleans up the previously allocated mem areas for wqs/engines/groups if there is any failure after the allocations? > return rc; > } > Thanks. -Fenghua
在 2025/2/19 04:20, Fenghua Yu 写道: > Hi, Shuai, > > On 2/14/25 21:44, Shuai Xue wrote: >> Memory allocated for groups is not freed if an error occurs during >> idxd_setup_groups(). To fix it, free the allocated memory in the reverse >> order of allocation before exiting the function in case of an error. >> >> Fixes: defe49f96012 ("dmaengine: idxd: fix group conf_dev lifetime") >> Signed-off-by: Shuai Xue <xueshuai@linux.alibaba.com> >> Reviewed-by: Dave Jiang <dave.jiang@intel.com> >> --- >> drivers/dma/idxd/init.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c >> index 4e47075c5bef..a2da68e6144d 100644 >> --- a/drivers/dma/idxd/init.c >> +++ b/drivers/dma/idxd/init.c >> @@ -328,6 +328,7 @@ static int idxd_setup_groups(struct idxd_device *idxd) >> rc = dev_set_name(conf_dev, "group%d.%d", idxd->id, group->id); >> if (rc < 0) { >> put_device(conf_dev); >> + kfree(group); >> goto err; >> } >> @@ -352,7 +353,10 @@ static int idxd_setup_groups(struct idxd_device *idxd) >> while (--i >= 0) { >> group = idxd->groups[i]; >> put_device(group_confdev(group)); >> + kfree(group); >> } >> + kfree(idxd->groups); >> + > > What happens to the memory areas previously allocated for wqs and engines after idxd_setup_groups() fails? They need to be freed as well, but currently they are not. > > Maybe a separate patch cleans up the previously allocated mem areas for wqs/engines/groups if there is any failure after the allocations? > Agreed, will add a new patch to fix it. Thanks Shuai
diff --git a/drivers/dma/idxd/init.c b/drivers/dma/idxd/init.c index 4e47075c5bef..a2da68e6144d 100644 --- a/drivers/dma/idxd/init.c +++ b/drivers/dma/idxd/init.c @@ -328,6 +328,7 @@ static int idxd_setup_groups(struct idxd_device *idxd) rc = dev_set_name(conf_dev, "group%d.%d", idxd->id, group->id); if (rc < 0) { put_device(conf_dev); + kfree(group); goto err; } @@ -352,7 +353,10 @@ static int idxd_setup_groups(struct idxd_device *idxd) while (--i >= 0) { group = idxd->groups[i]; put_device(group_confdev(group)); + kfree(group); } + kfree(idxd->groups); + return rc; }