2021-08-06 09:03:02

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v2 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()

Make wwan_create_dev() to return either valid or error pointer,
In some cases it may return NULL. Prevent this by converting
it to the respective error pointer.

Fixes: 9a44c1cc6388 ("net: Add a WWAN subsystem")
Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: rewrote to return error pointer, align callers (Loic)
drivers/net/wwan/wwan_core.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 674a81d79db3..35ece98134c0 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -164,11 +164,14 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
goto done_unlock;

id = ida_alloc(&wwan_dev_ids, GFP_KERNEL);
- if (id < 0)
+ if (id < 0) {
+ wwandev = ERR_PTR(id);
goto done_unlock;
+ }

wwandev = kzalloc(sizeof(*wwandev), GFP_KERNEL);
if (!wwandev) {
+ wwandev = ERR_PTR(-ENOMEM);
ida_free(&wwan_dev_ids, id);
goto done_unlock;
}
@@ -182,7 +185,8 @@ static struct wwan_device *wwan_create_dev(struct device *parent)
err = device_register(&wwandev->dev);
if (err) {
put_device(&wwandev->dev);
- wwandev = NULL;
+ wwandev = ERR_PTR(err);
+ goto done_unlock;
}

done_unlock:
@@ -1014,8 +1018,8 @@ int wwan_register_ops(struct device *parent, const struct wwan_ops *ops,
return -EINVAL;

wwandev = wwan_create_dev(parent);
- if (!wwandev)
- return -ENOMEM;
+ if (IS_ERR(wwandev))
+ return PTR_ERR(wwandev);

if (WARN_ON(wwandev->ops)) {
wwan_remove_dev(wwandev);
--
2.30.2


2021-08-06 09:03:52

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

ida_alloc_range)) may return other than -ENOMEM error code.
Unshadow it in the wwan_create_port().

Signed-off-by: Andy Shevchenko <[email protected]>
---
v2: new patch
drivers/net/wwan/wwan_core.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wwan/wwan_core.c b/drivers/net/wwan/wwan_core.c
index 35ece98134c0..d293ab688044 100644
--- a/drivers/net/wwan/wwan_core.c
+++ b/drivers/net/wwan/wwan_core.c
@@ -359,8 +359,8 @@ struct wwan_port *wwan_create_port(struct device *parent,
{
struct wwan_device *wwandev;
struct wwan_port *port;
- int minor, err = -ENOMEM;
char namefmt[0x20];
+ int minor, err;

if (type > WWAN_PORT_MAX || !ops)
return ERR_PTR(-EINVAL);
@@ -374,11 +374,14 @@ struct wwan_port *wwan_create_port(struct device *parent,

/* A port is exposed as character device, get a minor */
minor = ida_alloc_range(&minors, 0, WWAN_MAX_MINORS - 1, GFP_KERNEL);
- if (minor < 0)
+ if (minor < 0) {
+ err = minor;
goto error_wwandev_remove;
+ }

port = kzalloc(sizeof(*port), GFP_KERNEL);
if (!port) {
+ err = -ENOMEM;
ida_free(&minors, minor);
goto error_wwandev_remove;
}
--
2.30.2

2021-08-06 09:17:02

by Sergey Ryazanov

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
<[email protected]> wrote:
> Make wwan_create_dev() to return either valid or error pointer,
> In some cases it may return NULL. Prevent this by converting
> it to the respective error pointer.
>
> Fixes: 9a44c1cc6388 ("net: Add a WWAN subsystem")
> Signed-off-by: Andy Shevchenko <[email protected]>
> ---
> v2: rewrote to return error pointer, align callers (Loic)

Acked-by: Sergey Ryazanov <[email protected]>

2021-08-06 09:21:37

by Loic Poulain

[permalink] [raw]
Subject: Re: [PATCH v2 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()

On Fri, 6 Aug 2021 at 11:00, Andy Shevchenko
<[email protected]> wrote:
>
> Make wwan_create_dev() to return either valid or error pointer,
> In some cases it may return NULL. Prevent this by converting
> it to the respective error pointer.
>
> Fixes: 9a44c1cc6388 ("net: Add a WWAN subsystem")
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Loic Poulain <[email protected]>

2021-08-06 14:14:30

by Sergey Ryazanov

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
<[email protected]> wrote:
> ida_alloc_range)) may return other than -ENOMEM error code.
> Unshadow it in the wwan_create_port().
>
> Signed-off-by: Andy Shevchenko <[email protected]>

A small nitpick, looks like "ida_alloc_range))" in the description is
a typo and should be "ida_alloc_range()". Besides this:

Reviewed-by: Sergey Ryazanov <[email protected]>

2021-08-06 15:03:01

by Loic Poulain

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

On Fri, 6 Aug 2021 at 11:00, Andy Shevchenko
<[email protected]> wrote:
>
> ida_alloc_range)) may return other than -ENOMEM error code.
> Unshadow it in the wwan_create_port().
>
> Signed-off-by: Andy Shevchenko <[email protected]>

Reviewed-by: Loic Poulain <[email protected]>

2021-08-06 20:49:18

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov <[email protected]> wrote:
> On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
> <[email protected]> wrote:
> > ida_alloc_range)) may return other than -ENOMEM error code.
> > Unshadow it in the wwan_create_port().
> >
> > Signed-off-by: Andy Shevchenko <[email protected]>
>
> A small nitpick, looks like "ida_alloc_range))" in the description is
> a typo and should be "ida_alloc_range()". Besides this:

Shall I resend?

> Reviewed-by: Sergey Ryazanov <[email protected]>

Thanks!

--
With Best Regards,
Andy Shevchenko

2021-08-07 00:13:18

by Sergey Ryazanov

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

On Fri, Aug 6, 2021 at 5:20 PM Andy Shevchenko
<[email protected]> wrote:
> On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov <[email protected]> wrote:
>> On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
>> <[email protected]> wrote:
>>> ida_alloc_range)) may return other than -ENOMEM error code.
>>> Unshadow it in the wwan_create_port().
>>>
>>> Signed-off-by: Andy Shevchenko <[email protected]>
>>
>> A small nitpick, looks like "ida_alloc_range))" in the description is
>> a typo and should be "ida_alloc_range()". Besides this:
>
> Shall I resend?

Yes, please. And specify the target tree in the subject, please. See
patchwork warning [1, 2]. The first patch is a clear bug fix, so it
should be targeted to the 'net' tree, while the second patch despite
its usefulness could not be considered a bug fix, so it should be
targeted to the 'net-next' tree. Subjects could be like this:

[PATCHv3 net 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()
[PATCHv3 net-next 2/2] wwan: core: Unshadow error code returned by
ida_alloc_range()

Or since the second patch is not depends on the first one and patches
target different trees, patches could be submitted independently:

[PATCHv3 net] wwan: core: Avoid returning NULL from wwan_create_dev()
[PATCHv3 net-next] wwan: core: Unshadow error code returned by ida_alloc_range()




1. https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
2. https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

--
Sergey

2021-08-11 12:53:09

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] wwan: core: Unshadow error code returned by ida_alloc_range))

On Fri, Aug 06, 2021 at 11:35:04PM +0300, Sergey Ryazanov wrote:
> On Fri, Aug 6, 2021 at 5:20 PM Andy Shevchenko
> <[email protected]> wrote:
> > On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov <[email protected]> wrote:
> >> On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
> >> <[email protected]> wrote:
> >>> ida_alloc_range)) may return other than -ENOMEM error code.
> >>> Unshadow it in the wwan_create_port().
> >>>
> >>> Signed-off-by: Andy Shevchenko <[email protected]>
> >>
> >> A small nitpick, looks like "ida_alloc_range))" in the description is
> >> a typo and should be "ida_alloc_range()". Besides this:
> >
> > Shall I resend?
>
> Yes, please. And specify the target tree in the subject, please. See
> patchwork warning [1, 2]. The first patch is a clear bug fix, so it
> should be targeted to the 'net' tree, while the second patch despite
> its usefulness could not be considered a bug fix, so it should be
> targeted to the 'net-next' tree. Subjects could be like this:
>
> [PATCHv3 net 1/2] wwan: core: Avoid returning NULL from wwan_create_dev()
> [PATCHv3 net-next 2/2] wwan: core: Unshadow error code returned by
> ida_alloc_range()
>
> Or since the second patch is not depends on the first one and patches
> target different trees, patches could be submitted independently:
>
> [PATCHv3 net] wwan: core: Avoid returning NULL from wwan_create_dev()
> [PATCHv3 net-next] wwan: core: Unshadow error code returned by ida_alloc_range()

Split and sent separately, thanks!

> 1. https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
> 2. https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/

--
With Best Regards,
Andy Shevchenko