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
| ||
|
Date: Thu, 13 Sep 2007 17:21:07 -0400 From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca> To: Rusty Russell <rusty@...tcorp.com.au> Cc: Alexey Dobriyan <adobriyan@...il.com>, akpm@...ux-foundation.org, linux-kernel@...r.kernel.org, "H. Peter Anvin" <hpa@...or.com> Subject: Re: [patch 1/8] Immediate Values - Global Modules List and Module Mutex * Rusty Russell (rusty@...tcorp.com.au) wrote: > On Tue, 2007-09-11 at 10:27 -0400, Mathieu Desnoyers wrote: > > * Rusty Russell (rusty@...tcorp.com.au) wrote: > > > On Mon, 2007-09-10 at 20:45 -0400, Mathieu Desnoyers wrote: > > > > Code patching of _live_ SMP code is allowed. This is why I went through > > > > all this trouble on i386. > > > > > > Oh, I was pretty sure it wasn't. OK. > > > > > > So now why three versions of immediate_set()? And why are you using my > > > lock for exclusion? Against what? > > > > > > > If we need to patch code at boot time, when interrupts are still > > disabled (it happens when we parse the kernel arguments for instance), > > we cannot afford to use IPIs to call sync_core() on each cpu, using > > breakpoints/notifier chains could be tricky (because we are very early > > at boot and alternatives or paravirt may not have been applied yet). > > Hi Mathieu, > > Sure, but why is that the caller's problem? immediate_set() isn't > fastpath, so why not make it do an "if (early_boot)" internally? > I see two reasons: 1 - early_boot, or anything that looks like this, does not exist currently (and the following reason might show why). 2 - If we use this, we cannot declare the early code with __init, so it will have to stay there forever insteaf of being removable once boot is over. Therefore, I think it's better to stick to an immediate_set_early version. > > _immediate_set() has been introduced because of the way immediate values > > are used by markers: the linux kernel markers already hold the module > > mutex when they need to update the immediate values. Taking the mutex > > twice makes no sence, so _immediate_set() is used when the caller > > already holds the module mutex. > > > Why not just have one immediate_set() which iterates through and fixes > > > up all the references? > > > > (reasons explained above) > > > > > It can use an internal lock if you want to avoid > > > concurrent immediate_set() calls. > > > > > > > An internal lock won't protect against modules load/unload race. We have > > to iterate on the module list. > > Sure, but it seems like that's fairly easy to do within module.c: > > /* This updates all the immediates even though only one might have > * changed. But it's so rare it's not worth optimizing. */ > void module_update_immediates(void) > { > mutex_lock(&module_mutex); > list_for_each_entry(mod, &modules, list) > update_immediates(mod->immediate, mod->num_immediate); > mutex_unlock(&module_mutex); > } > > Then during module load you do: > > update_immediates(mod->immediate, mod->num_immediate); > > Your immediate_update() just becomes: > > update_immediates(__start___immediate, > __stop___immediate - __start___immediate); > module_update_immediates(); > > update_immediates() can grab the immediate_mutex if you want. > Yup, excellent idea. I just changed the linux kernel markers too. > > > Why is it easier to patch the sites now than later? Currently it's just > > > churn. You could go back and find them when this mythical patch gets > > > merged into this mythical future gcc version. It could well need a > > > completely different macro style, like "cond_imm(var, code)". > > > > Maybe you're right. My though was that if we have a way to express a > > strictly boolean if() statement that can later be optimized further by > > gcc using a jump rather than a conditionnal branch and currently emulate > > it by using a load immediate/test/branch, we might want to do so right > > now so we don't have to do a second code transition from > > if (immediate_read(&var)) to immediate_if (&var) later. But you might be > > right in that the form could potentially change anyway when the > > implementation would come, although I don't see how. > > I was thinking that we might find useful specific cases before we get > GCC support, which archs can override with tricky asm if they wish. > The first useful case is the Linux Kernel Markers, which really needs a completely boolean if: active or inactive. That would be a good test case to get gcc support. Mathieu > Cheers, > Rusty. > > -- Mathieu Desnoyers Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F BA06 3F25 A8FE 3BAE 9A68 - 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/
Powered by blists - more mailing lists