Re: od_set_powersave_bias: NULL pointer dereference

[Viresh Kumar]

On 26 June 2013 23:27, Jacob Shin <jacob.shin@xxxxxxx> wrote:
> On Wed, Jun 26, 2013 at 08:02:29PM +0530, Viresh Kumar wrote:
>> On 26 June 2013 19:58, Jacob Shin <jacob.shin@xxxxxxx> wrote:
>> > On Wed, Jun 26, 2013 at 12:18:27PM +0530, Viresh Kumar wrote:
>>
>> >> I am not sure if this is enough. What if we had ondemand as the
>> >> governor initially, then we changed it to something else. Now also
>> >> cur_policy contains a address and isn't zero.
>
> I just tested this case with this patch applied, and did not have any
> problems.

Try this:
- you need a system with multiple policy groups to test it
- Suppose we have two groups of CPUs: 0 and 1
- Set ondemand as governor for both
- change governor of group 1 to something else (we still have valid policy
struct in Ondemand)
- offline all CPUs from group 1. this will free struct cpufreq_policy
- Online these CPUs back, this will reallocate policy
- Now run this function, the earlier policy struct is already freed and
you are accessing it here.

>> >> >                 cpumask_or(&done, &done, policy->cpus);
>> >> > +
>> >> > +               if (policy->governor != &cpufreq_gov_ondemand)
>> >> > +                       continue;
>> >
>> > This should catch that case no ?
>>
>> Policy might be freed and reallocated by then. And so doing
>> policy->governor is dangerous.
>
> Are you worried that after we have passed the above if check, and
> before we access ->tuner governor change might occur?
>
> Is there something synonymous to get/put_online_cpus() for cpufreq to
> prevent governor change while we update ->tuner values?
>
> Otherwise, should just spinlock?

No, i wasn't worrying about this but a sequence of events that I told to
you earlier.

Replying to your other mail:
> Hm . any hints on how to check for if ondemand is running on this CPU
> or not ? I'm not sure what the best way to handle this is ..

Make cur_policy zero in cpufreq_governor_dbs() for
CPUFREQ_GOV_STOP notification. This will make sure we use correct
policy pointer.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/