* [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.
@ 2019-01-09 5:57 Pi-Hsun Shih
2019-01-30 6:04 ` Peter Shih
2019-02-07 16:25 ` Matthias Brugger
0 siblings, 2 replies; 7+ messages in thread
From: Pi-Hsun Shih @ 2019-01-09 5:57 UTC (permalink / raw)
Cc: Pi-Hsun Shih, Zhang Rui, Eduardo Valentin, Daniel Lezcano,
Matthias Brugger, open list:THERMAL,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support, open list
The mtk_thermal struct contains a 'struct mtk_thermal_bank banks[];',
but the allocation only allocates sizeof(struct mtk_thermal) bytes,
which cause out of bound access with the ->banks[] member. Change it to
a fixed size array instead.
Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
---
drivers/thermal/mtk_thermal.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
index 0691f260f6eabe..ea11edb3fcced6 100644
--- a/drivers/thermal/mtk_thermal.c
+++ b/drivers/thermal/mtk_thermal.c
@@ -159,6 +159,9 @@
#define MT7622_NUM_SENSORS_PER_ZONE 1
#define MT7622_TS1 0
+/* The maximum number of banks */
+#define MAX_NUM_ZONES 8
+
struct mtk_thermal;
struct thermal_bank_cfg {
@@ -178,7 +181,7 @@ struct mtk_thermal_data {
const int *sensor_mux_values;
const int *msr;
const int *adcpnp;
- struct thermal_bank_cfg bank_data[];
+ struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
};
struct mtk_thermal {
@@ -197,7 +200,7 @@ struct mtk_thermal {
s32 vts[MT8173_NUM_SENSORS];
const struct mtk_thermal_data *conf;
- struct mtk_thermal_bank banks[];
+ struct mtk_thermal_bank banks[MAX_NUM_ZONES];
};
/* MT8173 thermal sensor data */
--
2.20.1.97.g81188d93c3-goog
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.
2019-01-09 5:57 [PATCH] thermal: mtk: Allocate enough space for mtk_thermal Pi-Hsun Shih
@ 2019-01-30 6:04 ` Peter Shih
2019-01-30 7:44 ` Daniel Lezcano
2019-02-07 16:25 ` Matthias Brugger
1 sibling, 1 reply; 7+ messages in thread
From: Peter Shih @ 2019-01-30 6:04 UTC (permalink / raw)
Cc: Zhang Rui, Eduardo Valentin, Daniel Lezcano, Matthias Brugger,
open list:THERMAL, moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support, open list, Michael.Kao
Adding Michael Kao to cc list.
On Wed, Jan 9, 2019 at 1:57 PM Pi-Hsun Shih <pihsun@chromium.org> wrote:
>
> The mtk_thermal struct contains a 'struct mtk_thermal_bank banks[];',
> but the allocation only allocates sizeof(struct mtk_thermal) bytes,
> which cause out of bound access with the ->banks[] member. Change it to
> a fixed size array instead.
>
> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> ---
> drivers/thermal/mtk_thermal.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 0691f260f6eabe..ea11edb3fcced6 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -159,6 +159,9 @@
> #define MT7622_NUM_SENSORS_PER_ZONE 1
> #define MT7622_TS1 0
>
> +/* The maximum number of banks */
> +#define MAX_NUM_ZONES 8
> +
> struct mtk_thermal;
>
> struct thermal_bank_cfg {
> @@ -178,7 +181,7 @@ struct mtk_thermal_data {
> const int *sensor_mux_values;
> const int *msr;
> const int *adcpnp;
> - struct thermal_bank_cfg bank_data[];
> + struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
> };
>
> struct mtk_thermal {
> @@ -197,7 +200,7 @@ struct mtk_thermal {
> s32 vts[MT8173_NUM_SENSORS];
>
> const struct mtk_thermal_data *conf;
> - struct mtk_thermal_bank banks[];
> + struct mtk_thermal_bank banks[MAX_NUM_ZONES];
> };
>
> /* MT8173 thermal sensor data */
> --
> 2.20.1.97.g81188d93c3-goog
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.
2019-01-30 6:04 ` Peter Shih
@ 2019-01-30 7:44 ` Daniel Lezcano
2019-01-30 9:25 ` Pi-Hsun Shih
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Lezcano @ 2019-01-30 7:44 UTC (permalink / raw)
To: Peter Shih
Cc: Zhang Rui, Eduardo Valentin, Matthias Brugger, open list:THERMAL,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support, open list, Michael.Kao
On 30/01/2019 07:04, Peter Shih wrote:
> Adding Michael Kao to cc list.
>
> On Wed, Jan 9, 2019 at 1:57 PM Pi-Hsun Shih <pihsun@chromium.org> wrote:
>>
>> The mtk_thermal struct contains a 'struct mtk_thermal_bank banks[];',
>> but the allocation only allocates sizeof(struct mtk_thermal) bytes,
>> which cause out of bound access with the ->banks[] member. Change it to
>> a fixed size array instead.
Even if the fix is correct, it pushes back the bug later in time if a
new board containing more than MAX_NUM_ZONES is introduced. I suggest to
dynamically allocate the array with the 'num_banks' value.
>> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
>> ---
>> drivers/thermal/mtk_thermal.c | 7 +++++--
>> 1 file changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
>> index 0691f260f6eabe..ea11edb3fcced6 100644
>> --- a/drivers/thermal/mtk_thermal.c
>> +++ b/drivers/thermal/mtk_thermal.c
>> @@ -159,6 +159,9 @@
>> #define MT7622_NUM_SENSORS_PER_ZONE 1
>> #define MT7622_TS1 0
>>
>> +/* The maximum number of banks */
>> +#define MAX_NUM_ZONES 8
>> +
>> struct mtk_thermal;
>>
>> struct thermal_bank_cfg {
>> @@ -178,7 +181,7 @@ struct mtk_thermal_data {
>> const int *sensor_mux_values;
>> const int *msr;
>> const int *adcpnp;
>> - struct thermal_bank_cfg bank_data[];
>> + struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
>> };
>>
>> struct mtk_thermal {
>> @@ -197,7 +200,7 @@ struct mtk_thermal {
>> s32 vts[MT8173_NUM_SENSORS];
>>
>> const struct mtk_thermal_data *conf;
>> - struct mtk_thermal_bank banks[];
>> + struct mtk_thermal_bank banks[MAX_NUM_ZONES];
>> };
>>
>> /* MT8173 thermal sensor data */
>> --
>> 2.20.1.97.g81188d93c3-goog
>>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.
2019-01-30 7:44 ` Daniel Lezcano
@ 2019-01-30 9:25 ` Pi-Hsun Shih
2019-01-30 10:04 ` Daniel Lezcano
0 siblings, 1 reply; 7+ messages in thread
From: Pi-Hsun Shih @ 2019-01-30 9:25 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Zhang Rui, Eduardo Valentin, Matthias Brugger, open list:THERMAL,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support, open list, Michael.Kao
On Wed, Jan 30, 2019 at 3:44 PM Daniel Lezcano
<daniel.lezcano@linaro.org> wrote:
>
> On 30/01/2019 07:04, Peter Shih wrote:
> > Adding Michael Kao to cc list.
> >
> > On Wed, Jan 9, 2019 at 1:57 PM Pi-Hsun Shih <pihsun@chromium.org> wrote:
> >>
> >> The mtk_thermal struct contains a 'struct mtk_thermal_bank banks[];',
> >> but the allocation only allocates sizeof(struct mtk_thermal) bytes,
> >> which cause out of bound access with the ->banks[] member. Change it to
> >> a fixed size array instead.
>
> Even if the fix is correct, it pushes back the bug later in time if a
> new board containing more than MAX_NUM_ZONES is introduced. I suggest to
> dynamically allocate the array with the 'num_banks' value.
>
For the current code structure, those mtk_thermal_data are statically declared,
so if there's new board containing more than MAX_NUM_ZONES of bank_data, it
would actually be a compile error.
I'm fine with either way, but feel like that this is simpler than manually
calculating the size needed for allocation.
>
> >> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
> >> ---
> >> drivers/thermal/mtk_thermal.c | 7 +++++--
> >> 1 file changed, 5 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> >> index 0691f260f6eabe..ea11edb3fcced6 100644
> >> --- a/drivers/thermal/mtk_thermal.c
> >> +++ b/drivers/thermal/mtk_thermal.c
> >> @@ -159,6 +159,9 @@
> >> #define MT7622_NUM_SENSORS_PER_ZONE 1
> >> #define MT7622_TS1 0
> >>
> >> +/* The maximum number of banks */
> >> +#define MAX_NUM_ZONES 8
> >> +
> >> struct mtk_thermal;
> >>
> >> struct thermal_bank_cfg {
> >> @@ -178,7 +181,7 @@ struct mtk_thermal_data {
> >> const int *sensor_mux_values;
> >> const int *msr;
> >> const int *adcpnp;
> >> - struct thermal_bank_cfg bank_data[];
> >> + struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
> >> };
> >>
> >> struct mtk_thermal {
> >> @@ -197,7 +200,7 @@ struct mtk_thermal {
> >> s32 vts[MT8173_NUM_SENSORS];
> >>
> >> const struct mtk_thermal_data *conf;
> >> - struct mtk_thermal_bank banks[];
> >> + struct mtk_thermal_bank banks[MAX_NUM_ZONES];
> >> };
> >>
> >> /* MT8173 thermal sensor data */
> >> --
> >> 2.20.1.97.g81188d93c3-goog
> >>
>
>
> --
> <http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
>
> Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
> <http://twitter.com/#!/linaroorg> Twitter |
> <http://www.linaro.org/linaro-blog/> Blog
>
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.
2019-01-30 9:25 ` Pi-Hsun Shih
@ 2019-01-30 10:04 ` Daniel Lezcano
2019-01-30 13:38 ` Zhang Rui
0 siblings, 1 reply; 7+ messages in thread
From: Daniel Lezcano @ 2019-01-30 10:04 UTC (permalink / raw)
To: Pi-Hsun Shih
Cc: Zhang Rui, Eduardo Valentin, Matthias Brugger, open list:THERMAL,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support, open list, Michael.Kao
On 30/01/2019 10:25, Pi-Hsun Shih wrote:
> On Wed, Jan 30, 2019 at 3:44 PM Daniel Lezcano
> <daniel.lezcano@linaro.org> wrote:
>>
>> On 30/01/2019 07:04, Peter Shih wrote:
>>> Adding Michael Kao to cc list.
>>>
>>> On Wed, Jan 9, 2019 at 1:57 PM Pi-Hsun Shih <pihsun@chromium.org> wrote:
>>>>
>>>> The mtk_thermal struct contains a 'struct mtk_thermal_bank banks[];',
>>>> but the allocation only allocates sizeof(struct mtk_thermal) bytes,
>>>> which cause out of bound access with the ->banks[] member. Change it to
>>>> a fixed size array instead.
>>
>> Even if the fix is correct, it pushes back the bug later in time if a
>> new board containing more than MAX_NUM_ZONES is introduced. I suggest to
>> dynamically allocate the array with the 'num_banks' value.
>>
>
> For the current code structure, those mtk_thermal_data are statically declared,
> so if there's new board containing more than MAX_NUM_ZONES of bank_data, it
> would actually be a compile error.
>
> I'm fine with either way, but feel like that this is simpler than manually
> calculating the size needed for allocation.
Right, I missed it can be caught at compile time.
Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.
2019-01-30 10:04 ` Daniel Lezcano
@ 2019-01-30 13:38 ` Zhang Rui
0 siblings, 0 replies; 7+ messages in thread
From: Zhang Rui @ 2019-01-30 13:38 UTC (permalink / raw)
To: Daniel Lezcano, Pi-Hsun Shih
Cc: Eduardo Valentin, Matthias Brugger, open list:THERMAL,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support, open list, Michael.Kao
On 三, 2019-01-30 at 11:04 +0100, Daniel Lezcano wrote:
> On 30/01/2019 10:25, Pi-Hsun Shih wrote:
> >
> > On Wed, Jan 30, 2019 at 3:44 PM Daniel Lezcano
> > <daniel.lezcano@linaro.org> wrote:
> > >
> > >
> > > On 30/01/2019 07:04, Peter Shih wrote:
> > > >
> > > > Adding Michael Kao to cc list.
> > > >
> > > > On Wed, Jan 9, 2019 at 1:57 PM Pi-Hsun Shih <pihsun@chromium.or
> > > > g> wrote:
> > > > >
> > > > >
> > > > > The mtk_thermal struct contains a 'struct mtk_thermal_bank
> > > > > banks[];',
> > > > > but the allocation only allocates sizeof(struct mtk_thermal)
> > > > > bytes,
> > > > > which cause out of bound access with the ->banks[] member.
> > > > > Change it to
> > > > > a fixed size array instead.
> > > Even if the fix is correct, it pushes back the bug later in time
> > > if a
> > > new board containing more than MAX_NUM_ZONES is introduced. I
> > > suggest to
> > > dynamically allocate the array with the 'num_banks' value.
> > >
> > For the current code structure, those mtk_thermal_data are
> > statically declared,
> > so if there's new board containing more than MAX_NUM_ZONES of
> > bank_data, it
> > would actually be a compile error.
> >
> > I'm fine with either way, but feel like that this is simpler than
> > manually
> > calculating the size needed for allocation.
> Right, I missed it can be caught at compile time.
>
> Reviewed-by: Daniel Lezcano <daniel.lezcano@linaro.org>
>
As this is a bugfix, I will take it and queue it for next -rc.
thanks,
rui
>
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.
2019-01-09 5:57 [PATCH] thermal: mtk: Allocate enough space for mtk_thermal Pi-Hsun Shih
2019-01-30 6:04 ` Peter Shih
@ 2019-02-07 16:25 ` Matthias Brugger
1 sibling, 0 replies; 7+ messages in thread
From: Matthias Brugger @ 2019-02-07 16:25 UTC (permalink / raw)
To: Pi-Hsun Shih
Cc: Zhang Rui, Eduardo Valentin, Daniel Lezcano, open list:THERMAL,
moderated list:ARM/Mediatek SoC support,
moderated list:ARM/Mediatek SoC support, open list, stable
On 09/01/2019 06:57, Pi-Hsun Shih wrote:
> The mtk_thermal struct contains a 'struct mtk_thermal_bank banks[];',
> but the allocation only allocates sizeof(struct mtk_thermal) bytes,
> which cause out of bound access with the ->banks[] member. Change it to
> a fixed size array instead.
>
> Signed-off-by: Pi-Hsun Shih <pihsun@chromium.org>
For the next time, please don't forget to provide a fixes tag so that it can get
into stable trees automatically.
For the records, it fixes commit (v4.9):
b7cf0053738c ("thermal: Add Mediatek thermal driver for mt2701.")
> ---
> drivers/thermal/mtk_thermal.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/thermal/mtk_thermal.c b/drivers/thermal/mtk_thermal.c
> index 0691f260f6eabe..ea11edb3fcced6 100644
> --- a/drivers/thermal/mtk_thermal.c
> +++ b/drivers/thermal/mtk_thermal.c
> @@ -159,6 +159,9 @@
> #define MT7622_NUM_SENSORS_PER_ZONE 1
> #define MT7622_TS1 0
>
> +/* The maximum number of banks */
> +#define MAX_NUM_ZONES 8
> +
> struct mtk_thermal;
>
> struct thermal_bank_cfg {
> @@ -178,7 +181,7 @@ struct mtk_thermal_data {
> const int *sensor_mux_values;
> const int *msr;
> const int *adcpnp;
> - struct thermal_bank_cfg bank_data[];
> + struct thermal_bank_cfg bank_data[MAX_NUM_ZONES];
> };
>
> struct mtk_thermal {
> @@ -197,7 +200,7 @@ struct mtk_thermal {
> s32 vts[MT8173_NUM_SENSORS];
>
> const struct mtk_thermal_data *conf;
> - struct mtk_thermal_bank banks[];
> + struct mtk_thermal_bank banks[MAX_NUM_ZONES];
> };
>
> /* MT8173 thermal sensor data */
>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-02-07 16:25 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-01-09 5:57 [PATCH] thermal: mtk: Allocate enough space for mtk_thermal Pi-Hsun Shih
2019-01-30 6:04 ` Peter Shih
2019-01-30 7:44 ` Daniel Lezcano
2019-01-30 9:25 ` Pi-Hsun Shih
2019-01-30 10:04 ` Daniel Lezcano
2019-01-30 13:38 ` Zhang Rui
2019-02-07 16:25 ` Matthias Brugger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).