* [PATCH] Input: bma150: fix ref count leak in bma150_open
@ 2020-06-14 5:56 Navid Emamdoost
2020-06-14 9:27 ` Andy Shevchenko
0 siblings, 1 reply; 5+ messages in thread
From: Navid Emamdoost @ 2020-06-14 5:56 UTC (permalink / raw)
To: Dmitry Torokhov, Navid Emamdoost, Jonathan Bakker,
Paweł Chmiel, linux-input, linux-kernel
Cc: emamd001, wu000273, kjlu, smccaman
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 <navid.emamdoost@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Input: bma150: fix ref count leak in bma150_open
2020-06-14 5:56 [PATCH] Input: bma150: fix ref count leak in bma150_open Navid Emamdoost
@ 2020-06-14 9:27 ` Andy Shevchenko
2020-06-15 6:35 ` [PATCH v2] " Navid Emamdoost
2020-06-15 6:36 ` [PATCH] " Navid Emamdoost
0 siblings, 2 replies; 5+ messages in thread
From: Andy Shevchenko @ 2020-06-14 9:27 UTC (permalink / raw)
To: Navid Emamdoost
Cc: Dmitry Torokhov, Jonathan Bakker, Paweł Chmiel, linux-input,
Linux Kernel Mailing List, Navid Emamdoost, wu000273, Kangjie Lu,
Stephen McCamant
On Sun, Jun 14, 2020 at 8:58 AM Navid Emamdoost
<navid.emamdoost@gmail.com> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] Input: bma150: fix ref count leak in bma150_open
2020-06-14 9:27 ` Andy Shevchenko
@ 2020-06-15 6:35 ` Navid Emamdoost
2020-06-15 6:36 ` [PATCH] " Navid Emamdoost
1 sibling, 0 replies; 5+ messages in thread
From: Navid Emamdoost @ 2020-06-15 6:35 UTC (permalink / raw)
To: Dmitry Torokhov, Navid Emamdoost, Paweł Chmiel,
Jonathan Bakker, linux-input, linux-kernel
Cc: emamd001, wu000273, kjlu, mccamant, andy.shevchenko
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 <navid.emamdoost@gmail.com>
---
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] Input: bma150: fix ref count leak in bma150_open
2020-06-14 9:27 ` Andy Shevchenko
2020-06-15 6:35 ` [PATCH v2] " Navid Emamdoost
@ 2020-06-15 6:36 ` Navid Emamdoost
1 sibling, 0 replies; 5+ messages in thread
From: Navid Emamdoost @ 2020-06-15 6:36 UTC (permalink / raw)
To: Andy Shevchenko
Cc: Dmitry Torokhov, Jonathan Bakker, Paweł Chmiel, linux-input,
Linux Kernel Mailing List, Navid Emamdoost, Qiushi Wu,
Kangjie Lu, Stephen McCamant
On Sun, Jun 14, 2020 at 4:27 AM Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
>
> On Sun, Jun 14, 2020 at 8:58 AM Navid Emamdoost
> <navid.emamdoost@gmail.com> 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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] Input: bma150: fix ref count leak in bma150_open
@ 2020-06-14 9:17 Markus Elfring
0 siblings, 0 replies; 5+ messages in thread
From: Markus Elfring @ 2020-06-14 9:17 UTC (permalink / raw)
To: Navid Emamdoost, linux-input
Cc: Navid Emamdoost, Kangjie Lu, Stephen McCamant, Qiushi Wu,
Dmitry Torokhov, Jonathan Bakker, Paweł Chmiel,
kernel-janitors, linux-kernel
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-06-15 6:36 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-14 5:56 [PATCH] Input: bma150: fix ref count leak in bma150_open Navid Emamdoost
2020-06-14 9:27 ` Andy Shevchenko
2020-06-15 6:35 ` [PATCH v2] " Navid Emamdoost
2020-06-15 6:36 ` [PATCH] " Navid Emamdoost
2020-06-14 9:17 Markus Elfring
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).