| Message ID | 20230602085546.376086-1-d.dulov@aladdin.ru (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | thermal: intel_powerclamp: Check for a possible array out of bounds access. | expand |
Cc list trimmed. On Fri, Jun 2, 2023 at 11:12 AM Daniil Dulov <d.dulov@aladdin.ru> wrote: > > ratio may be equal to MAX_TARGET_RATIO - 1 that will result in > out of bound access. > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > Fixes: d6d71ee4a14a ("PM: Introduce Intel PowerClamp Driver") > Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru> > --- > drivers/thermal/intel/intel_powerclamp.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c > index fb04470d7d4b..9deaf2b8ccf6 100644 > --- a/drivers/thermal/intel/intel_powerclamp.c > +++ b/drivers/thermal/intel/intel_powerclamp.c > @@ -277,7 +277,8 @@ static unsigned int get_compensation(int ratio) > comp = (cal_data[ratio].steady_comp + > cal_data[ratio - 1].steady_comp + > cal_data[ratio - 2].steady_comp) / 3; > - } else if (cal_data[ratio].confidence >= CONFIDENCE_OK && > + } else if (ratio < MAX_TARGET_RATIO - 1 && > + cal_data[ratio].confidence >= CONFIDENCE_OK && > cal_data[ratio - 1].confidence >= CONFIDENCE_OK && > cal_data[ratio + 1].confidence >= CONFIDENCE_OK) { > comp = (cal_data[ratio].steady_comp + > -- Rui, Srinivas, can you have a look at this, please?
On Sun, 2023-06-04 at 18:13 +0200, Rafael J. Wysocki wrote: > Cc list trimmed. > > On Fri, Jun 2, 2023 at 11:12 AM Daniil Dulov <d.dulov@aladdin.ru> > wrote: > > > > ratio may be equal to MAX_TARGET_RATIO - 1 that will result in > > out of bound access. > > The description was not clear to me. May be something like this: " The max value of "ratio" parameter passed to the function get_compensation() is MAX_TARGET_RATIO - 1. The size of cal_data array is MAX_TARGET_RATIO. Hence, accessing cal_data[ratio + 1], will result in out of bound access. Add a condition to check ratio < MAX_TARGET_RATIO - 1, before accsssing cal_data[ratio + 1]. " The change is correct. But for actual code change, the ratio < MAX_TARGET_RATIO - 1 is also checked in else if() before, which can be merged to check only once for this condition. Thanks, Srinivas > > Found by Linux Verification Center (linuxtesting.org) with SVACE. > > > > Fixes: d6d71ee4a14a ("PM: Introduce Intel PowerClamp Driver") > > Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru> > > --- > > drivers/thermal/intel/intel_powerclamp.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/thermal/intel/intel_powerclamp.c > > b/drivers/thermal/intel/intel_powerclamp.c > > index fb04470d7d4b..9deaf2b8ccf6 100644 > > --- a/drivers/thermal/intel/intel_powerclamp.c > > +++ b/drivers/thermal/intel/intel_powerclamp.c > > @@ -277,7 +277,8 @@ static unsigned int get_compensation(int ratio) > > comp = (cal_data[ratio].steady_comp + > > cal_data[ratio - 1].steady_comp + > > cal_data[ratio - 2].steady_comp) / 3; > > - } else if (cal_data[ratio].confidence >= CONFIDENCE_OK && I think the concern is that cal_data[ratio + 1] is going out of bound for the "ratio" passed to this function, when ratio == ARRAY_SIZE(cal_data) - 1; Here size of cal_data array is 50. The max possible "ratio" passed can be 49. > > + } else if (ratio < MAX_TARGET_RATIO - 1 && > > + cal_data[ratio].confidence >= CONFIDENCE_OK && > > cal_data[ratio - 1].confidence >= CONFIDENCE_OK && > > cal_data[ratio + 1].confidence >= CONFIDENCE_OK) { > > comp = (cal_data[ratio].steady_comp + > > -- > > Rui, Srinivas, can you have a look at this, please?
diff --git a/drivers/thermal/intel/intel_powerclamp.c b/drivers/thermal/intel/intel_powerclamp.c index fb04470d7d4b..9deaf2b8ccf6 100644 --- a/drivers/thermal/intel/intel_powerclamp.c +++ b/drivers/thermal/intel/intel_powerclamp.c @@ -277,7 +277,8 @@ static unsigned int get_compensation(int ratio) comp = (cal_data[ratio].steady_comp + cal_data[ratio - 1].steady_comp + cal_data[ratio - 2].steady_comp) / 3; - } else if (cal_data[ratio].confidence >= CONFIDENCE_OK && + } else if (ratio < MAX_TARGET_RATIO - 1 && + cal_data[ratio].confidence >= CONFIDENCE_OK && cal_data[ratio - 1].confidence >= CONFIDENCE_OK && cal_data[ratio + 1].confidence >= CONFIDENCE_OK) { comp = (cal_data[ratio].steady_comp +
ratio may be equal to MAX_TARGET_RATIO - 1 that will result in out of bound access. Found by Linux Verification Center (linuxtesting.org) with SVACE. Fixes: d6d71ee4a14a ("PM: Introduce Intel PowerClamp Driver") Signed-off-by: Daniil Dulov <d.dulov@aladdin.ru> --- drivers/thermal/intel/intel_powerclamp.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)