iio: multiplexer: add NULL check on devm_kzalloc() return value

PATCH In review Gustavo A. R. Silva 3 participants 12 messages Jul 6, 2017 – Sep 3, 2017

Subsystems (2)

IIO IIO multiplexer

lore

Share & tools

lore

Gustavo A. R. Silva

author

2017-07-06 22:09 UTC #0 lore raw

PATCH 1/1 iio: multiplexer: add NULL check on devm_kzalloc() return value

Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.

This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
... when != x == NULL
x->fld

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/iio/multiplexer/iio-mux.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/iio/multiplexer/iio-mux.c +3 -0

index 37ba007..a8d672b 100644
@@ -285,6 +285,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
 	child->ext_info_cache = devm_kzalloc(dev,
 					     sizeof(*child->ext_info_cache) *
 					     num_ext_info, GFP_KERNEL);
+	if (!child->ext_info_cache)
+		return -ENOMEM;
+
 	for (i = 0; i < num_ext_info; ++i) {
 		child->ext_info_cache[i].size = -1;

Signature · 1 line

2.5.0

Peter Rosin

2017-07-07 04:07 UTC #1 lore raw

Quoted 2 lines · Gustavo A. R. Silva

Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.

Right, thanks for finding that one! There's another one inside the for loop that is just starting in the context of this patch. Care to fix checking the return value of that devm_kmemdup as well?

And someone should perhaps teach Coccinelle about devm_kmemdup...

Quoted 12 lines

This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@
* x = devm_kzalloc(...);
... when != x == NULL
x->fld

One of these blank lines should perhaps be a "Fixes:" tag?

Cheers,
peda

Quoted 20 lines

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
 drivers/iio/multiplexer/iio-mux.c | 3 +++
 1 file changed, 3 insertions(+)

drivers/iio/multiplexer/iio-mux.c +3 -0

index 37ba007..a8d672b 100644
@@ -285,6 +285,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
 	child->ext_info_cache = devm_kzalloc(dev,
 					     sizeof(*child->ext_info_cache) *
 					     num_ext_info, GFP_KERNEL);
+	if (!child->ext_info_cache)
+		return -ENOMEM;
+
 	for (i = 0; i < num_ext_info; ++i) {
 		child->ext_info_cache[i].size = -1;

Peter Rosin

2017-07-07 04:46 UTC #3 lore raw

Quoted 33 lines · Gustavo A. R. Silva

Hi Peter,
Quoting Peter Rosin <[email protected]>:
Quoted 8 lines · Gustavo A. R. Silva
Quoted 2 lines
Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.
Right, thanks for finding that one! There's another one inside the for loop that is just starting in the context of this patch. Care to fix checking the return value of that devm_kmemdup as well?
Sure, I'll send a new patch shortly.
Quoted 2 lines
And someone should perhaps teach Coccinelle about devm_kmemdup...
Good catch, I just implemented that script.
Quoted 13 lines
Quoted 12 lines
This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@
* x = devm_kzalloc(...);
... when != x == NULL
x->fld

Quoted 4 lines

Quoted 2 lines
One of these blank lines should perhaps be a "Fixes:" tag?
mmm, I don't get this...

If you add a Fixes-tag, like below, you help the stable kernel maintainers decide what to look at. In this case it might be overkill since the thing you fix is so fresh and does not apply to any old kernel. But I think it is a good habit...

Fixes 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver")

(and it is a bit unusual to see two blank lines before the SoB-tag)

Sorry for not spelling it out the first time.

Cheers,
peda

Gustavo A. R. Silva

author

2017-07-07 04:51 UTC #4 lore raw

Quoting Peter Rosin <[email protected]>:

Quoted 44 lines · Gustavo A. R. Silva

Quoted 33 lines
Hi Peter,
Quoting Peter Rosin <[email protected]>:
Quoted 8 lines · Gustavo A. R. Silva
Quoted 2 lines
Check return value from call to devm_kzalloc()
in order to prevent a NULL pointer dereference.
Right, thanks for finding that one! There's another one inside the for loop that is just starting in the context of this patch. Care to fix checking the return value of that devm_kmemdup as well?
Sure, I'll send a new patch shortly.
Quoted 2 lines
And someone should perhaps teach Coccinelle about devm_kmemdup...
Good catch, I just implemented that script.
Quoted 13 lines
Quoted 12 lines
This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@
* x = devm_kzalloc(...);
... when != x == NULL
x->fld
Quoted 4 lines
Quoted 2 lines
One of these blank lines should perhaps be a "Fixes:" tag?
mmm, I don't get this...
If you add a Fixes-tag, like below, you help the stable kernel maintainers decide what to look at. In this case it might be overkill since the thing you fix is so fresh and does not apply to any old kernel. But I think it is a good habit...

Quoted 6 lines

Fixes 7ba9df54b091 ("iio: multiplexer: new iio category and iio-mux driver")
(and it is a bit unusual to see two blank lines before the SoB-tag)
Sorry for not spelling it out the first time.

Oh, I get it now. Thank you very much for clarifying. I will definitely keep that in mind for future fixes when that could apply.

Thanks!
--
Gustavo A. R. Silva

Gustavo A. R. Silva

author

2017-07-07 04:53 UTC #5 lore raw

PATCH 1/1 iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

Check return values from call to devm_kzalloc() and devm_kmemup() in order to prevent a NULL pointer dereference.

This issue was detected using Coccinelle and the following semantic patch:

@@
expression x;
identifier fld;
@@

* x = devm_kzalloc(...);
... when != x == NULL
x->fld

Cc Peter Rosin <[email protected]>

Signed-off-by Gustavo A. R. Silva <[email protected]>

---
Changes in v2:
Add NULL check on devm_kmemup() return value.

 drivers/iio/multiplexer/iio-mux.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/iio/multiplexer/iio-mux.c +6 -0

index 37ba007..74831fc 100644
@@ -285,6 +285,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
 	child->ext_info_cache = devm_kzalloc(dev,
 					     sizeof(*child->ext_info_cache) *
 					     num_ext_info, GFP_KERNEL);
+	if (!child->ext_info_cache)
+		return -ENOMEM;
+
 	for (i = 0; i < num_ext_info; ++i) {
 		child->ext_info_cache[i].size = -1;

@@ -309,6 +312,9 @@ static int mux_configure_channel(struct device *dev, struct mux *mux,
 
 		child->ext_info_cache[i].data = devm_kmemdup(dev, page, ret + 1,
 							     GFP_KERNEL);
+		if (!child->ext_info_cache[i].data)
+			return -ENOMEM;
+
 		child->ext_info_cache[i].data[ret] = 0;
 		child->ext_info_cache[i].size = ret;
 	}

Signature · 1 line

2.5.0

Peter Rosin

Reviewed-by on v1 1/1

2017-07-07 04:57 UTC #6 lore raw

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

Quoted 1 line · Gustavo A. R. Silva

Check return values from call to devm_kzalloc() and devm_kmemup()

If someone cares enough: s/devm_kmemup/evm_kmemdup/

Quoted 9 lines

in order to prevent a NULL pointer dereference.
This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@

Quoted 6 lines

* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld
Cc Peter Rosin <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>

Either way,

Reviewed-by Peter Rosin <[email protected]>

Thanks!

Peter Rosin

2017-07-07 09:26 UTC #7 lore raw

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

Quoted 4 lines · Peter Rosin

Quoted 1 line · Gustavo A. R. Silva
Check return values from call to devm_kzalloc() and devm_kmemup()
If someone cares enough: s/devm_kmemup/evm_kmemdup/

Strange, there seems to be some inherent shortage of 'd' characters...

Cheers,
pea :-)

Quoted 23 lines

Quoted 10 lines
in order to prevent a NULL pointer dereference.
This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@
Quoted 6 lines
* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld
Cc Peter Rosin <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
Either way,
Reviewed-by Peter Rosin <[email protected]>
Thanks!

Jonathan Cameron

maintainer

2017-07-09 17:10 UTC #8 lore raw

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

Quoted 34 lines · On Fri, 7 Jul 2017 11:26:35 +0200 Peter Rosin <[email protected]> wrote:

Quoted 4 lines · Peter Rosin
Quoted 1 line · Gustavo A. R. Silva
Check return values from call to devm_kzalloc() and devm_kmemup()
If someone cares enough: s/devm_kmemup/evm_kmemdup/
Strange, there seems to be some inherent shortage of 'd' characters...
Cheers,
pea :-)
Quoted 23 lines
Quoted 10 lines
in order to prevent a NULL pointer dereference.
This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@
Quoted 6 lines
* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld
Cc Peter Rosin <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
Either way,
Reviewed-by Peter Rosin <[email protected]>
Thanks!

The relevant patch adding the file in questions hasn't yet worked it's way back to the iio tree so I can't apply this until it does.

That is likely to be a few weeks away yet. Please give me a poke if I seem to have forgotten it!

Jonathan

Quoted 5 lines

--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Gustavo A. R. Silva

author

2017-07-10 19:44 UTC #9 lore raw

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

Hi Jonathan,

Quoting Jonathan Cameron <[email protected]>:

Quoted 36 lines

Quoted 33 lines · On Fri, 7 Jul 2017 11:26:35 +0200 Peter Rosin <[email protected]> wrote:
Quoted 4 lines · Peter Rosin
Quoted 1 line · Gustavo A. R. Silva
Check return values from call to devm_kzalloc() and devm_kmemup()
If someone cares enough: s/devm_kmemup/evm_kmemdup/
Strange, there seems to be some inherent shortage of 'd' characters...
Cheers,
pea :-)
Quoted 4 lines
Quoted 4 lines
in order to prevent a NULL pointer dereference.
This issue was detected using Coccinelle and the following
semantic patch:
Quoted 17 lines
Quoted 6 lines
@@
expression x;
identifier fld;
@@
Quoted 6 lines
* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld
Cc Peter Rosin <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
Either way,
Reviewed-by Peter Rosin <[email protected]>

Quoted 8 lines

Quoted 2 lines
Quoted 2 lines
Thanks!
The relevant patch adding the file in questions hasn't yet worked it's way back to the iio tree so I can't apply this until it does.
That is likely to be a few weeks away yet. Please give me a poke if I seem to have forgotten it!

OK, I will do that. :)

Thanks!
--
Gustavo A. R. Silva

Peter Rosin

2017-08-26 06:09 UTC #10 lore raw

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

In reply to Jonathan Cameron

Quoted 35 lines · Jonathan Cameron

Quoted 32 lines · On Fri, 7 Jul 2017 11:26:35 +0200 Peter Rosin <[email protected]> wrote:
Quoted 4 lines · Peter Rosin
Quoted 1 line · Gustavo A. R. Silva
Check return values from call to devm_kzalloc() and devm_kmemup()
If someone cares enough: s/devm_kmemup/evm_kmemdup/
Strange, there seems to be some inherent shortage of 'd' characters...
Cheers,
pea :-)
Quoted 21 lines
Quoted 10 lines
in order to prevent a NULL pointer dereference.
This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@
Quoted 6 lines
* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld
Cc Peter Rosin <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
Either way,
Reviewed-by Peter Rosin <[email protected]>

Quoted 7 lines

Quoted 2 lines
Quoted 2 lines
Thanks!
The relevant patch adding the file in questions hasn't yet worked it's way back to the iio tree so I can't apply this until it does.
That is likely to be a few weeks away yet. Please give me a poke if I seem to have forgotten it!

Hi Jonathan!

I saw a patch from Christophe JAILLET [1] and it seemed familiar so I did a bit of digging. Now, I did go back and check this a couple of weeks after you request, but iio-mux.c still wasn't in the iio tree, and then I did forgot about it. Sorry. But here's the poke...

Cheers,
Peter

[1] https://lkml.org/lkml/2017/8/26/3

Jonathan Cameron

maintainer Applied

2017-09-03 12:13 UTC #11 lore raw

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() and devm_kmemdup() return values

Quoted 44 lines · On Sat, 26 Aug 2017 08:09:43 +0200 Peter Rosin <[email protected]> wrote:

Quoted 35 lines · Jonathan Cameron
Quoted 32 lines · On Fri, 7 Jul 2017 11:26:35 +0200 Peter Rosin <[email protected]> wrote:
Quoted 4 lines · Peter Rosin
Quoted 1 line · Gustavo A. R. Silva
Check return values from call to devm_kzalloc() and devm_kmemup()
If someone cares enough: s/devm_kmemup/evm_kmemdup/
Strange, there seems to be some inherent shortage of 'd' characters...
Cheers,
pea :-)
Quoted 21 lines
Quoted 10 lines
in order to prevent a NULL pointer dereference.
This issue was detected using Coccinelle and the following semantic patch:
@@
expression x;
identifier fld;
@@
Quoted 6 lines
* x = devm_kzalloc(...);
   ... when != x == NULL
   x->fld
Cc Peter Rosin <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
Either way,
Reviewed-by Peter Rosin <[email protected]>
Quoted 7 lines
Quoted 2 lines
Quoted 2 lines
Thanks!
The relevant patch adding the file in questions hasn't yet worked it's way back to the iio tree so I can't apply this until it does.
That is likely to be a few weeks away yet. Please give me a poke if I seem to have forgotten it!

Quoted 7 lines

Hi Jonathan!
I saw a patch from Christophe JAILLET [1] and it seemed familiar so I did a bit of digging. Now, I did go back and check this a couple of weeks after you request, but iio-mux.c still wasn't in the iio tree, and then I did forgot about it. Sorry. But here's the poke...

Applied. Thanks - this one had completely dropped off the back of my patch queue. Oops.

Jonathan

Quoted 8 lines

Cheers,
Peter
[1] https://lkml.org/lkml/2017/8/26/3
--
To unsubscribe from this list: send the line "unsubscribe linux-iio" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html