| Message ID | 20221126073315.365567-3-yangyingliang@huawei.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series | gpu: host1x: possible double free and memory leak | expand |
On 11/26/22 09:33, Yang Yingliang wrote: > The device names allocated by dev_set_name() need be freed > before module unloading, but they can not be freed because > the kobject's refcount which was set in device_initialize() > has not be decreased to 0. > > As comment of device_add() says, if it fails, use only > put_device() drop the refcount, then the name will be > freed in kobejct_cleanup(). > > device_del() and put_device() can be replaced with > device_unregister(), so call it to unregister the added > successfully devices, and just call put_device() to the > not added device. > > Add a release() function to device to avoid null release() > function WARNING in device_release(), it's empty, because > the context devices are freed together in > host1x_memory_context_list_free(). > > Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/gpu/host1x/context.c | 23 +++++++++++++++-------- > 1 file changed, 15 insertions(+), 8 deletions(-) > > diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c > index 291f34562e2e..047696432eb2 100644 > --- a/drivers/gpu/host1x/context.c > +++ b/drivers/gpu/host1x/context.c > @@ -13,6 +13,11 @@ > #include "context.h" > #include "dev.h" > > +static void host1x_memory_context_release(struct device *dev) > +{ > + /* context device is freed in host1x_memory_context_list_free() */ > +} > + > int host1x_memory_context_list_init(struct host1x *host1x) > { > struct host1x_memory_context_list *cdl = &host1x->context_list; > @@ -53,28 +58,30 @@ int host1x_memory_context_list_init(struct host1x *host1x) > dev_set_name(&ctx->dev, "host1x-ctx.%d", i); > ctx->dev.bus = &host1x_context_device_bus_type; > ctx->dev.parent = host1x->dev; > + ctx->dev.release = host1x_memory_context_release; > > dma_set_max_seg_size(&ctx->dev, UINT_MAX); > > err = device_add(&ctx->dev); > if (err) { > dev_err(host1x->dev, "could not add context device %d: %d\n", i, err); > - goto del_devices; > + put_device(&ctx->dev); > + goto unreg_devices; > } > > err = of_dma_configure_id(&ctx->dev, node, true, &i); > if (err) { > dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n", > i, err); > - device_del(&ctx->dev); > - goto del_devices; > + device_unregister(&ctx->dev); > + goto unreg_devices; > } > > fwspec = dev_iommu_fwspec_get(&ctx->dev); > if (!fwspec || !device_iommu_mapped(&ctx->dev)) { > dev_err(host1x->dev, "Context device %d has no IOMMU!\n", i); > - device_del(&ctx->dev); > - goto del_devices; > + device_unregister(&ctx->dev); > + goto unreg_devices; > } > > ctx->stream_id = fwspec->ids[0] & 0xffff; > @@ -82,9 +89,9 @@ int host1x_memory_context_list_init(struct host1x *host1x) > > return 0; > > -del_devices: > +unreg_devices: > while (i--) > - device_del(&cdl->devs[i].dev); > + device_unregister(&cdl->devs[i].dev); > > kfree(cdl->devs); > cdl->devs = NULL; > @@ -98,7 +105,7 @@ void host1x_memory_context_list_free(struct host1x_memory_context_list *cdl) > unsigned int i; > > for (i = 0; i < cdl->len; i++) > - device_del(&cdl->devs[i].dev); > + device_unregister(&cdl->devs[i].dev); > > kfree(cdl->devs); > cdl->len = 0; Reviewed-by: Mikko Perttunen <mperttunen@nvidia.com>
diff --git a/drivers/gpu/host1x/context.c b/drivers/gpu/host1x/context.c index 291f34562e2e..047696432eb2 100644 --- a/drivers/gpu/host1x/context.c +++ b/drivers/gpu/host1x/context.c @@ -13,6 +13,11 @@ #include "context.h" #include "dev.h" +static void host1x_memory_context_release(struct device *dev) +{ + /* context device is freed in host1x_memory_context_list_free() */ +} + int host1x_memory_context_list_init(struct host1x *host1x) { struct host1x_memory_context_list *cdl = &host1x->context_list; @@ -53,28 +58,30 @@ int host1x_memory_context_list_init(struct host1x *host1x) dev_set_name(&ctx->dev, "host1x-ctx.%d", i); ctx->dev.bus = &host1x_context_device_bus_type; ctx->dev.parent = host1x->dev; + ctx->dev.release = host1x_memory_context_release; dma_set_max_seg_size(&ctx->dev, UINT_MAX); err = device_add(&ctx->dev); if (err) { dev_err(host1x->dev, "could not add context device %d: %d\n", i, err); - goto del_devices; + put_device(&ctx->dev); + goto unreg_devices; } err = of_dma_configure_id(&ctx->dev, node, true, &i); if (err) { dev_err(host1x->dev, "IOMMU configuration failed for context device %d: %d\n", i, err); - device_del(&ctx->dev); - goto del_devices; + device_unregister(&ctx->dev); + goto unreg_devices; } fwspec = dev_iommu_fwspec_get(&ctx->dev); if (!fwspec || !device_iommu_mapped(&ctx->dev)) { dev_err(host1x->dev, "Context device %d has no IOMMU!\n", i); - device_del(&ctx->dev); - goto del_devices; + device_unregister(&ctx->dev); + goto unreg_devices; } ctx->stream_id = fwspec->ids[0] & 0xffff; @@ -82,9 +89,9 @@ int host1x_memory_context_list_init(struct host1x *host1x) return 0; -del_devices: +unreg_devices: while (i--) - device_del(&cdl->devs[i].dev); + device_unregister(&cdl->devs[i].dev); kfree(cdl->devs); cdl->devs = NULL; @@ -98,7 +105,7 @@ void host1x_memory_context_list_free(struct host1x_memory_context_list *cdl) unsigned int i; for (i = 0; i < cdl->len; i++) - device_del(&cdl->devs[i].dev); + device_unregister(&cdl->devs[i].dev); kfree(cdl->devs); cdl->len = 0;
The device names allocated by dev_set_name() need be freed before module unloading, but they can not be freed because the kobject's refcount which was set in device_initialize() has not be decreased to 0. As comment of device_add() says, if it fails, use only put_device() drop the refcount, then the name will be freed in kobejct_cleanup(). device_del() and put_device() can be replaced with device_unregister(), so call it to unregister the added successfully devices, and just call put_device() to the not added device. Add a release() function to device to avoid null release() function WARNING in device_release(), it's empty, because the context devices are freed together in host1x_memory_context_list_free(). Fixes: 8aa5bcb61612 ("gpu: host1x: Add context device management code") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/gpu/host1x/context.c | 23 +++++++++++++++-------- 1 file changed, 15 insertions(+), 8 deletions(-)