2018-06-28 02:36:00

by Baolin Wang

[permalink] [raw]
Subject: [PATCH] hwspinlock: Fix incorrect return pointers

The commit 4f1acd758b08 ("hwspinlock: Add devm_xxx() APIs to request/free
hwlock") introduces one bug, that will return one error pointer if failed
to request one hwlock, but we expect NULL pointer on error for consumers.
This patch will fix this issue.

Reported-by: Dan Carpenter <[email protected]>
Signed-off-by: Baolin Wang <[email protected]>
---
drivers/hwspinlock/hwspinlock_core.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
index e16d648..2bad40d 100644
--- a/drivers/hwspinlock/hwspinlock_core.c
+++ b/drivers/hwspinlock/hwspinlock_core.c
@@ -877,10 +877,10 @@ struct hwspinlock *devm_hwspin_lock_request(struct device *dev)

ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
if (!ptr)
- return ERR_PTR(-ENOMEM);
+ return NULL;

hwlock = hwspin_lock_request();
- if (!IS_ERR(hwlock)) {
+ if (hwlock) {
*ptr = hwlock;
devres_add(dev, ptr);
} else {
@@ -913,10 +913,10 @@ struct hwspinlock *devm_hwspin_lock_request_specific(struct device *dev,

ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
if (!ptr)
- return ERR_PTR(-ENOMEM);
+ return NULL;

hwlock = hwspin_lock_request_specific(id);
- if (!IS_ERR(hwlock)) {
+ if (hwlock) {
*ptr = hwlock;
devres_add(dev, ptr);
} else {
--
1.7.9.5



2018-07-30 11:35:38

by Baolin Wang

[permalink] [raw]
Subject: Re: [PATCH] hwspinlock: Fix incorrect return pointers

Hi Bjorn,

On 28 June 2018 at 10:32, Baolin Wang <[email protected]> wrote:
> The commit 4f1acd758b08 ("hwspinlock: Add devm_xxx() APIs to request/free
> hwlock") introduces one bug, that will return one error pointer if failed
> to request one hwlock, but we expect NULL pointer on error for consumers.
> This patch will fix this issue.
>
> Reported-by: Dan Carpenter <[email protected]>
> Signed-off-by: Baolin Wang <[email protected]>

Could you pick up this patch which fixes the incorrect return value
issue? Thanks.

> ---
> drivers/hwspinlock/hwspinlock_core.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
> index e16d648..2bad40d 100644
> --- a/drivers/hwspinlock/hwspinlock_core.c
> +++ b/drivers/hwspinlock/hwspinlock_core.c
> @@ -877,10 +877,10 @@ struct hwspinlock *devm_hwspin_lock_request(struct device *dev)
>
> ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
> if (!ptr)
> - return ERR_PTR(-ENOMEM);
> + return NULL;
>
> hwlock = hwspin_lock_request();
> - if (!IS_ERR(hwlock)) {
> + if (hwlock) {
> *ptr = hwlock;
> devres_add(dev, ptr);
> } else {
> @@ -913,10 +913,10 @@ struct hwspinlock *devm_hwspin_lock_request_specific(struct device *dev,
>
> ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
> if (!ptr)
> - return ERR_PTR(-ENOMEM);
> + return NULL;
>
> hwlock = hwspin_lock_request_specific(id);
> - if (!IS_ERR(hwlock)) {
> + if (hwlock) {
> *ptr = hwlock;
> devres_add(dev, ptr);
> } else {
> --
> 1.7.9.5
>



--
Baolin Wang
Best Regards

2018-07-31 04:04:20

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] hwspinlock: Fix incorrect return pointers

On Mon 30 Jul 04:34 PDT 2018, Baolin Wang wrote:

> Hi Bjorn,
>
> On 28 June 2018 at 10:32, Baolin Wang <[email protected]> wrote:
> > The commit 4f1acd758b08 ("hwspinlock: Add devm_xxx() APIs to request/free
> > hwlock") introduces one bug, that will return one error pointer if failed
> > to request one hwlock, but we expect NULL pointer on error for consumers.
> > This patch will fix this issue.
> >
> > Reported-by: Dan Carpenter <[email protected]>
> > Signed-off-by: Baolin Wang <[email protected]>
>
> Could you pick up this patch which fixes the incorrect return value
> issue? Thanks.
>

I thought I had picked this already, it's applied now. Sorry about the
delay.

Regards,
Bjorn

> > ---
> > drivers/hwspinlock/hwspinlock_core.c | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/hwspinlock/hwspinlock_core.c b/drivers/hwspinlock/hwspinlock_core.c
> > index e16d648..2bad40d 100644
> > --- a/drivers/hwspinlock/hwspinlock_core.c
> > +++ b/drivers/hwspinlock/hwspinlock_core.c
> > @@ -877,10 +877,10 @@ struct hwspinlock *devm_hwspin_lock_request(struct device *dev)
> >
> > ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
> > if (!ptr)
> > - return ERR_PTR(-ENOMEM);
> > + return NULL;
> >
> > hwlock = hwspin_lock_request();
> > - if (!IS_ERR(hwlock)) {
> > + if (hwlock) {
> > *ptr = hwlock;
> > devres_add(dev, ptr);
> > } else {
> > @@ -913,10 +913,10 @@ struct hwspinlock *devm_hwspin_lock_request_specific(struct device *dev,
> >
> > ptr = devres_alloc(devm_hwspin_lock_release, sizeof(*ptr), GFP_KERNEL);
> > if (!ptr)
> > - return ERR_PTR(-ENOMEM);
> > + return NULL;
> >
> > hwlock = hwspin_lock_request_specific(id);
> > - if (!IS_ERR(hwlock)) {
> > + if (hwlock) {
> > *ptr = hwlock;
> > devres_add(dev, ptr);
> > } else {
> > --
> > 1.7.9.5
> >
>
>
>
> --
> Baolin Wang
> Best Regards