Re: kgdb test suite failure

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Alexander Beregalov <a.beregalov@...>
Cc: <kgdb-bugreport@...>, <linux-kernel@...>, Ingo Molnar <mingo@...>
Date: Wednesday, May 21, 2008 - 12:27 pm

Alexander Beregalov wrote:


I duplicated the problem by trying the kernel out with the
CONFIG_DEBUG_RODATA, which appears to only be implemented on the x86
architecture.  I am not exactly certain what the right level of fix
should be in this case, or if it should be fixed at all.  It seems
that the probe_kernel_write should have a force option to force
writing to a read-only memory page (meaning it will be
unprotected/re-protected), or there should be another function to deal
with writes that are deemed to be critical which calls the
probe_kernel_write() from with in kgdb.

It is an interesting problem, but the first inclination is not to fix
it at all because debugging read-only text sections is generally a job
for hardware breakpoints.  The kgdb test suite fell into the trap of
trying to debug a function call that was in a read-only page using
software breakpoints with trap instructions.

An RFC type patch follows which addresses the problem, but in my
opinion not in a terribly clean way...  Perhaps other folks will
comment on a different or better way to approach the problem or chime
in on if it should be fixed at all.  Certainly the test suite could be
changed to use HW breakpoints for the case where the text segments are
going to be read only as an example.

Another approach I considered was to add a global option to kgdb which
causes it to force write to read only pages.  This could be used by
the kgdb test suite as well as by a "human" via a debugger to change
the setting as needed.  How often you might want to set a software
breakpoint in a read-only page remains to be seen, but it does seem
like something that is marginally useful.

Jason.


===RFC only to illustrate a work around===

---
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index 60bcb5b..e4d0039 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -737,7 +737,7 @@ static int change_page_attr_set_clr(unsigned long addr, int numpages,
 	/*
 	 * Check whether we really changed something:
 	 */
-	if (!cpa.flushtlb)
+	if (!cpa.flushtlb || in_atomic())
 		goto out;
 
 	/*
diff --git a/mm/maccess.c b/mm/maccess.c
index ac40796..7609fb9 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -4,6 +4,7 @@
 #include <linux/uaccess.h>
 #include <linux/module.h>
 #include <linux/mm.h>
+#include <asm/cacheflush.h>
 
 /**
  * probe_kernel_read(): safely attempt to read from a location
@@ -42,11 +43,24 @@ EXPORT_SYMBOL_GPL(probe_kernel_read);
 long probe_kernel_write(void *dst, void *src, size_t size)
 {
 	long ret;
+#ifdef CONFIG_X86
+	unsigned int level;
+	pte_t *pte = lookup_address((unsigned long)dst, &level);
+	int unprotect = !pte_write(*pte);
+#endif
 	mm_segment_t old_fs = get_fs();
 
 	set_fs(KERNEL_DS);
 	pagefault_disable();
+#ifdef CONFIG_X86
+	if (unprotect)
+		set_memory_rw(((unsigned long)dst) & PAGE_MASK, 1);
+#endif
 	ret = __copy_to_user_inatomic((__force void __user *)dst, src, size);
+#ifdef CONFIG_X86
+	if (unprotect)
+		set_memory_ro(((unsigned long)dst) & PAGE_MASK, 1);
+#endif
 	pagefault_enable();
 	set_fs(old_fs);
 
--
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
kgdb test suite failure, Alexander Beregalov, (Fri May 9, 10:14 am)
Re: kgdb test suite failure, Jason Wessel, (Tue May 20, 11:18 am)
Re: kgdb test suite failure, Alexander Beregalov, (Tue May 20, 2:30 pm)
Re: kgdb test suite failure, Jason Wessel, (Wed May 21, 12:27 pm)
Re: kgdb test suite failure, Thomas Gleixner, (Thu May 22, 5:14 pm)
Re: kgdb test suite failure, Jason Wessel, (Tue May 27, 9:37 pm)
Re: kgdb test suite failure, David Miller, (Fri May 23, 3:01 am)
Re: kgdb test suite failure, Jason Wessel, (Tue May 27, 9:43 pm)
Re: kgdb test suite failure, Rafael J. Wysocki, (Sun May 11, 3:32 pm)