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);
--
