Messages in this thread | | | Subject | Re: [PATCH v2] thunderbolt: Fix to check return value of ida_simple_get | From | Mukesh Ojha <> | Date | Thu, 21 Mar 2019 02:09:41 +0530 |
| |
On 3/20/2019 9:59 PM, Mika Westerberg wrote: > On Wed, Mar 20, 2019 at 11:24:45AM -0500, Aditya Pakki wrote: >> In enumerate_services, ida_simple_get on failure can return an error and >> leaks memory during device_register failure. The patch ensures that >> the dev_set_name is set on non failure cases, and releases memory in >> case of failure. >> >> Signed-off-by: Aditya Pakki <pakki001@umn.edu> >> >> --- >> v1: Missed cleanup of svc in case of allocation failure and >> device_register failure. >> --- >> drivers/thunderbolt/xdomain.c | 9 ++++++++- >> 1 file changed, 8 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/thunderbolt/xdomain.c b/drivers/thunderbolt/xdomain.c >> index e27dd8beb94b..eb08275185bf 100644 >> --- a/drivers/thunderbolt/xdomain.c >> +++ b/drivers/thunderbolt/xdomain.c >> @@ -740,6 +740,7 @@ static void enumerate_services(struct tb_xdomain *xd) >> struct tb_service *svc; >> struct tb_property *p; >> struct device *dev; >> + int id; >> >> /* >> * First remove all services that are not available anymore in >> @@ -768,7 +769,12 @@ static void enumerate_services(struct tb_xdomain *xd) >> break; >> } >> >> - svc->id = ida_simple_get(&xd->service_ids, 0, 0, GFP_KERNEL); >> + id = ida_simple_get(&xd->service_ids, 0, 0, GFP_KERNEL); >> + if (id < 0) { >> + kfree(svc); >> + break; >> + } >> + svc->id = id; >> svc->dev.bus = &tb_bus_type; >> svc->dev.type = &tb_service_type; >> svc->dev.parent = &xd->dev; >> @@ -776,6 +782,7 @@ static void enumerate_services(struct tb_xdomain *xd) >> >> if (device_register(&svc->dev)) { >> put_device(&svc->dev); >> + kfree(svc); > You can't do this after device_register() is called. The put_device() > above is sufficient.
If device_register fails, how would svc gets freed? we need to kfree svc here as well.
Thanks, Mukesh
| |