| Message ID | 20230801115105.76690-1-wangzhu9@huawei.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [-next] Input: serio - fix possible memory leak while device_add() fails | expand |
Hi Zhu, On Tue, Aug 01, 2023 at 07:51:05PM +0800, Zhu Wang wrote: > If device_add() returns error, the name allocated by dev_set_name() need > be freed. As comment of device_add() says, it should use put_device() to > release the reference in the error path. So fix this by calling > put_device, then the name can be freed in kobject_cleanp(). Unfortunately this is not as simple. To support serio ports chained behind other serio ports (to accommodate Synaptics touchpads with Trackpoints connected to pass-through ports) serio device registration is done on a thread. So even if you free the device the driver that ultimately issued serio_add_port() call will not be aware of the failure and will continue using the device, which will much more likely crash the kernel as opposed to leaving the device structure around. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Zhu Wang <wangzhu9@huawei.com> > --- > drivers/input/serio/serio.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c > index 767fc9efb4a8..d3bb6ec91326 100644 > --- a/drivers/input/serio/serio.c > +++ b/drivers/input/serio/serio.c > @@ -537,10 +537,12 @@ static void serio_add_port(struct serio *serio) > serio->start(serio); > > error = device_add(&serio->dev); > - if (error) > + if (error) { > + put_device(&serio->dev); > dev_err(&serio->dev, > "device_add() failed for %s (%s), error: %d\n", > serio->phys, serio->name, error); > + } > } > > /* > -- > 2.17.1 > Thanks.
diff --git a/drivers/input/serio/serio.c b/drivers/input/serio/serio.c index 767fc9efb4a8..d3bb6ec91326 100644 --- a/drivers/input/serio/serio.c +++ b/drivers/input/serio/serio.c @@ -537,10 +537,12 @@ static void serio_add_port(struct serio *serio) serio->start(serio); error = device_add(&serio->dev); - if (error) + if (error) { + put_device(&serio->dev); dev_err(&serio->dev, "device_add() failed for %s (%s), error: %d\n", serio->phys, serio->name, error); + } } /*
If device_add() returns error, the name allocated by dev_set_name() need be freed. As comment of device_add() says, it should use put_device() to release the reference in the error path. So fix this by calling put_device, then the name can be freed in kobject_cleanp(). Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Zhu Wang <wangzhu9@huawei.com> --- drivers/input/serio/serio.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)