2020-06-14 06:00:23

by Navid Emamdoost

Subject: [PATCH] Input: bma150: fix ref count leak in bma150_open

in bma150_open, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost <[email protected]>
---
drivers/input/misc/bma150.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index a9d984da95f3..e2f1b05fcb2a 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -348,7 +348,7 @@ static int bma150_open(struct input_dev *input)

error = pm_runtime_get_sync(&bma150->client->dev);
if (error < 0 && error != -ENOSYS)
- return error;
+ goto out;

/*
* See if runtime PM woke up the device. If runtime PM
@@ -357,10 +357,13 @@ static int bma150_open(struct input_dev *input)
if (bma150->mode != BMA150_MODE_NORMAL) {
error = bma150_set_mode(bma150, BMA150_MODE_NORMAL);
if (error)
- return error;
+ goto out;
}

return 0;
+out:
+ pm_runtime_put(&bma150->client->dev);
+ return error;
}

static void bma150_close(struct input_dev *input)
--
2.17.1

2020-06-14 09:20:15

by Markus Elfring

Subject: Re: [PATCH] Input: bma150: fix ref count leak in bma150_open

> in bma150_open, …

* Can the term “reference count” become relevant also for this commit message
besides other possible adjustments?

* Will the tag “Fixes” become helpful?


…
> +++ b/drivers/input/misc/bma150.c
…
> @@ -357,10 +357,13 @@ static int bma150_open(struct input_dev *input)
> if (bma150->mode != BMA150_MODE_NORMAL) {
> error = bma150_set_mode(bma150, BMA150_MODE_NORMAL);
> if (error)
> - return error;
> + goto out;
> }
>
> return 0;
> +out:
> + pm_runtime_put(&bma150->client->dev);
> + return error;
> }
…

Perhaps use the label “put_runtime” instead?

Regards,
Markus

2020-06-14 09:30:01

by Andy Shevchenko

Subject: Re: [PATCH] Input: bma150: fix ref count leak in bma150_open

On Sun, Jun 14, 2020 at 8:58 AM Navid Emamdoost
<[email protected]> wrote:
>
> in bma150_open, pm_runtime_get_sync is called which
> increments the counter even in case of failure, leading to incorrect
> ref count. In case of failure, decrement the ref count before returning.

...

> error = pm_runtime_get_sync(&bma150->client->dev);
> if (error < 0 && error != -ENOSYS)
> - return error;
> + goto out;

So, what will happen in case of -ENOSYS?

...

> + pm_runtime_put(&bma150->client->dev);

Slightly better to use _put_noidle(). (More consistency with error path)

--
With Best Regards,
Andy Shevchenko

2020-06-15 06:38:05

by Navid Emamdoost

Subject: [PATCH v2] Input: bma150: fix ref count leak in bma150_open

in bma150_open, pm_runtime_get_sync is called which
increments the counter even in case of failure, leading to incorrect
ref count. In case of failure, decrement the ref count before returning.

Signed-off-by: Navid Emamdoost <[email protected]>
---
Changes in v2:
-- repplace pm_runtime_put with pm_runtime_put_noidle
---
drivers/input/misc/bma150.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/input/misc/bma150.c b/drivers/input/misc/bma150.c
index a9d984da95f3..ec394b4faa14 100644
--- a/drivers/input/misc/bma150.c
+++ b/drivers/input/misc/bma150.c
@@ -348,7 +348,7 @@ static int bma150_open(struct input_dev *input)

error = pm_runtime_get_sync(&bma150->client->dev);
if (error < 0 && error != -ENOSYS)
- return error;
+ goto out;

/*
* See if runtime PM woke up the device. If runtime PM
@@ -357,10 +357,13 @@ static int bma150_open(struct input_dev *input)
if (bma150->mode != BMA150_MODE_NORMAL) {
error = bma150_set_mode(bma150, BMA150_MODE_NORMAL);
if (error)
- return error;
+ goto out;
}

return 0;
+out:
+ pm_runtime_put_noidle(&bma150->client->dev);
+ return error;
}

static void bma150_close(struct input_dev *input)
--
2.17.1

2020-06-15 06:39:37

by Navid Emamdoost

Subject: Re: [PATCH] Input: bma150: fix ref count leak in bma150_open

On Sun, Jun 14, 2020 at 4:27 AM Andy Shevchenko
<[email protected]> wrote:
>
> On Sun, Jun 14, 2020 at 8:58 AM Navid Emamdoost
> <[email protected]> wrote:
> >
> > in bma150_open, pm_runtime_get_sync is called which
> > increments the counter even in case of failure, leading to incorrect
> > ref count. In case of failure, decrement the ref count before returning.
>
> ...
>
> > error = pm_runtime_get_sync(&bma150->client->dev);
> > if (error < 0 && error != -ENOSYS)
> > - return error;
> > + goto out;
>
> So, what will happen in case of -ENOSYS?
I'm not sure!

>
> ...
>
> > + pm_runtime_put(&bma150->client->dev);
>
> Slightly better to use _put_noidle(). (More consistency with error path)

v2 is sent.

>
> --
> With Best Regards,
> Andy Shevchenko



--
Navid.

2020-06-15 16:20:54

by Markus Elfring

Subject: Re: [PATCH v2] Input: bma150: fix ref count leak in bma150_open

> in bma150_open, …

* Can the term “reference count” become relevant also for this commit message
besides other possible adjustments?

* Would you like to add the tag “Fixes”?


…
> +++ b/drivers/input/misc/bma150.c
…
> @@ -357,10 +357,13 @@ static int bma150_open(struct input_dev *input)
> if (bma150->mode != BMA150_MODE_NORMAL) {
> error = bma150_set_mode(bma150, BMA150_MODE_NORMAL);
> if (error)
> - return error;
> + goto out;
> }
>
> return 0;
> +out:
> + pm_runtime_put_noidle(&bma150->client->dev);
> + return error;
> }
…

Perhaps use the label “put_runtime” instead?

Regards,
Markus