linux-kernel - Re: [PATCH 2/3] work_on_cpu: Use our own workqueue.

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <b647ffbd0902050944m40d69bc6m3b031b7a0ea80c7c@mail.gmail.com>
Date:	Thu, 5 Feb 2009 18:44:36 +0100
From:	Dmitry Adamushko <dmitry.adamushko@...il.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
Cc:	Ingo Molnar <mingo@...e.hu>, rusty@...tcorp.com.au, travis@....com,
	mingo@...hat.com, davej@...hat.com, cpufreq@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] work_on_cpu: Use our own workqueue.

2009/2/4 Andrew Morton <akpm@...ux-foundation.org>:
> On Wed, 4 Feb 2009 22:35:19 +0100
> Ingo Molnar <mingo@...e.hu> wrote:
>
>>
>> * Andrew Morton <akpm@...ux-foundation.org> wrote:
>>
>> > mm/pdflush.c:
>> >
>> >   wtf what the heck is all that stuff and who added it?  weird.
>> >
>> >   Leave it alone I guess.  Can admins manually move kernel threads to
>> >   other CPUs?
>>
>> they can - and there's even tools that do that (there's some -rt tools where
>> you can put kernel thread priorities into a config file).
>>
>
> Oh well, DontDoThatThen.
>
> I expect that the same argument applies to most of the set_cpus_allowed()
> callsites - they're run by root-only code.  Sure, root can (with
> careful timing) move root's own thread onto the wrong CPU in the middle
> of microcode loading.  In which case root gets to own both pieces.

Another issue is that those set_cpus_allowed() callsites may
effectivelly cancel the effect of sched_setaffinity() being run by an
administrator in parallel with a target process calling e.g.
cpufreq_get(0)

[ there are a couple of callsites in drivers/{video,pcmcia}, possibly
running in a process context for which sched_setaffinity() with mask
!= 'all_cpus_set' may be legitimate from admin's POV :-) ]

iow, not all use-cases are so obvious (like microcode) to fall into
the category of "didn't you know that this action could do some
unsynchronized cpu-mask-fiddling work behind your back". Not to say
that the existence of sych category is wrong, imho.

(regarding microcode)

> This code is just nuts.  What's the point in pinning itself to > a CPU for the act of loading the microcode into main
> memory?  It's only
> the loading of the microcode which should care about
> which CPU
> executes the code.  ie: apply_microcode().

Well, basically I tried to preserve the existing mechanisms/schemes as
much as possible when reworking this code and yes, I'm the person to
be blamed for this 'nuts' code.

> The code needs some laundering, switch to >schedule_work_on().

I had a patch doing exactly this but then there were other concerns
with the 'schedule_work_on()' approach
(e.g. http://linux.derkeiler.com/Mailing-Lists/Kernel/2008-08/msg02827.html)

then I had another idea (run it from start_secondary() or something
like this), but I never managed to look at this issue again
(and noone else seemed to care about really running "it as early as
possible" ;-)

In any case, it should be fixable one way or another.

> Ensure that the callback functions don't take >microcode_mutex.

yes, these actually look redundant in cpu-hotplug paths (other callers
call get/put_online_cpus() so this part should be ok).

Will fix. Thanks for your comments!


-- 
Best regards,
Dmitry Adamushko
--
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/