Message ID | 20221122125425.1107474-1-yangyingliang@huawei.com |
---|---|
State | New |
Headers | show |
Series | PNP: fix name memory leak in pnp_register_protocol() | expand |
On Tue, Nov 22, 2022 at 1:57 PM Yang Yingliang <yangyingliang@huawei.com> wrote: > > After commit 1fa5ae857bb1 ("driver core: get rid of struct device's > bus_id string array"), the name of device is allocated dynamically, > it need be freed in error path. > > Current all protocols used in pnp_register_protocol() is static, they > don't have a release function(), so just call kfree_const() to free > the name. > > Fixes: 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id string array") > Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> > --- > drivers/pnp/core.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c > index 6a60c5d83383..390e449c17ef 100644 > --- a/drivers/pnp/core.c > +++ b/drivers/pnp/core.c > @@ -72,8 +72,10 @@ int pnp_register_protocol(struct pnp_protocol *protocol) > mutex_unlock(&pnp_lock); > > ret = device_register(&protocol->dev); > - if (ret) > + if (ret) { > pnp_remove_protocol(protocol); > + kfree_const(protocol->dev.kobj.name); Having to refer to dev.kobj here doesn't seem right. Shouldn't this just use dev->init_name and allow the core to sort out the error case? > + } > > return ret; > } > --
On 2022/11/24 2:50, Rafael J. Wysocki wrote: > On Tue, Nov 22, 2022 at 1:57 PM Yang Yingliang <yangyingliang@huawei.com> wrote: >> After commit 1fa5ae857bb1 ("driver core: get rid of struct device's >> bus_id string array"), the name of device is allocated dynamically, >> it need be freed in error path. >> >> Current all protocols used in pnp_register_protocol() is static, they >> don't have a release function(), so just call kfree_const() to free >> the name. >> >> Fixes: 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id string array") >> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >> --- >> drivers/pnp/core.c | 4 +++- >> 1 file changed, 3 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c >> index 6a60c5d83383..390e449c17ef 100644 >> --- a/drivers/pnp/core.c >> +++ b/drivers/pnp/core.c >> @@ -72,8 +72,10 @@ int pnp_register_protocol(struct pnp_protocol *protocol) >> mutex_unlock(&pnp_lock); >> >> ret = device_register(&protocol->dev); >> - if (ret) >> + if (ret) { >> pnp_remove_protocol(protocol); >> + kfree_const(protocol->dev.kobj.name); > Having to refer to dev.kobj here doesn't seem right. After calling dev_set_name(), the 'dev.kobj.name' pointer a allocated name. > > Shouldn't this just use dev->init_name and allow the core to sort out > the error case? dev_set_name(&protocol->dev, "pnp%d", nodenum) is called outside device_register(), the name should be freed by calling put_device() (kobject_cleanup()). Comment of device_register says: NOTE: _Never_ directly free @dev after calling this function, even if it returned an error! Always use put_device() to give up the reference initialized in this function instead. Current all caller's device(pnpacpi_protocol.dev and pnpbios_protocol.dev) is const, they don't need be freed, and don't have a release() function. So I just call kfree(name) instead of calling put_device() here. Thanks, Yang > >> + } >> >> return ret; >> } >> -- > .
diff --git a/drivers/pnp/core.c b/drivers/pnp/core.c index 6a60c5d83383..390e449c17ef 100644 --- a/drivers/pnp/core.c +++ b/drivers/pnp/core.c @@ -72,8 +72,10 @@ int pnp_register_protocol(struct pnp_protocol *protocol) mutex_unlock(&pnp_lock); ret = device_register(&protocol->dev); - if (ret) + if (ret) { pnp_remove_protocol(protocol); + kfree_const(protocol->dev.kobj.name); + } return ret; }
After commit 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id string array"), the name of device is allocated dynamically, it need be freed in error path. Current all protocols used in pnp_register_protocol() is static, they don't have a release function(), so just call kfree_const() to free the name. Fixes: 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id string array") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/pnp/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)