linux-kernel - Re: [PATCH V2] cpufreq: Fix double addition of sysfs links

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Thu, 23 Jul 2015 21:29:46 +0200
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	"Rafael J. Wysocki" <rafael@...nel.org>
Cc:	Viresh Kumar <viresh.kumar@...aro.org>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Rafael Wysocki <rjw@...ysocki.net>,
	Lists linaro-kernel <linaro-kernel@...ts.linaro.org>,
	"linux-pm@...r.kernel.org" <linux-pm@...r.kernel.org>,
	open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH V2] cpufreq: Fix double addition of sysfs links

On Thu, Jul 23, 2015 at 7:22 PM, Rafael J. Wysocki <rafael@...nel.org> wrote:
> Hi Viresh,
>
> On Thu, Jul 23, 2015 at 8:09 AM, Viresh Kumar <viresh.kumar@...aro.org> wrote:
>> On 22-07-15, 18:42, Rafael J. Wysocki wrote:
>>> > 3. what happens when 'policy' is NULL at the point when the first (few) CPUs
>>> >    are added - how do the symlinks get created later if/when policy becomes
>>> >    non-NULL (can it?)
>>>
>>> Yes, it can, and we have a design issue here that bothers me a bit.
>>
>> I replied to Russell with a NO here as the first CPU should have
>> created the policy. BUT...
>>
>>> Namley, we need a driver's ->init callback to populate policy->cpus
>>> for us, but this is not the only thing it is doing, so the concern is
>>> that it may not be able to deal with CPUs that aren't online.
>>
>> ... the first few CPUs could have been offline and so we might not
>> have tried to add the policy at all.. Need to fix that for sure.
>
> Wait here.
>
> The current Linus' tree doesn't have that problem as far as I can say.
>
> Say cpufreq_interface->add_dev() is called for an offline CPU (say
> CPU2).  It points to cpufreq_add_dev(), so we see that the CPU is
> offline and call add_cpu_dev_symlink() for it.  But the first argument
> we pass to that is per_cpu(cpufreq_cpu_data, cpu) and that is NULL,
> because the policy is not there yet.  So we just return 0 (and the CPU
> has no policy and no link).
>
> Now say cpufreq_interface->add_dev() is called for an online CPU (say
> CPU3).  It goes and creates the policy for it and the driver's
> ->init() tells us that CPU2 is related to it.  So
> cpufreq_add_dev_interface() creates the link for CPU2 and we're fine.
>
> Now say CPU3 was offline too when cpufreq_interface->add_dev() was
> called for it.  We don't create a policy or a link for it.  Now say
> CPU2 becomes online.  cpufreq_cpu_callback() calls cpufreq_add_dev()
> for it and we land in the previous case.
>
> The *broken* case is when CPU2 is online to start with and it had
> created the link for CPU3, so when an offline CPU3 is now being added,
> we try to create the link for it again.  That is the case we need to
> address in -rc without introducing new problems.  The $subject patch
> adresses that issue, but it introduces the above problem.  On the
> other hand, my patch at https://patchwork.kernel.org/patch/6839151/
> should take care of this too (unless it is broken in a way I'm not
> seeing now).

It doesn't address the case when the CPU being removed is the policy owner.

Let me prepare a new version of it and we'll start over from there.

Thanks,
Rafael
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/