svn commit: r315914 - in head/sys: amd64/amd64 i386/i386

Bruce Evans bde at FreeBSD.org
Fri Mar 24 17:34:56 UTC 2017


Author: bde
Date: Fri Mar 24 17:34:55 2017
New Revision: 315914
URL: https://svnweb.freebsd.org/changeset/base/315914

Log:
  Remove buggy adjustment of page tables in db_write_bytes().
  
  Long ago, perhaps only on i386, kernel text was mapped read-only and
  it was necessary to change the mapping to read-write to set breakpoints
  in kernel text.  Other writes by ddb to kernel text were also allowed.
  This write protection is harder to implement with 4MB pages, and was
  lost even for 4K pages when 4MB pages were implemented.  So changing
  the mapping became useless.  It was actually worse than useless since
  it followed followed various null and otherwise garbage pointers to
  not change random memory instead of the mapping.  (On i386s, the
  pointers became good in pmap_bootstrap(), and on amd64 the pointers
  became bad in pmap_bootstrap() if not before.)
  
  Another bug broke detection of following of null pointers on i386,
  except early in boot where not detecting this was a feature.  When
  I fixed the bug, I accidentally broke the feature and soon got traps
  in db_write_bytes().  Setting breakpoints early in ddb was broken.
  
  kib pointed out that a clean way to do the adjustment would be to use
  a special [sub]map giving a small window on the bytes to be written.
  
  The trap handler didn't know how to fix up errors for pagefaults
  accessing the map itself.  Such errors rarely need fixups, since most
  traps for the map are for the first access which is a read.
  
  Reviewed by:	kib

Modified:
  head/sys/amd64/amd64/db_interface.c
  head/sys/i386/i386/db_interface.c

Modified: head/sys/amd64/amd64/db_interface.c
==============================================================================
--- head/sys/amd64/amd64/db_interface.c	Fri Mar 24 16:26:11 2017	(r315913)
+++ head/sys/amd64/amd64/db_interface.c	Fri Mar 24 17:34:55 2017	(r315914)
@@ -30,17 +30,11 @@ __FBSDID("$FreeBSD$");
 /*
  * Interface to new debugger.
  */
+
 #include <sys/param.h>
 #include <sys/systm.h>
 #include <sys/kdb.h>
-#include <sys/cons.h>
 #include <sys/pcpu.h>
-#include <sys/proc.h>
-
-#include <machine/cpu.h>
-
-#include <vm/vm.h>
-#include <vm/pmap.h>
 
 #include <ddb/ddb.h>
 
@@ -75,63 +69,16 @@ db_write_bytes(vm_offset_t addr, size_t 
 	jmp_buf jb;
 	void *prev_jb;
 	char *dst;
-	pt_entry_t	*ptep0 = NULL;
-	pt_entry_t	oldmap0 = 0;
-	vm_offset_t	addr1;
-	pt_entry_t	*ptep1 = NULL;
-	pt_entry_t	oldmap1 = 0;
 	int ret;
 
 	prev_jb = kdb_jmpbuf(jb);
 	ret = setjmp(jb);
 	if (ret == 0) {
-		if (addr > trunc_page((vm_offset_t)btext) - size &&
-		    addr < round_page((vm_offset_t)etext)) {
-
-			ptep0 = vtopte(addr);
-			oldmap0 = *ptep0;
-			*ptep0 |= PG_RW;
-
-			/*
-			 * Map another page if the data crosses a page
-			 * boundary.
-			 */
-			if ((*ptep0 & PG_PS) == 0) {
-				addr1 = trunc_page(addr + size - 1);
-				if (trunc_page(addr) != addr1) {
-					ptep1 = vtopte(addr1);
-					oldmap1 = *ptep1;
-					*ptep1 |= PG_RW;
-				}
-			} else {
-				addr1 = trunc_2mpage(addr + size - 1);
-				if (trunc_2mpage(addr) != addr1) {
-					ptep1 = vtopte(addr1);
-					oldmap1 = *ptep1;
-					*ptep1 |= PG_RW;
-				}
-			}
-
-			invltlb();
-		}
-
 		dst = (char *)addr;
-
 		while (size-- > 0)
 			*dst++ = *data++;
 	}
-
 	(void)kdb_jmpbuf(prev_jb);
-
-	if (ptep0) {
-		*ptep0 = oldmap0;
-
-		if (ptep1)
-			*ptep1 = oldmap1;
-
-		invltlb();
-	}
-
 	return (ret);
 }
 

Modified: head/sys/i386/i386/db_interface.c
==============================================================================
--- head/sys/i386/i386/db_interface.c	Fri Mar 24 16:26:11 2017	(r315913)
+++ head/sys/i386/i386/db_interface.c	Fri Mar 24 17:34:55 2017	(r315914)
@@ -30,17 +30,14 @@ __FBSDID("$FreeBSD$");
 /*
  * Interface to new debugger.
  */
+
 #include <sys/param.h>
 #include <sys/systm.h>
-#include <sys/cons.h>
 #include <sys/kdb.h>
 #include <sys/pcpu.h>
 #include <sys/proc.h>
 
-#include <machine/cpu.h>
-
-#include <vm/vm.h>
-#include <vm/pmap.h>
+#include <machine/psl.h>
 
 #include <ddb/ddb.h>
 
@@ -75,63 +72,16 @@ db_write_bytes(vm_offset_t addr, size_t 
 	jmp_buf jb;
 	void *prev_jb;
 	char *dst;
-	pt_entry_t *ptep0 = NULL;
-	pt_entry_t oldmap0 = 0;
-	vm_offset_t addr1;
-	pt_entry_t *ptep1 = NULL;
-	pt_entry_t oldmap1 = 0;
 	int ret;
 
 	prev_jb = kdb_jmpbuf(jb);
 	ret = setjmp(jb);
 	if (ret == 0) {
-		if (addr > trunc_page((vm_offset_t)btext) - size &&
-		    addr < round_page((vm_offset_t)etext)) {
-
-			ptep0 = pmap_pte(kernel_pmap, addr);
-			oldmap0 = *ptep0;
-			*ptep0 |= PG_RW;
-
-			/*
-			 * Map another page if the data crosses a page
-			 * boundary.
-			 */
-			if ((*ptep0 & PG_PS) == 0) {
-				addr1 = trunc_page(addr + size - 1);
-				if (trunc_page(addr) != addr1) {
-					ptep1 = pmap_pte(kernel_pmap, addr1);
-					oldmap1 = *ptep1;
-					*ptep1 |= PG_RW;
-				}
-			} else {
-				addr1 = trunc_4mpage(addr + size - 1);
-				if (trunc_4mpage(addr) != addr1) {
-					ptep1 = pmap_pte(kernel_pmap, addr1);
-					oldmap1 = *ptep1;
-					*ptep1 |= PG_RW;
-				}
-			}
-
-			invltlb();
-		}
-
 		dst = (char *)addr;
-
 		while (size-- > 0)
 			*dst++ = *data++;
 	}
-
 	(void)kdb_jmpbuf(prev_jb);
-
-	if (ptep0) {
-		*ptep0 = oldmap0;
-
-		if (ptep1)
-			*ptep1 = oldmap1;
-
-		invltlb();
-	}
-
 	return (ret);
 }
 


More information about the svn-src-all mailing list