Re: [PATCH] i2c: core: Fix possible memleak in i2c_new_client_device()

[Yang Yingliang]

Hi,

On 2021/10/17 23:27, Wolfram Sang wrote:
> > In error path after calling i2c_dev_set_name(), the put_device()
> > should be used to give up the device reference, then the name
> > allocated in dev_set_name() will be freed in kobject_cleanup().
> I don't see it. dev_set_name does not call device_get, so why should we
> call device_put on failure? No other user of dev_set_name seems to do
> this. So, if this is an imbalance, where does the unmatched get_device
> really come from?
The reference is initialized in device_initialize() called in 
device_register(), if device_register()
fails, the 'kobj->name' is leaked.

Thanks,
Yang
> > Reported-by: Hulk Robot <hulkci@xxxxxxxxxx>
> > Signed-off-by: Yang Yingliang <yangyingliang@xxxxxxxxxx>
> > ---
> >   drivers/i2c/i2c-core-base.c | 13 +++++++------
> >   1 file changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
> > index 54964fbe3f03..190d4fd5e594 100644
> > --- a/drivers/i2c/i2c-core-base.c
> > +++ b/drivers/i2c/i2c-core-base.c
> > @@ -1047,8 +1047,6 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
> >   	client->dev.of_node = of_node_get(info->of_node);
> >   	client->dev.fwnode = info->fwnode;
> >   
> > -	i2c_dev_set_name(adap, client, info);
> > -
> >   	if (info->swnode) {
> >   		status = device_add_software_node(&client->dev, info->swnode);
> >   		if (status) {
> > @@ -1059,17 +1057,20 @@ i2c_new_client_device(struct i2c_adapter *adap, struct i2c_board_info const *inf
> >   		}
> >   	}
> >   
> > +	i2c_dev_set_name(adap, client, info);
> >   	status = device_register(&client->dev);
> > -	if (status)
> > -		goto out_remove_swnode;
> > +	if (status) {
> > +		device_remove_software_node(&client->dev);
> > +		of_node_put(info->of_node);
> > +		put_device(&client->dev);
> > +		return ERR_PTR(status);
> > +	}
> >   
> >   	dev_dbg(&adap->dev, "client [%s] registered with bus id %s\n",
> >   		client->name, dev_name(&client->dev));
> >   
> >   	return client;
> >   
> > -out_remove_swnode:
> > -	device_remove_software_node(&client->dev);
> >   out_err_put_of_node:
> >   	of_node_put(info->of_node);
> >   out_err:
> > --
> > 2.25.1