* [bug report] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
@ 2016-10-25 8:43 Dan Carpenter
2016-10-25 9:12 ` Sudeep Holla
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2016-10-25 8:43 UTC (permalink / raw)
To: Sudeep.Holla; +Cc: linux-acpi
Hello Sudeep Holla,
The patch a36a7fecfe60: "ACPI / processor_idle: Add support for Low
Power Idle(LPI) states" from Jul 21, 2016, leads to the following
static checker warning:
drivers/acpi/processor_idle.c:1261 acpi_processor_setup_lpi_states()
warn: buffer overflow 'pr->power.lpi_states' 8 <= 9
drivers/acpi/processor_idle.c
1250 static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
1251 {
1252 int i;
1253 struct acpi_lpi_state *lpi;
1254 struct cpuidle_state *state;
1255 struct cpuidle_driver *drv = &acpi_idle_driver;
1256
1257 if (!pr->flags.has_lpi)
1258 return -EOPNOTSUPP;
1259
1260 for (i = 0; i < pr->power.count && i < CPUIDLE_STATE_MAX; i++) {
^^^^^^^^^^^^^^^^^
Should this be ACPI_PROCESSOR_MAX_POWER?
1261 lpi = &pr->power.lpi_states[i];
^^^^^^^^^^
because that's how many elements we have in this array.
1262
1263 state = &drv->states[i];
1264 snprintf(state->name, CPUIDLE_NAME_LEN, "LPI-%d", i);
1265 strlcpy(state->desc, lpi->desc, CPUIDLE_DESC_LEN);
1266 state->exit_latency = lpi->wake_latency;
1267 state->target_residency = lpi->min_residency;
1268 if (lpi->arch_flags)
1269 state->flags |= CPUIDLE_FLAG_TIMER_STOP;
1270 state->enter = acpi_idle_lpi_enter;
1271 drv->safe_state_index = i;
1272 }
1273
1274 drv->state_count = i;
1275
1276 return 0;
1277 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-10-25 8:43 [bug report] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Dan Carpenter
@ 2016-10-25 9:12 ` Sudeep Holla
2016-10-25 9:23 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Sudeep Holla @ 2016-10-25 9:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Sudeep Holla, linux-acpi
Hi Dan,
I am finding it difficult to understand this bug report. Please help.
On 25/10/16 09:43, Dan Carpenter wrote:
> Hello Sudeep Holla,
>
> The patch a36a7fecfe60: "ACPI / processor_idle: Add support for Low
> Power Idle(LPI) states" from Jul 21, 2016, leads to the following
> static checker warning:
>
> drivers/acpi/processor_idle.c:1261 acpi_processor_setup_lpi_states()
> warn: buffer overflow 'pr->power.lpi_states' 8 <= 9
>
> drivers/acpi/processor_idle.c
> 1250 static int acpi_processor_setup_lpi_states(struct acpi_processor *pr)
> 1251 {
> 1252 int i;
> 1253 struct acpi_lpi_state *lpi;
> 1254 struct cpuidle_state *state;
> 1255 struct cpuidle_driver *drv = &acpi_idle_driver;
> 1256
> 1257 if (!pr->flags.has_lpi)
> 1258 return -EOPNOTSUPP;
> 1259
Currently CPUIDLE_STATE_MAX = 10 and ACPI_PROCESSOR_MAX_POWER = 8
> 1260 for (i = 0; i < pr->power.count && i < CPUIDLE_STATE_MAX; i++) {
>
^^^^^^^^^^^^^^^^^
We have drv->states[10] and pr->power.lpi_states[8] and above check is
to ensure we take the minimum of the 2 so that the logic continues to
work if either of these are changed.
>
> Should this be ACPI_PROCESSOR_MAX_POWER?
No, because we don't want to overflow CPUIDLE_STATE_MAX too in case
that's reduced to say 6. pr->power.count is assured to be <=
ACPI_PROCESSOR_MAX_POWER
>
> 1261 lpi = &pr->power.lpi_states[i];
> ^^^^^^^^^^
> because that's how many elements we have in this array.
>
Yes that's correct but i < CPUIDLE_STATE_MAX is to ensure we don't
overflow drv->states.
Let me know if I am being stupid and not able to understand something here.
--
Regards,
Sudeep
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] ACPI / processor_idle: Add support for Low Power Idle(LPI) states
2016-10-25 9:12 ` Sudeep Holla
@ 2016-10-25 9:23 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2016-10-25 9:23 UTC (permalink / raw)
To: Sudeep Holla; +Cc: linux-acpi
On Tue, Oct 25, 2016 at 10:12:11AM +0100, Sudeep Holla wrote:
> Let me know if I am being stupid and not able to understand something here.
No no. That works. Thanks for the explanation.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-25 9:23 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25 8:43 [bug report] ACPI / processor_idle: Add support for Low Power Idle(LPI) states Dan Carpenter
2016-10-25 9:12 ` Sudeep Holla
2016-10-25 9:23 ` Dan Carpenter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.