On 06/14/2016 03:18 AM, Benjamin Herrenschmidt wrote:
> On Wed, 2016-06-08 at 11:54 -0500, Shreyas B. Prabhu wrote:
>>
>> /*
>> * States for dedicated partition case.
>> */
>> @@ -167,6 +183,8 @@ static int powernv_add_idle_states(void)
>> int nr_idle_states = 1; /* Snooze */
>> int dt_idle_states;
>> u32 *latency_ns, *residency_ns, *flags;
>> + u64 *psscr_val = NULL;
>> + const char *names[CPUIDLE_STATE_MAX];
>> int i, rc;
>>
>> /* Currently we have snooze statically defined */
>> @@ -199,12 +217,41 @@ static int powernv_add_idle_states(void)
>> goto out_free_latency;
>> }
>>
>> + rc = of_property_read_string_array(power_mgt,
>> + "ibm,cpu-idle-state-names", names,
>> + dt_idle_states);
>
> Ok so from this I assume that dt_idle_states is the number of entries,
> which has been checked properly to be < CPUIDLE_STATE_MAX correct ?
>
> Beause ...
>
While dt_idle_states should not be > CPUIDLE_STATE_MAX, if that were the
case we will end up corrupting memory while updating powernv_states[].
I'll add a WARN_ON for such a case and
handle adding idle states to powernv_states accordingly. Thanks for
pointing this out.
>> + if (rc < 0) {
>> + pr_warn("cpuidle-powernv: missing ibm,cpu-idle-state-names in
>> DT\n");
>> + goto out_free_latency;
>> + }
>> +
>> + /*
>> + * If the idle states use stop instruction, probe for psscr values
>> + * which are necessary to specify required stop level.
>> + */
>> + if (flags[0] & (OPAL_PM_STOP_INST_FAST | OPAL_PM_STOP_INST_DEEP)) {
>> + psscr_val = kcalloc(dt_idle_states, sizeof(*psscr_val),
>> + GFP_KERNEL);
>> + rc = of_property_read_u64_array(power_mgt,
>> + "ibm,cpu-idle-state-psscr",
>> + psscr_val, dt_idle_states);
>
> Here, psscr val is only one u64 ... shouldn't you kmalloc sizeof(..) *
> dt_idle_states ?
I'm using kcalloc here since checkpatch script suggested kcalloc over
kzalloc for allocating memory for arrays.
I'll also include a patch to use kcalloc throughout the file for
uniformity in next version. I was originally planning to post that
cleanup separately.
Thanks,
Shreyas
