Subject: Re: kern/36046 locking issue in gdt_get_slot
To: None <gnats-admin@netbsd.org, netbsd-bugs@netbsd.org,>
From: Andrew Doran <ad@NetBSD.org>
List: netbsd-bugs
Date: 04/13/2007 00:02:03
Currently testing the following:

Index: i386/gdt.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/gdt.c,v
retrieving revision 1.37
diff -u -r1.37 gdt.c
--- i386/gdt.c	15 Feb 2007 15:40:50 -0000	1.37
+++ i386/gdt.c	12 Apr 2007 23:00:49 -0000
@@ -305,22 +305,23 @@
 /*
  * Caller must have pmap locked for both of these functions.
  */
-void
-ldt_alloc(struct pmap *pmap, union descriptor *ldtp, size_t len)
+int
+ldt_alloc(union descriptor *ldtp, size_t len)
 {
 	int slot;
 
 	slot = gdt_get_slot();
 	setgdt(slot, ldtp, len - 1, SDT_SYSLDT, SEL_KPL, 0, 0);
-	pmap->pm_ldt_sel = GSEL(slot, SEL_KPL);
+
+	return GSEL(slot, SEL_KPL);
 }
 
 void
-ldt_free(struct pmap *pmap)
+ldt_free(int sel)
 {
 	int slot;
 
-	slot = IDXSEL(pmap->pm_ldt_sel);
+	slot = IDXSEL(sel);
 
 	gdt_put_slot(slot);
 }
Index: i386/pmap.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/pmap.c,v
retrieving revision 1.203
diff -u -r1.203 pmap.c
--- i386/pmap.c	12 Mar 2007 18:18:25 -0000	1.203
+++ i386/pmap.c	12 Apr 2007 23:00:51 -0000
@@ -1671,7 +1671,7 @@
 		 * No need to lock the pmap for ldt_free (or anything else),
 		 * we're the last one to use it.
 		 */
-		ldt_free(pmap);
+		ldt_free(pmap->pm_ldt_sel);
 		uvm_km_free(kernel_map, (vaddr_t)pmap->pm_ldt,
 		    pmap->pm_ldt_len * sizeof(union descriptor), UVM_KMF_WIRED);
 	}
@@ -1704,26 +1704,53 @@
 	struct pmap *pmap1, *pmap2;
 {
 #ifdef USER_LDT
+	union descriptor *new_ldt;
+	size_t len;
+	int sel;
+
+ retry:
+	if (pmap1->pm_flags & PMF_USER_LDT) {
+		len = pmap1->pm_ldt_len * sizeof(union descriptor);
+		new_ldt = (union descriptor *)uvm_km_alloc(kernel_map,
+		    len, 0, UVM_KMF_WIRED);
+		sel = ldt_alloc(new_ldt, len);
+	} else {
+		sel = -1;
+		len = -1;
+	}
+
 	simple_lock(&pmap1->pm_obj.vmobjlock);
 	simple_lock(&pmap2->pm_obj.vmobjlock);
 
 	/* Copy the LDT, if necessary. */
 	if (pmap1->pm_flags & PMF_USER_LDT) {
-		union descriptor *new_ldt;
-		size_t len;
+		if (len != pmap1->pm_ldt_len * sizeof(union descriptor)) {
+			simple_unlock(&pmap2->pm_obj.vmobjlock);
+			simple_unlock(&pmap1->pm_obj.vmobjlock);
+			if (sel != -1) {
+				ldt_free(sel);
+				uvm_km_free(kernel_map, (vaddr_t)new_ldt,
+				    len, UVM_KMF_WIRED);
+			}
+			goto retry;
+		}
 
-		len = pmap1->pm_ldt_len * sizeof(union descriptor);
-		new_ldt = (union descriptor *)uvm_km_alloc(kernel_map,
-		    len, 0, UVM_KMF_WIRED);
 		memcpy(new_ldt, pmap1->pm_ldt, len);
 		pmap2->pm_ldt = new_ldt;
 		pmap2->pm_ldt_len = pmap1->pm_ldt_len;
 		pmap2->pm_flags |= PMF_USER_LDT;
-		ldt_alloc(pmap2, new_ldt, len);
+		pmap2->pm_ldt_sel = sel;
+		sel = -1;
 	}
 
 	simple_unlock(&pmap2->pm_obj.vmobjlock);
 	simple_unlock(&pmap1->pm_obj.vmobjlock);
+
+	if (sel != -1) {
+		ldt_free(sel);
+		uvm_km_free(kernel_map, (vaddr_t)new_ldt, len,
+		    UVM_KMF_WIRED);
+	}
 #endif /* USER_LDT */
 }
 #endif /* PMAP_FORK */
@@ -1746,7 +1773,7 @@
 	simple_lock(&pmap->pm_obj.vmobjlock);
 
 	if (pmap->pm_flags & PMF_USER_LDT) {
-		ldt_free(pmap);
+		ldt_free(pmap->pm_ldt_sel);
 		pmap->pm_ldt_sel = GSEL(GLDT_SEL, SEL_KPL);
 		pcb->pcb_ldt_sel = pmap->pm_ldt_sel;
 		if (pcb == curpcb)
Index: i386/sys_machdep.c
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/i386/sys_machdep.c,v
retrieving revision 1.81
diff -u -r1.81 sys_machdep.c
--- i386/sys_machdep.c	4 Mar 2007 05:59:58 -0000	1.81
+++ i386/sys_machdep.c	12 Apr 2007 23:00:51 -0000
@@ -182,7 +182,7 @@
 	void *args;
 	register_t *retval;
 {
-	int error, i, n;
+	int error, i, n, sel, fsel;
 	struct proc *p = l->l_proc;
 	struct pcb *pcb = &l->l_addr->u_pcb;
 	pmap_t pmap = p->p_vmspace->vm_map.pmap;
@@ -280,6 +280,7 @@
 	}
 
 	/* allocate user ldt */
+	fsel = -1;
 	simple_lock(&pmap->pm_lock);
 	if (pmap->pm_ldt == 0 || (ua.start + ua.num) > pmap->pm_ldt_len) {
 		if (pmap->pm_flags & PMF_USER_LDT)
@@ -293,6 +294,8 @@
 		simple_unlock(&pmap->pm_lock);
 		new_ldt = (union descriptor *)uvm_km_alloc(kernel_map,
 		    new_len, 0, UVM_KMF_WIRED);
+		memset(new_ldt, 0, new_len);
+		sel = ldt_alloc(new_ldt, new_len);
 		simple_lock(&pmap->pm_lock);
 
 		if (pmap->pm_ldt != NULL && ldt_len <= pmap->pm_ldt_len) {
@@ -329,10 +332,12 @@
 		pmap->pm_ldt_len = ldt_len;
 
 		if (pmap->pm_flags & PMF_USER_LDT)
-			ldt_free(pmap);
-		else
+			fsel = pmap->pm_ldt_sel;
+		else {
 			pmap->pm_flags |= PMF_USER_LDT;
-		ldt_alloc(pmap, new_ldt, new_len);
+			fsel = -1;
+		}
+		pmap->pm_ldt_sel = sel;
 		pcb->pcb_ldt_sel = pmap->pm_ldt_sel;
 		if (pcb == curpcb)
 			lldt(pcb->pcb_ldt_sel);
@@ -344,9 +349,9 @@
 		pmap->pm_ldt[n] = descv[i];
 
 	simple_unlock(&pmap->pm_lock);
-
 	*retval = ua.start;
-
+	if (fsel != -1)
+		ldt_free(fsel);
 out:
 	free(descv, M_TEMP);
 	return (error);
Index: include/gdt.h
===================================================================
RCS file: /cvsroot/src/sys/arch/i386/include/gdt.h,v
retrieving revision 1.10
diff -u -r1.10 gdt.h
--- include/gdt.h	27 Oct 2003 13:44:20 -0000	1.10
+++ include/gdt.h	12 Apr 2007 23:00:51 -0000
@@ -47,8 +47,8 @@
 void gdt_alloc_cpu(struct cpu_info *);
 int tss_alloc(struct pcb *);
 void tss_free(int);
-void ldt_alloc(struct pmap *, union descriptor *, size_t);
-void ldt_free(struct pmap *);
+int ldt_alloc(union descriptor *, size_t);
+void ldt_free(int);
 
 #endif /* LOCORE */