[uml-devel] UML running on a SMP host: bug in SKAS3 patch

[uml-devel] UML running on a SMP host: bug in SKAS3 patch

[Bodo Stroesser]

[-- Attachment #1: Type: text/plain, Size: 30 bytes --]

Description: see patch.

Bodo

[-- Attachment #2: host-2.6.9-skas3.v7-smp-fix-ldt.patch --]
[-- Type: text/x-diff, Size: 2549 bytes --]

From: Bodo Stroesser <bstroesser@fujitsu-siemens.com>

The SKAS3 host patch isn't smp-safe.

The wrong part is the implementation of PTRACE_LDT.
If the ptraced child runs on an other processor than its parent,
and the child's mm still is the active_mm, the changed LDT
isn't flushed out.
The problem occurs in alloc_ldt, but to fix this, I had to
change the params of copy_ldt, too.

---

--- a/arch/i386/kernel/ldt.c	2005-01-26 15:35:44.000000000 +0100
+++ b/arch/i386/kernel/ldt.c	2005-01-26 16:57:54.000000000 +0100
@@ -28,11 +28,12 @@ static void flush_ldt(void *null)
 }
 #endif
 
-static int alloc_ldt(mm_context_t *pc, int mincount, int reload)
+static int alloc_ldt(struct mm_struct *mm, int mincount, int reload)
 {
 	void *oldldt;
 	void *newldt;
 	int oldsize;
+	mm_context_t * pc = &mm->context;
 
 	if (mincount <= pc->size)
 		return 0;
@@ -55,17 +56,19 @@ static int alloc_ldt(mm_context_t *pc, i
 	pc->size = mincount;
 	wmb();
 
-	if (reload && (&current->active_mm->context == pc)) {
+	if (reload) {
 #ifdef CONFIG_SMP
 		cpumask_t mask;
 		preempt_disable();
-		load_LDT(pc);
+		if (&current->active_mm->context == pc)
+			load_LDT(pc);
 		mask = cpumask_of_cpu(smp_processor_id());
-		if (!cpus_equal(current->mm->cpu_vm_mask, mask))
+		if (!cpus_equal(mm->cpu_vm_mask, mask))
 			smp_call_function(flush_ldt, NULL, 1, 1);
 		preempt_enable();
 #else
-		load_LDT(pc);
+		if (&current->active_mm->context == pc)
+			load_LDT(pc);
 #endif
 	}
 	if (oldsize) {
@@ -77,12 +80,12 @@ static int alloc_ldt(mm_context_t *pc, i
 	return 0;
 }
 
-static inline int copy_ldt(mm_context_t *new, mm_context_t *old)
+static inline int copy_ldt(struct mm_struct *new, struct mm_struct *old)
 {
-	int err = alloc_ldt(new, old->size, 0);
+	int err = alloc_ldt(new, old->context.size, 0);
 	if (err < 0)
 		return err;
-	memcpy(new->ldt, old->ldt, old->size*LDT_ENTRY_SIZE);
+	memcpy(new->context.ldt, old->context.ldt, old->context.size*LDT_ENTRY_SIZE);
 	return 0;
 }
 
@@ -96,7 +99,7 @@ int copy_context(struct mm_struct *mm, s
 
 	if (old_mm && old_mm->context.size > 0) {
 		down(&old_mm->context.sem);
-		retval = copy_ldt(&mm->context, &old_mm->context);
+		retval = copy_ldt(mm, old_mm);
 		up(&old_mm->context.sem);
 	}
 	return retval;
@@ -202,7 +205,7 @@ static int write_ldt(struct mm_struct * 
 
 	down(&mm->context.sem);
 	if (ldt_info.entry_number >= mm->context.size) {
-		error = alloc_ldt(&mm->context, ldt_info.entry_number+1, 1);
+		error = alloc_ldt(mm, ldt_info.entry_number+1, 1);
 		if (error < 0)
 			goto out_unlock;
 	}

[uml-devel] Re: UML running on a SMP host: bug in SKAS3 patch

[Blaisorblade]

For me it's perfectly ok... I don't remember the exact traces, but some 
sporadical SKAS-related Oops were reported... in no case they were repeatable 
(since -V1 at least), so they were likely very thin races conditions.

I'm going to merge also the patches for PTRACE_SYSEMU_SINGLESTEP in SKAS, 
since I'm now at releasing SKAS updates.

I'm also going to add this patch, which fixes another potential problem:

diff -puN arch/i386/kernel/ptrace.c~skas-add-wmb-for-mm-switch 
arch/i386/kernel/ptrace.c
--- vanilla-linux-2.6.9/arch/i386/kernel/ptrace.c~skas-add-wmb-for-mm-switch    
2005-01-26 20:12:14.233441416 +0100
+++ vanilla-linux-2.6.9-paolo/arch/i386/kernel/ptrace.c 2005-01-26 
20:28:00.902525840 +0100
@@ -570,8 +570,10 @@ asmlinkage int sys_ptrace(long request,
                }

                atomic_inc(&new->mm_users);
+               task_lock(child);
                child->mm = new;
                child->active_mm = new;
+               task_unlock(child);
                mmput(old);
                ret = 0;
                break;

/*
 * Protects ->fs, ->files, ->mm, ->ptrace, ->group_info, ->comm and
 * synchronises with wait4().
 *
 * Nests both inside and outside of read_lock(&tasklist_lock).
 * It must not be nested with write_lock_irq(&tasklist_lock),
 * neither inside nor outside.
 */
static inline void task_lock(struct task_struct *p)
{
        spin_lock(&p->alloc_lock);
}

The release will wait at least a couple of days, since I'm adding all this 
stuff... especially, since there is a lock addition, I'd like at least one 
positive report from one SMP user before *officially* releasing.

I've not seen any update to them from their original version (i.e. when they 
were first discussed). Is this correct?

And my doubts about their technical merit were totally wrong. I simply was 
missing that SYSEMU stands to SYSEMU_SINGLESTEP as SYSCALL stands on 
SINGLESTEP (in correct, >=2.6.9 kernels), while I thought SYSEMU_SINGLESTEP 
was equal to the correct SINGLESTEP (I was maybe misguided by the "using it 
to check the host kernel correctness" hack we need to use).

-- 
Paolo Giarrusso, aka Blaisorblade
Linux registered user n. 292729
http://www.user-mode-linux.org/~blaisorblade


-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
User-mode-linux-devel mailing list
User-mode-linux-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/user-mode-linux-devel

[uml-devel] Re: UML running on a SMP host: bug in SKAS3 patch

[Bodo Stroesser]

[-- Attachment #1: Type: text/plain, Size: 981 bytes --]

Blaisorblade wrote:
> I've not seen any update to them from their original version (i.e. when they 
> were first discussed). Is this correct?
I guess, you're talking about my SYSEMU_SINGLESTEP patches. Since I created two
patches each for 2.6.9 and 2.6.7, you should have these four patches:

../tmp/patch-2.6.7-skas-v7-reorganize
../tmp/patch-2.6.7-skas-v7-add-SYSEMU_SINGLESTEP

../tmp/patch-2.6.9-skas-v7-reorganize
../tmp/patch-2.6.9-skas-v7-add-SYSEMU_SINGLESTEP

On my system, the change date is Nov 12. So, I guess the ones you have still
are up to date.

But on my 2.6.9, I added a further patch to catch some rare conditions, when
doing sysaudit together with ptracing, or switching between different ptrace
commands.
I don't know, if I ever sent it to the list. IIRC, uml runs fine without it
(but IMHO, we should use the more "complete" solution).
The sysaudit/ptrace issue is a general problem for i386, too.

Have attached a tarball containing all five patches.

Bodo

[-- Attachment #2: sysemu-singlestep.tar --]
[-- Type: application/x-tar, Size: 30720 bytes --]