Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.

2019-02-07 Thread Matthias Brugger 



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 

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_ZONES8
> +
>  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 */
>

Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.

2019-01-30 Thread Zhang Rui 
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
> >  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  > > > 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 
> 
As this is a bugfix, I will take it and queue it for next -rc.

thanks,
rui
> 
>

Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.

2019-01-30 Thread Daniel Lezcano 
On 30/01/2019 10:25, Pi-Hsun Shih wrote:
> On Wed, Jan 30, 2019 at 3:44 PM Daniel Lezcano
>  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  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 



-- 
  Linaro.org β”‚ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.

2019-01-30 Thread Pi-Hsun Shih 
On Wed, Jan 30, 2019 at 3:44 PM Daniel Lezcano
 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  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 
> >> ---
> >>  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_ZONE1
> >>  #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
> >>
>
>
> --
>   Linaro.org β”‚ Open source software for ARM SoCs
>
> Follow Linaro:   Facebook |
>  Twitter |
>  Blog
>

Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.

2019-01-29 Thread Daniel Lezcano 
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  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 
>> ---
>>  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_ZONE1
>>  #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
>>


-- 
  Linaro.org β”‚ Open source software for ARM SoCs

Follow Linaro:   Facebook |
 Twitter |
 Blog

Re: [PATCH] thermal: mtk: Allocate enough space for mtk_thermal.

2019-01-29 Thread Peter Shih 
Adding Michael Kao to cc list.

On Wed, Jan 9, 2019 at 1:57 PM 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 
> ---
>  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_ZONE1
>  #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
>

[PATCH] thermal: mtk: Allocate enough space for mtk_thermal.

2019-01-08 Thread Pi-Hsun Shih 
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 
---
 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_ZONE1
 #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