drivers/cpuidle/governors/ladder.c | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-)
On certain platforms (PowerNV systems without a power-mgt DT node),
cpuidle may register only a single idle state. In cases where that
single state is a polling state (state 0), the ladder governor may
incorrectly treat state 1 as the first usable state and pass an
out-of-bounds index. This can lead to a NULL enter callback being
invoked, ultimately resulting in a system crash.
[ 13.342636] cpuidle-powernv : Only Snooze is available
[ 13.351854] Faulting instruction address: 0x00000000
[ 13.376489] NIP [0000000000000000] 0x0
[ 13.378351] LR [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
Fix this by determining the first non-polling state index based on
the number of registered states, and by returning state 0 when only
one state is registered.
Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
---
drivers/cpuidle/governors/ladder.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
index 6617eb494a11..294a688ed0bb 100644
--- a/drivers/cpuidle/governors/ladder.c
+++ b/drivers/cpuidle/governors/ladder.c
@@ -42,6 +42,21 @@ struct ladder_device {
static DEFINE_PER_CPU(struct ladder_device, ladder_devices);
+/**
+ * ladder_get_first_idx - get the first non-polling state index
+ * @drv: cpuidle driver
+ *
+ * Returns the index of the first non-polling state, or 0 if state 0 is not
+ * polling or if there's only one state available.
+ */
+static inline int ladder_get_first_idx(struct cpuidle_driver *drv)
+{
+ if (drv->state_count > 1 &&
+ drv->states[0].flags & CPUIDLE_FLAG_POLLING)
+ return 1;
+ return 0;
+}
+
/**
* ladder_do_selection - prepares private data for a state change
* @dev: the CPU
@@ -70,16 +85,17 @@ static int ladder_select_state(struct cpuidle_driver *drv,
struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
struct ladder_device_state *last_state;
int last_idx = dev->last_state_idx;
- int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
+ int first_idx;
s64 latency_req = cpuidle_governor_latency_req(dev->cpu);
s64 last_residency;
- /* Special case when user has set very strict latency requirement */
- if (unlikely(latency_req == 0)) {
+ /* Special case when there's only one state or strict latency requirement */
+ if (unlikely(drv->state_count <= 1 || latency_req == 0)) {
ladder_do_selection(dev, ldev, last_idx, 0);
return 0;
}
+ first_idx = ladder_get_first_idx(drv);
last_state = &ldev->states[last_idx];
last_residency = dev->last_residency_ns - drv->states[last_idx].exit_latency_ns;
@@ -134,7 +150,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
struct cpuidle_device *dev)
{
int i;
- int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
+ int first_idx = ladder_get_first_idx(drv);
struct ladder_device *ldev = &per_cpu(ladder_devices, dev->cpu);
struct ladder_device_state *lstate;
struct cpuidle_state *state;
--
2.52.0On 2/11/26 05:35, Aboorva Devarajan wrote:
> On certain platforms (PowerNV systems without a power-mgt DT node),
> cpuidle may register only a single idle state. In cases where that
> single state is a polling state (state 0), the ladder governor may
> incorrectly treat state 1 as the first usable state and pass an
> out-of-bounds index. This can lead to a NULL enter callback being
> invoked, ultimately resulting in a system crash.
>
> [ 13.342636] cpuidle-powernv : Only Snooze is available
> [ 13.351854] Faulting instruction address: 0x00000000
> [ 13.376489] NIP [0000000000000000] 0x0
> [ 13.378351] LR [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
>
> Fix this by determining the first non-polling state index based on
> the number of registered states, and by returning state 0 when only
> one state is registered.
>
> Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
Agreed that the current behavior is a bug, but is there really much value
in using a cpuidle governor with just a polling state?
It's dead code and trivial to bail out of in cpuidle, right?
> ---
> drivers/cpuidle/governors/ladder.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
> index 6617eb494a11..294a688ed0bb 100644
> --- a/drivers/cpuidle/governors/ladder.c
> +++ b/drivers/cpuidle/governors/ladder.c
> @@ -42,6 +42,21 @@ struct ladder_device {
>
> static DEFINE_PER_CPU(struct ladder_device, ladder_devices);
>
> +/**
> + * ladder_get_first_idx - get the first non-polling state index
> + * @drv: cpuidle driver
> + *
> + * Returns the index of the first non-polling state, or 0 if state 0 is not
> + * polling or if there's only one state available.
> + */
> +static inline int ladder_get_first_idx(struct cpuidle_driver *drv)
> +{
> + if (drv->state_count > 1 &&
> + drv->states[0].flags & CPUIDLE_FLAG_POLLING)
> + return 1;
> + return 0;
> +}
> +
> /**
> * ladder_do_selection - prepares private data for a state change
> * @dev: the CPU
> @@ -70,16 +85,17 @@ static int ladder_select_state(struct cpuidle_driver *drv,
> struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
> struct ladder_device_state *last_state;
> int last_idx = dev->last_state_idx;
> - int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> + int first_idx;
> s64 latency_req = cpuidle_governor_latency_req(dev->cpu);
> s64 last_residency;
>
> - /* Special case when user has set very strict latency requirement */
> - if (unlikely(latency_req == 0)) {
> + /* Special case when there's only one state or strict latency requirement */
> + if (unlikely(drv->state_count <= 1 || latency_req == 0)) {
> ladder_do_selection(dev, ldev, last_idx, 0);
> return 0;
> }
>
> + first_idx = ladder_get_first_idx(drv);
> last_state = &ldev->states[last_idx];
>
> last_residency = dev->last_residency_ns - drv->states[last_idx].exit_latency_ns;
> @@ -134,7 +150,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
> struct cpuidle_device *dev)
> {
> int i;
> - int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> + int first_idx = ladder_get_first_idx(drv);
> struct ladder_device *ldev = &per_cpu(ladder_devices, dev->cpu);
> struct ladder_device_state *lstate;
> struct cpuidle_state *state;
On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
> On 2/11/26 05:35, Aboorva Devarajan wrote:
> > On certain platforms (PowerNV systems without a power-mgt DT node),
> > cpuidle may register only a single idle state. In cases where that
> > single state is a polling state (state 0), the ladder governor may
> > incorrectly treat state 1 as the first usable state and pass an
> > out-of-bounds index. This can lead to a NULL enter callback being
> > invoked, ultimately resulting in a system crash.
> >
> > [ 13.342636] cpuidle-powernv : Only Snooze is available
> > [ 13.351854] Faulting instruction address: 0x00000000
> > [ 13.376489] NIP [0000000000000000] 0x0
> > [ 13.378351] LR [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
> >
> > Fix this by determining the first non-polling state index based on
> > the number of registered states, and by returning state 0 when only
> > one state is registered.
> >
> > Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
> > Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
>
> Agreed that the current behavior is a bug, but is there really much value
> in using a cpuidle governor with just a polling state?
> It's dead code and trivial to bail out of in cpuidle, right?
>
Hi Christian,
Thanks for the review.
Other governors (teo, menu) already handle this single-state scenario
correctly. Fixing ladder's first_idx calculation seemed like the most
targeted fix, however since ladder is not widely used this is likely
to go unnoticed, it only popped up during testing with a missing
power-mgt device tree node.
yes, adding a bail-out in the core cpuidle_select() is also trivial and
would benefit all governors uniformly. Setting stop_tick to false keeps
the tick running, which is correct for a single state configuration.
Please let me know if you'd prefer this approach instead.
---
diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
index c7876e9e024f..ea082419f7db 100644
--- a/drivers/cpuidle/cpuidle.c
+++ b/drivers/cpuidle/cpuidle.c
@@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
cpuidle_device *dev,
int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
*dev,
bool *stop_tick)
{
+ /*
+ * If there is only a single idle state (or none), there is
nothing
+ * meaningful for the governor to choose. Skip the governor and
+ * always use state 0 with the tick running.
+ */
+ if (unlikely(drv->state_count <= 1)) {
+ *stop_tick = false;
+ return 0;
+ }
+
return cpuidle_curr_governor->select(drv, dev, stop_tick);
}
---
Regards,
Aboorva
> > ---
> > drivers/cpuidle/governors/ladder.c | 24 ++++++++++++++++++++----
> > 1 file changed, 20 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/cpuidle/governors/ladder.c b/drivers/cpuidle/governors/ladder.c
> > index 6617eb494a11..294a688ed0bb 100644
> > --- a/drivers/cpuidle/governors/ladder.c
> > +++ b/drivers/cpuidle/governors/ladder.c
> > @@ -42,6 +42,21 @@ struct ladder_device {
> >
> > static DEFINE_PER_CPU(struct ladder_device, ladder_devices);
> >
> > +/**
> > + * ladder_get_first_idx - get the first non-polling state index
> > + * @drv: cpuidle driver
> > + *
> > + * Returns the index of the first non-polling state, or 0 if state 0 is not
> > + * polling or if there's only one state available.
> > + */
> > +static inline int ladder_get_first_idx(struct cpuidle_driver *drv)
> > +{
> > + if (drv->state_count > 1 &&
> > + drv->states[0].flags & CPUIDLE_FLAG_POLLING)
> > + return 1;
> > + return 0;
> > +}
> > +
> > /**
> > * ladder_do_selection - prepares private data for a state change
> > * @dev: the CPU
> > @@ -70,16 +85,17 @@ static int ladder_select_state(struct cpuidle_driver *drv,
> > struct ladder_device *ldev = this_cpu_ptr(&ladder_devices);
> > struct ladder_device_state *last_state;
> > int last_idx = dev->last_state_idx;
> > - int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> > + int first_idx;
> > s64 latency_req = cpuidle_governor_latency_req(dev->cpu);
> > s64 last_residency;
> >
> > - /* Special case when user has set very strict latency requirement */
> > - if (unlikely(latency_req == 0)) {
> > + /* Special case when there's only one state or strict latency requirement */
> > + if (unlikely(drv->state_count <= 1 || latency_req == 0)) {
> > ladder_do_selection(dev, ldev, last_idx, 0);
> > return 0;
> > }
> >
> > + first_idx = ladder_get_first_idx(drv);
> > last_state = &ldev->states[last_idx];
> >
> > last_residency = dev->last_residency_ns - drv->states[last_idx].exit_latency_ns;
> > @@ -134,7 +150,7 @@ static int ladder_enable_device(struct cpuidle_driver *drv,
> > struct cpuidle_device *dev)
> > {
> > int i;
> > - int first_idx = drv->states[0].flags & CPUIDLE_FLAG_POLLING ? 1 : 0;
> > + int first_idx = ladder_get_first_idx(drv);
> > struct ladder_device *ldev = &per_cpu(ladder_devices, dev->cpu);
> > struct ladder_device_state *lstate;
> > struct cpuidle_state *state;
On 2/13/26 08:29, Aboorva Devarajan wrote:
> On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
>> On 2/11/26 05:35, Aboorva Devarajan wrote:
>>> On certain platforms (PowerNV systems without a power-mgt DT node),
>>> cpuidle may register only a single idle state. In cases where that
>>> single state is a polling state (state 0), the ladder governor may
>>> incorrectly treat state 1 as the first usable state and pass an
>>> out-of-bounds index. This can lead to a NULL enter callback being
>>> invoked, ultimately resulting in a system crash.
>>>
>>> [ 13.342636] cpuidle-powernv : Only Snooze is available
>>> [ 13.351854] Faulting instruction address: 0x00000000
>>> [ 13.376489] NIP [0000000000000000] 0x0
>>> [ 13.378351] LR [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
>>>
>>> Fix this by determining the first non-polling state index based on
>>> the number of registered states, and by returning state 0 when only
>>> one state is registered.
>>>
>>> Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
>>> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
>>
>> Agreed that the current behavior is a bug, but is there really much value
>> in using a cpuidle governor with just a polling state?
>> It's dead code and trivial to bail out of in cpuidle, right?
>>
>
> Hi Christian,
>
> Thanks for the review.
>
> Other governors (teo, menu) already handle this single-state scenario
> correctly. Fixing ladder's first_idx calculation seemed like the most
> targeted fix, however since ladder is not widely used this is likely
> to go unnoticed, it only popped up during testing with a missing
> power-mgt device tree node.
>
> yes, adding a bail-out in the core cpuidle_select() is also trivial and
> would benefit all governors uniformly. Setting stop_tick to false keeps
> the tick running, which is correct for a single state configuration.
>
> Please let me know if you'd prefer this approach instead.
>
> ---
>
> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> index c7876e9e024f..ea082419f7db 100644
> --- a/drivers/cpuidle/cpuidle.c
> +++ b/drivers/cpuidle/cpuidle.c
> @@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
> cpuidle_device *dev,
> int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
> *dev,
> bool *stop_tick)
> {
> + /*
> + * If there is only a single idle state (or none), there is
> nothing
> + * meaningful for the governor to choose. Skip the governor and
> + * always use state 0 with the tick running.
> + */
> + if (unlikely(drv->state_count <= 1)) {
I think the unlikely isn't helping here, this just let the branch predictor
handle this as it won't change anyway.
> + *stop_tick = false;
> + return 0;
> + }
> +
> return cpuidle_curr_governor->select(drv, dev, stop_tick);
> }
>
I prefer this, additionally of course:
-------8<-------
diff --git a/drivers/cpuidle/governors/menu.c b/drivers/cpuidle/governors/menu.c
index 64d6f7a1c776..fdfa5d7e10a6 100644
--- a/drivers/cpuidle/governors/menu.c
+++ b/drivers/cpuidle/governors/menu.c
@@ -271,7 +271,7 @@ static int menu_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
data->bucket = BUCKETS - 1;
}
- if (unlikely(drv->state_count <= 1 || latency_req == 0) ||
+ if (unlikely(latency_req == 0) ||
((data->next_timer_ns < drv->states[1].target_residency_ns ||
latency_req < drv->states[1].exit_latency_ns) &&
!dev->states_usage[0].disable)) {
diff --git a/drivers/cpuidle/governors/teo.c b/drivers/cpuidle/governors/teo.c
index 81ac5fd58a1c..9b5b8c617806 100644
--- a/drivers/cpuidle/governors/teo.c
+++ b/drivers/cpuidle/governors/teo.c
@@ -317,12 +317,6 @@ static int teo_select(struct cpuidle_driver *drv, struct cpuidle_device *dev,
*/
cpu_data->sleep_length_ns = KTIME_MAX;
- /* Check if there is any choice in the first place. */
- if (drv->state_count < 2) {
- idx = 0;
- goto out_tick;
- }
-
if (!dev->states_usage[0].disable)
idx = 0;
On 2/13/26 13:44, Christian Loehle wrote:
> On 2/13/26 08:29, Aboorva Devarajan wrote:
>> On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
>>> On 2/11/26 05:35, Aboorva Devarajan wrote:
>>>> On certain platforms (PowerNV systems without a power-mgt DT node),
>>>> cpuidle may register only a single idle state. In cases where that
>>>> single state is a polling state (state 0), the ladder governor may
>>>> incorrectly treat state 1 as the first usable state and pass an
>>>> out-of-bounds index. This can lead to a NULL enter callback being
>>>> invoked, ultimately resulting in a system crash.
>>>>
>>>> [ 13.342636] cpuidle-powernv : Only Snooze is available
>>>> [ 13.351854] Faulting instruction address: 0x00000000
>>>> [ 13.376489] NIP [0000000000000000] 0x0
>>>> [ 13.378351] LR [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
>>>>
>>>> Fix this by determining the first non-polling state index based on
>>>> the number of registered states, and by returning state 0 when only
>>>> one state is registered.
>>>>
>>>> Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
>>>> Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
>>>
>>> Agreed that the current behavior is a bug, but is there really much value
>>> in using a cpuidle governor with just a polling state?
>>> It's dead code and trivial to bail out of in cpuidle, right?
>>>
>>
>> Hi Christian,
>>
>> Thanks for the review.
>>
>> Other governors (teo, menu) already handle this single-state scenario
>> correctly. Fixing ladder's first_idx calculation seemed like the most
>> targeted fix, however since ladder is not widely used this is likely
>> to go unnoticed, it only popped up during testing with a missing
>> power-mgt device tree node.
>>
>> yes, adding a bail-out in the core cpuidle_select() is also trivial and
>> would benefit all governors uniformly. Setting stop_tick to false keeps
>> the tick running, which is correct for a single state configuration.
>>
>> Please let me know if you'd prefer this approach instead.
>>
>> ---
>>
>> diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
>> index c7876e9e024f..ea082419f7db 100644
>> --- a/drivers/cpuidle/cpuidle.c
>> +++ b/drivers/cpuidle/cpuidle.c
>> @@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
>> cpuidle_device *dev,
>> int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
>> *dev,
>> bool *stop_tick)
>> {
>> + /*
>> + * If there is only a single idle state (or none), there is
>> nothing
>> + * meaningful for the governor to choose. Skip the governor and
>> + * always use state 0 with the tick running.
>> + */
>> + if (unlikely(drv->state_count <= 1)) {
>
> I think the unlikely isn't helping here, this just let the branch predictor
> handle this as it won't change anyway.
>
>> + *stop_tick = false;
>> + return 0;
>> + }
>> +
>> return cpuidle_curr_governor->select(drv, dev, stop_tick);
>> }
>>
>
> I prefer this, additionally of course:
I've attached them as patches with a sign-off, feel free to pick them up as a series
or if you provide your signoff I can do that as well.
On Mon, 2026-02-16 at 11:05 +0000, Christian Loehle wrote:
> On 2/13/26 13:44, Christian Loehle wrote:
> > On 2/13/26 08:29, Aboorva Devarajan wrote:
> > > On Wed, 2026-02-11 at 15:00 +0000, Christian Loehle wrote:
> > > > On 2/11/26 05:35, Aboorva Devarajan wrote:
> > > > > On certain platforms (PowerNV systems without a power-mgt DT node),
> > > > > cpuidle may register only a single idle state. In cases where that
> > > > > single state is a polling state (state 0), the ladder governor may
> > > > > incorrectly treat state 1 as the first usable state and pass an
> > > > > out-of-bounds index. This can lead to a NULL enter callback being
> > > > > invoked, ultimately resulting in a system crash.
> > > > >
> > > > > [ 13.342636] cpuidle-powernv : Only Snooze is available
> > > > > [ 13.351854] Faulting instruction address: 0x00000000
> > > > > [ 13.376489] NIP [0000000000000000] 0x0
> > > > > [ 13.378351] LR [c000000001e01974] cpuidle_enter_state+0x2c4/0x668
> > > > >
> > > > > Fix this by determining the first non-polling state index based on
> > > > > the number of registered states, and by returning state 0 when only
> > > > > one state is registered.
> > > > >
> > > > > Fixes: dc2251bf98c6 ("cpuidle: Eliminate the CPUIDLE_DRIVER_STATE_START symbol")
> > > > > Signed-off-by: Aboorva Devarajan <aboorvad@linux.ibm.com>
> > > >
> > > > Agreed that the current behavior is a bug, but is there really much value
> > > > in using a cpuidle governor with just a polling state?
> > > > It's dead code and trivial to bail out of in cpuidle, right?
> > > >
> > >
> > > Hi Christian,
> > >
> > > Thanks for the review.
> > >
> > > Other governors (teo, menu) already handle this single-state scenario
> > > correctly. Fixing ladder's first_idx calculation seemed like the most
> > > targeted fix, however since ladder is not widely used this is likely
> > > to go unnoticed, it only popped up during testing with a missing
> > > power-mgt device tree node.
> > >
> > > yes, adding a bail-out in the core cpuidle_select() is also trivial and
> > > would benefit all governors uniformly. Setting stop_tick to false keeps
> > > the tick running, which is correct for a single state configuration.
> > >
> > > Please let me know if you'd prefer this approach instead.
> > >
> > > ---
> > >
> > > diff --git a/drivers/cpuidle/cpuidle.c b/drivers/cpuidle/cpuidle.c
> > > index c7876e9e024f..ea082419f7db 100644
> > > --- a/drivers/cpuidle/cpuidle.c
> > > +++ b/drivers/cpuidle/cpuidle.c
> > > @@ -359,6 +359,16 @@ noinstr int cpuidle_enter_state(struct
> > > cpuidle_device *dev,
> > > int cpuidle_select(struct cpuidle_driver *drv, struct cpuidle_device
> > > *dev,
> > > bool *stop_tick)
> > > {
> > > + /*
> > > + * If there is only a single idle state (or none), there is
> > > nothing
> > > + * meaningful for the governor to choose. Skip the governor and
> > > + * always use state 0 with the tick running.
> > > + */
> > > + if (unlikely(drv->state_count <= 1)) {
> >
> > I think the unlikely isn't helping here, this just let the branch predictor
> > handle this as it won't change anyway.
> >
> > > + *stop_tick = false;
> > > + return 0;
> > > + }
> > > +
> > > return cpuidle_curr_governor->select(drv, dev, stop_tick);
> > > }
> > >
> >
> > I prefer this, additionally of course:
>
> I've attached them as patches with a sign-off, feel free to pick them up as a series
> or if you provide your signoff I can do that as well.
Hi Christian,
Thanks, I've included your patches and sent them as a series:
https://lore.kernel.org/all/20260216185005.1131593-1-aboorvad@linux.ibm.com/
Regards,
Aboorva
© 2016 - 2026 Red Hat, Inc.