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

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

[Gustavo A. R. Silva]

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 <garsilva@embeddedor.com>
---
 drivers/iio/multiplexer/iio-mux.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
index 37ba007..a8d672b 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -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;
 
-- 
2.5.0

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Peter Rosin]

On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
> 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...

> 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

> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> ---
>  drivers/iio/multiplexer/iio-mux.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
> index 37ba007..a8d672b 100644
> --- a/drivers/iio/multiplexer/iio-mux.c
> +++ b/drivers/iio/multiplexer/iio-mux.c
> @@ -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;
>  
> 

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Gustavo A. R. Silva]

Hi Peter,

Quoting Peter Rosin <peda@axentia.se>:

> On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
>> 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.

> And someone should perhaps teach Coccinelle about devm_kmemdup...
>

Good catch, I just implemented that script.

>> 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?
>

mmm, I don't get this...

> Cheers,
> peda
>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> ---
>>  drivers/iio/multiplexer/iio-mux.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/iio/multiplexer/iio-mux.c  
>> b/drivers/iio/multiplexer/iio-mux.c
>> index 37ba007..a8d672b 100644
>> --- a/drivers/iio/multiplexer/iio-mux.c
>> +++ b/drivers/iio/multiplexer/iio-mux.c
>> @@ -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;
>>
>>

Thanks!
--
Gustavo A. R. Silva

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Peter Rosin]

On 2017-07-07 06:35, Gustavo A. R. Silva wrote:
> Hi Peter,
> 
> Quoting Peter Rosin <peda@axentia.se>:
> 
>> On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
>>> 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.
> 
>> And someone should perhaps teach Coccinelle about devm_kmemdup...
>>
> 
> Good catch, I just implemented that script.
> 
>>> 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?
>>
> 
> 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

Re: [PATCH] iio: multiplexer: add NULL check on devm_kzalloc() return value

[Gustavo A. R. Silva]

Quoting Peter Rosin <peda@axentia.se>:

> On 2017-07-07 06:35, Gustavo A. R. Silva wrote:
>> Hi Peter,
>>
>> Quoting Peter Rosin <peda@axentia.se>:
>>
>>> On 2017-07-07 00:08, Gustavo A. R. Silva wrote:
>>>> 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.
>>
>>> And someone should perhaps teach Coccinelle about devm_kmemdup...
>>>
>>
>> Good catch, I just implemented that script.
>>
>>>> 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?
>>>
>>
>> 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.
>

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

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

[Gustavo A. R. Silva]

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 <peda@axentia.se>
Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
---
Changes in v2:
 Add NULL check on devm_kmemup() return value.

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

diff --git a/drivers/iio/multiplexer/iio-mux.c b/drivers/iio/multiplexer/iio-mux.c
index 37ba007..74831fc 100644
--- a/drivers/iio/multiplexer/iio-mux.c
+++ b/drivers/iio/multiplexer/iio-mux.c
@@ -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;
 	}
-- 
2.5.0

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

[Peter Rosin]

On 2017-07-07 06:53, Gustavo A. R. Silva wrote:
> Check return values from call to devm_kzalloc() and devm_kmemup()

If someone cares enough: s/devm_kmemup/evm_kmemdup/


> 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 <peda@axentia.se>
> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>

Either way,

Reviewed-by: Peter Rosin <peda@axentia.se>

Thanks!

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

[Peter Rosin]

On 2017-07-07 06:57, Peter Rosin wrote:
> On 2017-07-07 06:53, Gustavo A. R. Silva wrote:
>> 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 :-)

> 
>> 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 <peda@axentia.se>
>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
> 
> Either way,
> 
> Reviewed-by: Peter Rosin <peda@axentia.se>
> 
> Thanks!
> 

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

[Jonathan Cameron]

On Fri, 7 Jul 2017 11:26:35 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2017-07-07 06:57, Peter Rosin wrote:
> > On 2017-07-07 06:53, Gustavo A. R. Silva wrote:  
> >> 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 :-)
> 
> >   
> >> 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 <peda@axentia.se>
> >> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>  
> > 
> > Either way,
> > 
> > Reviewed-by: Peter Rosin <peda@axentia.se>
> > 
> > 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  
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Gustavo A. R. Silva]

Hi Jonathan,

Quoting Jonathan Cameron <jic23@kernel.org>:

> On Fri, 7 Jul 2017 11:26:35 +0200
> Peter Rosin <peda@axentia.se> wrote:
>
>> On 2017-07-07 06:57, Peter Rosin wrote:
>> > On 2017-07-07 06:53, Gustavo A. R. Silva wrote:
>> >> 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 :-)
>>
>> >
>> >> 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 <peda@axentia.se>
>> >> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>
>> >
>> > Either way,
>> >
>> > Reviewed-by: Peter Rosin <peda@axentia.se>
>> >
>> > 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

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

[Peter Rosin]

On 2017-07-09 19:10, Jonathan Cameron wrote:
> On Fri, 7 Jul 2017 11:26:35 +0200
> Peter Rosin <peda@axentia.se> wrote:
> 
>> On 2017-07-07 06:57, Peter Rosin wrote:
>>> On 2017-07-07 06:53, Gustavo A. R. Silva wrote:  
>>>> 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 :-)
>>
>>>   
>>>> 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 <peda@axentia.se>
>>>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>  
>>>
>>> Either way,
>>>
>>> Reviewed-by: Peter Rosin <peda@axentia.se>
>>>
>>> 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

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

[Jonathan Cameron]

On Sat, 26 Aug 2017 08:09:43 +0200
Peter Rosin <peda@axentia.se> wrote:

> On 2017-07-09 19:10, Jonathan Cameron wrote:
> > On Fri, 7 Jul 2017 11:26:35 +0200
> > Peter Rosin <peda@axentia.se> wrote:
> >   
> >> On 2017-07-07 06:57, Peter Rosin wrote:  
> >>> On 2017-07-07 06:53, Gustavo A. R. Silva wrote:    
> >>>> 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 :-)
> >>  
> >>>     
> >>>> 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 <peda@axentia.se>
> >>>> Signed-off-by: Gustavo A. R. Silva <garsilva@embeddedor.com>    
> >>>
> >>> Either way,
> >>>
> >>> Reviewed-by: Peter Rosin <peda@axentia.se>
> >>>
> >>> 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...
> 

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

Jonathan

> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html