Skip to contentSkip to conversation

ACPI / processor_idle: Add support for Low Power Idle(LPI) states

bug report Dan Carpenter 2 participants 3 messages Oct 25, 2016 ACPI

lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

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

#1 lore raw

Hi Dan,

I am finding it difficult to understand this bug report. Please help.

Quoted 12 lines · Dan Carpenter

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 {

Quoted 8 lines
  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

Quoted 2 lines
  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.

Quoted 2 lines

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

Quoted 5 lines
  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.

Signature · 2 lines
Regards,
Sudeep
author
#2 lore raw
Quoted 1 line · Sudeep Holla

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