[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 <andriy.shevchenko@linux.intel.com>
---
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

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
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

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
[off-list ref] 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 <andriy.shevchenko@linux.intel.com>
---
v2: rewrote to return error pointer, align callers (Loic)

Acked-by: Sergey Ryazanov <ryazanov.s.a@gmail.com>

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
[off-list ref] wrote:

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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 <ryazanov.s.a@gmail.com>

On Fri, 6 Aug 2021 at 11:00, Andy Shevchenko
[off-list ref] 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 <andriy.shevchenko@linux.intel.com>

Reviewed-by: Loic Poulain <redacted>

On Fri, 6 Aug 2021 at 11:00, Andy Shevchenko
[off-list ref] wrote:

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

Reviewed-by: Loic Poulain <redacted>

On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov [off-list ref] wrote:

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
[off-list ref] wrote:

quoted

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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 <ryazanov.s.a@gmail.com>

Thanks!

-- 
With Best Regards,
Andy Shevchenko

On Fri, Aug 6, 2021 at 5:20 PM Andy Shevchenko
[off-list ref] wrote:

On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov [off-list ref] wrote:

quoted

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
[off-list ref] wrote:

quoted

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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/20210806085413.61536-1-andriy.shevchenko@linux.intel.com/
2. https://patchwork.kernel.org/project/netdevbpf/patch/20210806085413.61536-2-andriy.shevchenko@linux.intel.com/

-- 
Sergey

On Fri, Aug 06, 2021 at 11:35:04PM +0300, Sergey Ryazanov wrote:

On Fri, Aug 6, 2021 at 5:20 PM Andy Shevchenko
[off-list ref] wrote:

quoted

On Fri, Aug 6, 2021 at 5:14 PM Sergey Ryazanov [off-list ref] wrote:

quoted

On Fri, Aug 6, 2021 at 12:00 PM Andy Shevchenko
[off-list ref] wrote:

quoted

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

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>

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/20210806085413.61536-1-andriy.shevchenko@linux.intel.com/
2. https://patchwork.kernel.org/project/netdevbpf/patch/20210806085413.61536-2-andriy.shevchenko@linux.intel.com/

-- 
With Best Regards,
Andy Shevchenko