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(+)
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
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 <[email protected]>
> ---
> 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;
>
>
Hi Peter,
Quoting Peter Rosin <[email protected]>:
> 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 <[email protected]>
>> ---
>> 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
On 2017-07-07 06:35, Gustavo A. R. Silva wrote:
> Hi Peter,
>
> Quoting Peter Rosin <[email protected]>:
>
>> 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
Quoting Peter Rosin <[email protected]>:
> On 2017-07-07 06:35, Gustavo A. R. Silva wrote:
>> Hi Peter,
>>
>> Quoting Peter Rosin <[email protected]>:
>>
>>> 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
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(+)
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
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 <[email protected]>
> Signed-off-by: Gustavo A. R. Silva <[email protected]>
Either way,
Reviewed-by: Peter Rosin <[email protected]>
Thanks!
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 <[email protected]>
>> Signed-off-by: Gustavo A. R. Silva <[email protected]>
>
> Either way,
>
> Reviewed-by: Peter Rosin <[email protected]>
>
> Thanks!
>
On Fri, 7 Jul 2017 11:26:35 +0200
Peter Rosin <[email protected]> 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 <[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
>
> --
> 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
Hi Jonathan,
Quoting Jonathan Cameron <[email protected]>:
> On Fri, 7 Jul 2017 11:26:35 +0200
> Peter Rosin <[email protected]> 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 <[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!
>
OK, I will do that. :)
Thanks!
--
Gustavo A. R. Silva
On 2017-07-09 19:10, Jonathan Cameron wrote:
> On Fri, 7 Jul 2017 11:26:35 +0200
> Peter Rosin <[email protected]> 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 <[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!
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
On Sat, 26 Aug 2017 08:09:43 +0200
Peter Rosin <[email protected]> wrote:
> On 2017-07-09 19:10, Jonathan Cameron wrote:
> > On Fri, 7 Jul 2017 11:26:35 +0200
> > Peter Rosin <[email protected]> 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 <[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!
>
> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html