| 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
| ||
|
Message-ID: <20080226233445.GA1297@Krystal>
Date: Tue, 26 Feb 2008 18:34:45 -0500
From: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
To: Jason Baron <jbaron@...hat.com>
Cc: akpm@...ux-foundation.org, Ingo Molnar <mingo@...e.hu>,
linux-kernel@...r.kernel.org,
Rusty Russell <rusty@...tcorp.com.au>,
Jan Kiszka <jan.kiszka@...mens.com>
Subject: Re: [patch 1/7] Immediate Values - Architecture Independent Code
* Mathieu Desnoyers (mathieu.desnoyers@...ymtl.ca) wrote:
> * Jason Baron (jbaron@...hat.com) wrote:
> > On Sat, Feb 02, 2008 at 04:08:29PM -0500, Mathieu Desnoyers wrote:
> > > Changelog:
> > >
> > > - section __imv is already SHF_ALLOC
> > > - Because of the wonders of ELF, section 0 has sh_addr and sh_size 0. So
> > > the if (immediateindex) is unnecessary here.
> > > - Remove module_mutex usage: depend on functions implemented in module.c for
> > > that.
> >
> > hi,
> >
> > In testing this patch, i've run across a deadlock...apply_imv_update() can get
> > called again before, ipi_busy_loop() has had a chance to finsh, and set
> > wait_sync back to its initial value. This causes ipi_busy_loop() to get stuck
> > indefinitely and the subsequent apply_imv_update() hangs. I've shown this
> > deadlock below using nmi_watchdog=1 in item 1).
> >
>
> Hrm, yes, Jan pointed out the exact same problem in my ltt-test-tsc TSC
> test module in LTTng a few days ago. His fix implied to add another
> barrier upon which the smp_call_function() caller must wait for the ipis
> to finish. Since this immediate value code does the same I did in my
> ltt-test-tsc code, the same fix will likely apply.
>
> I'll cook something.
>
This should work. Untested for now. Can you give it a try ?
Fix Immediate Deadlock
Jason Baron <jbaron@...hat.com> :
In testing this patch, i've run across a deadlock...apply_imv_update() can get
called again before, ipi_busy_loop() has had a chance to finsh, and set
wait_sync back to its initial value. This causes ipi_busy_loop() to get stuck
indefinitely and the subsequent apply_imv_update() hangs. I've shown this
deadlock below using nmi_watchdog=1 in item 1).
Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca> :
Hrm, yes, Jan pointed out the exact same problem in my ltt-test-tsc TSC
test module in LTTng a few days ago. His fix implied to add another
barrier upon which the smp_call_function() caller must wait for the ipis
to finish. Since this immediate value code does the same I did in my
ltt-test-tsc code, the same fix will likely apply.
Thanks to Jason Baron for finding this bug and proposing an initial
implementation.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...ymtl.ca>
CC: Jason Baron <jbaron@...hat.com>
---
kernel/immediate.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
Index: linux-2.6-lttng/kernel/immediate.c
===================================================================
--- linux-2.6-lttng.orig/kernel/immediate.c 2008-02-26 18:16:10.000000000 -0500
+++ linux-2.6-lttng/kernel/immediate.c 2008-02-26 18:29:32.000000000 -0500
@@ -53,12 +53,12 @@ static void ipi_busy_loop(void *arg)
do {
/* Make sure the wait_sync gets re-read */
smp_mb();
- } while (atomic_read(&wait_sync) > loop_data.value);
+ } while (atomic_read(&wait_sync) > 2 * loop_data.value);
atomic_dec(&wait_sync);
do {
/* Make sure the wait_sync gets re-read */
smp_mb();
- } while (atomic_read(&wait_sync) > 0);
+ } while (atomic_read(&wait_sync) > loop_data.value);
/*
* Issuing a synchronizing instruction must be done on each CPU before
* reenabling interrupts after modifying an instruction. Required by
@@ -67,6 +67,7 @@ static void ipi_busy_loop(void *arg)
sync_core();
flush_icache_range(loop_data.imv->imv,
loop_data.imv->imv + loop_data.imv->size);
+ atomic_dec(&wait_sync);
local_irq_restore(flags);
}
@@ -113,7 +114,7 @@ static int apply_imv_update(const struct
kernel_text_lock();
get_online_cpus();
online_cpus = num_online_cpus();
- atomic_set(&wait_sync, 2 * online_cpus);
+ atomic_set(&wait_sync, 3 * online_cpus);
loop_data.value = online_cpus;
loop_data.imv = imv;
smp_call_function(ipi_busy_loop, NULL, 1, 0);
@@ -122,7 +123,7 @@ static int apply_imv_update(const struct
do {
/* Make sure the wait_sync gets re-read */
smp_mb();
- } while (atomic_read(&wait_sync) > online_cpus);
+ } while (atomic_read(&wait_sync) > 2 * online_cpus);
text_poke((void *)imv->imv, (void *)imv->var,
imv->size);
/*
@@ -133,6 +134,12 @@ static int apply_imv_update(const struct
atomic_dec(&wait_sync);
flush_icache_range(imv->imv,
imv->imv + imv->size);
+ /*
+ * Wait until all other CPUs are done so that we don't overwrite
+ * loop_data or wait_sync prematurely.
+ */
+ while (unlikely(atomic_read(&wait_sync) > 1))
+ cpu_relax();
local_irq_restore(flags);
put_online_cpus();
kernel_text_unlock();
--
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