diff mbox series

HID: vivaldi: fix sysfs attributes leak

Message ID YhmAAjNeTjiNoLlJ@google.com (mailing list archive)
State Mainlined
Commit cc71d37fd1f11e0495b1cf580909ebea37eaa886
Delegated to: Jiri Kosina
Headers show
Series HID: vivaldi: fix sysfs attributes leak | expand

Commit Message

Dmitry Torokhov Feb. 26, 2022, 1:18 a.m. UTC
The driver creates the top row map sysfs attribute in input_configured()
method; unfortunately we do not have a callback that is executed when HID
interface is unbound, thus we are leaking these sysfs attributes, for
example when device is disconnected.

To fix it let's switch to managed version of adding sysfs attributes which
will ensure that they are destroyed when the driver is unbound.

Fixes: 14c9c014babe ("HID: add vivaldi HID driver")
Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
---

Compiled only.

 drivers/hid/hid-vivaldi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Boyd Feb. 26, 2022, 4:55 a.m. UTC | #1
Quoting Dmitry Torokhov (2022-02-25 17:18:58)
> The driver creates the top row map sysfs attribute in input_configured()
> method; unfortunately we do not have a callback that is executed when HID
> interface is unbound, thus we are leaking these sysfs attributes, for
> example when device is disconnected.
>
> To fix it let's switch to managed version of adding sysfs attributes which
> will ensure that they are destroyed when the driver is unbound.
>
> Fixes: 14c9c014babe ("HID: add vivaldi HID driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---

Makes sense

Tested-by: Stephen Boyd <swboyd@chromium.org>
Reviewed-by: Stephen Boyd <swboyd@chromium.org>

will you make a similar change for the hid-google-hammer driver?
Dmitry Torokhov Feb. 26, 2022, 6 a.m. UTC | #2
On Fri, Feb 25, 2022 at 08:55:26PM -0800, Stephen Boyd wrote:
> Quoting Dmitry Torokhov (2022-02-25 17:18:58)
> > The driver creates the top row map sysfs attribute in input_configured()
> > method; unfortunately we do not have a callback that is executed when HID
> > interface is unbound, thus we are leaking these sysfs attributes, for
> > example when device is disconnected.
> >
> > To fix it let's switch to managed version of adding sysfs attributes which
> > will ensure that they are destroyed when the driver is unbound.
> >
> > Fixes: 14c9c014babe ("HID: add vivaldi HID driver")
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> 
> Makes sense
> 
> Tested-by: Stephen Boyd <swboyd@chromium.org>
> Reviewed-by: Stephen Boyd <swboyd@chromium.org>
> 
> will you make a similar change for the hid-google-hammer driver?

Yeah, I was taking a closer look at your series and that was the result
of it.

Thanks.
Jiri Kosina March 1, 2022, 2:31 p.m. UTC | #3
On Fri, 25 Feb 2022, Dmitry Torokhov wrote:

> The driver creates the top row map sysfs attribute in input_configured()
> method; unfortunately we do not have a callback that is executed when HID
> interface is unbound, thus we are leaking these sysfs attributes, for
> example when device is disconnected.
> 
> To fix it let's switch to managed version of adding sysfs attributes which
> will ensure that they are destroyed when the driver is unbound.
> 
> Fixes: 14c9c014babe ("HID: add vivaldi HID driver")
> Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> ---
> 
> Compiled only.
> 
>  drivers/hid/hid-vivaldi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hid/hid-vivaldi.c b/drivers/hid/hid-vivaldi.c
> index efa6140915f4..42ceb2058a09 100644
> --- a/drivers/hid/hid-vivaldi.c
> +++ b/drivers/hid/hid-vivaldi.c
> @@ -144,7 +144,7 @@ static void vivaldi_feature_mapping(struct hid_device *hdev,
>  static int vivaldi_input_configured(struct hid_device *hdev,
>  				    struct hid_input *hidinput)
>  {
> -	return sysfs_create_group(&hdev->dev.kobj, &input_attribute_group);
> +	return devm_device_add_group(&hdev->dev, &input_attribute_group);
>  }
>  
>  static const struct hid_device_id vivaldi_table[] = {

Applied, thanks Dmitry.
Dmitry Torokhov March 8, 2022, 9:13 p.m. UTC | #4
On Tue, Mar 1, 2022 at 6:31 AM Jiri Kosina <jikos@kernel.org> wrote:
>
> On Fri, 25 Feb 2022, Dmitry Torokhov wrote:
>
> > The driver creates the top row map sysfs attribute in input_configured()
> > method; unfortunately we do not have a callback that is executed when HID
> > interface is unbound, thus we are leaking these sysfs attributes, for
> > example when device is disconnected.
> >
> > To fix it let's switch to managed version of adding sysfs attributes which
> > will ensure that they are destroyed when the driver is unbound.
> >
> > Fixes: 14c9c014babe ("HID: add vivaldi HID driver")
> > Signed-off-by: Dmitry Torokhov <dmitry.torokhov@gmail.com>
> > ---
> >
> > Compiled only.
> >
> >  drivers/hid/hid-vivaldi.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hid/hid-vivaldi.c b/drivers/hid/hid-vivaldi.c
> > index efa6140915f4..42ceb2058a09 100644
> > --- a/drivers/hid/hid-vivaldi.c
> > +++ b/drivers/hid/hid-vivaldi.c
> > @@ -144,7 +144,7 @@ static void vivaldi_feature_mapping(struct hid_device *hdev,
> >  static int vivaldi_input_configured(struct hid_device *hdev,
> >                                   struct hid_input *hidinput)
> >  {
> > -     return sysfs_create_group(&hdev->dev.kobj, &input_attribute_group);
> > +     return devm_device_add_group(&hdev->dev, &input_attribute_group);
> >  }
> >
> >  static const struct hid_device_id vivaldi_table[] = {
>
> Applied, thanks Dmitry.

Jiri, are you planning to send this for 5.17 or 5.18?

Thanks.
Jiri Kosina March 9, 2022, 10:42 a.m. UTC | #5
On Tue, 8 Mar 2022, Dmitry Torokhov wrote:

> Jiri, are you planning to send this for 5.17 or 5.18?

Hi Dmitry,

I've sent it to Linus for 5.17-rc still earlier today.
diff mbox series

Patch

diff --git a/drivers/hid/hid-vivaldi.c b/drivers/hid/hid-vivaldi.c
index efa6140915f4..42ceb2058a09 100644
--- a/drivers/hid/hid-vivaldi.c
+++ b/drivers/hid/hid-vivaldi.c
@@ -144,7 +144,7 @@  static void vivaldi_feature_mapping(struct hid_device *hdev,
 static int vivaldi_input_configured(struct hid_device *hdev,
 				    struct hid_input *hidinput)
 {
-	return sysfs_create_group(&hdev->dev.kobj, &input_attribute_group);
+	return devm_device_add_group(&hdev->dev, &input_attribute_group);
 }
 
 static const struct hid_device_id vivaldi_table[] = {