Debug option to write-protect rodata: change_page_attr fixes

Hi,

I've been working on a patch that turns the kernel's .rodata section to be actually read only, eg any write attempts to it cause a segmentation fault.

This patch introduces the actual debug option to catch any writes to rodata

Quoted 6 lines · On Mon, 2005-11-07 at 10:58 +0000, [email protected] wrote:

Hi,

I've been working on a patch that turns the kernel's .rodata section to be actually read only, eg any write attempts to it cause a segmentation fault.

This patch introduces the actual debug option to catch any writes to rodata

Why a debug option? From what I can tell, it doesn't impact runtime performance much and it provides good protection. Any reason not to make it an always-on feature?

Quoted 11 lines · On Mon, 2005-11-07 at 10:58 +0000, [email protected] wrote:

Quoted 6 lines

Hi,

I've been working on a patch that turns the kernel's .rodata section to be actually read only, eg any write attempts to it cause a segmentation fault.

This patch introduces the actual debug option to catch any writes to rodata

Why a debug option? From what I can tell, it doesn't impact runtime performance much and it provides good protection. Any reason not to make it an always-on feature?

personally I'd like that but there is a chance of a tiny perf regression and usually there are people objecting to that.

(It's not clear cut: while the last bit of the kernel no longer is covered by a 2Mb tlb, most intel cpus have very few of such tlbs in the first place and this would free up one such tlb for other things (say the stack data) or even the userspace database), so it's not all that clear cut what the cost of this is)

people objecting to that.

Quoted 6 lines

(It's not clear cut: while the last bit of the kernel no longer is covered by a 2Mb tlb, most intel cpus have very few of such tlbs in the first place and this would free up one such tlb for other things (say the stack data) or even the userspace database), so it's not all that clear cut what the cost of this is)

I'm dumb. But how is "the last bit of the kernel no longer is covered by a 2Mb tlb"? Could you explain a bit more?

Quoted 10 lines

people objecting to that.

Quoted 6 lines

(It's not clear cut: while the last bit of the kernel no longer is covered by a 2Mb tlb, most intel cpus have very few of such tlbs in the first place and this would free up one such tlb for other things (say the stack data) or even the userspace database), so it's not all that clear cut what the cost of this is)

I'm dumb. But how is "the last bit of the kernel no longer is covered by a 2Mb tlb"? Could you explain a bit more?

in memory it'll look something like this

0 2 4 6
-- kernel text -- + -- kernel text -- + --- k. text-- rodata -- + --

normally the range from 0 to 6 is covered with 2Mb tlb's. Now to make rodata read only, the hugetlb entry covering 4-6 Mb range needs to be split into 4Kb entries, so that the rodata portion can have different permissions than the rest of that range.

And we could also mark text section read-only and data/stack section noexec if NX is supported. But I doubt the whole thing would really help much. Kill the kernel thread? We can't. We only run into a panic. Anyway I'd attach a quick patch to mark text section read only in the next mail.

If it's ok, I'd add Kconfig support. Comments?

Quoted 7 lines · Coywolf Qi Hunt

And we could also mark text section read-only and data/stack section noexec if NX is supported. But I doubt the whole thing would really help much. Kill the kernel thread? We can't. We only run into a panic. Anyway I'd attach a quick patch to mark text section read only in the next mail.

If it's ok, I'd add Kconfig support. Comments?

Quoted 7 lines · Coywolf Qi Hunt

And we could also mark text section read-only and data/stack section noexec if NX is supported. But I doubt the whole thing would really help much. Kill the kernel thread? We can't. We only run into a panic. Anyway I'd attach a quick patch to mark text section read only in the next mail.

If it's ok, I'd add Kconfig support. Comments?

Signed-off-by Coywolf Qi Hunt <[email protected]>

---

diff -pruN 2.6.14-mm2/init/main.c 2.6.14-mm2-cy/init/main.c

init/main.c +12 -0

@@ -660,6 +660,18 @@ static inline void fixup_cpu_present_map
 #endif
 }
 
+void mark_text_ro(void)
+{
+	unsigned long addr = (unsigned long)&_text;
+
+	for (; addr < (unsigned long)&_etext; addr += PAGE_SIZE)
+		change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RO);
+
+	printk ("Write protecting the kernel text data: %luk\n",
+			(unsigned long)(_etext - _text) >> 10);
+	global_flush_tlb();
+}
+
 static int init(void * unused)
 {
 	lock_kernel();

Quoted 37 lines · Coywolf Qi Hunt

Quoted 7 lines

And we could also mark text section read-only and data/stack section noexec if NX is supported. But I doubt the whole thing would really help much. Kill the kernel thread? We can't. We only run into a panic. Anyway I'd attach a quick patch to mark text section read only in the next mail.

If it's ok, I'd add Kconfig support. Comments?

Signed-off-by Coywolf Qi Hunt <[email protected]>

---

diff -pruN 2.6.14-mm2/init/main.c 2.6.14-mm2-cy/init/main.c

init/main.c 2005-11-12 02:50:45.000000000 +0800 renamed from init/main.c 2005-11-11 22:34:21.000000000 +0800 +12 -0

@@ -660,6 +660,18 @@ static inline void fixup_cpu_present_map
 #endif
 }
 
+void mark_text_ro(void)
+{
+     unsigned long addr = (unsigned long)&_text;
+
+     for (; addr < (unsigned long)&_etext; addr += PAGE_SIZE)
+             change_page_attr(virt_to_page(addr), 1, PAGE_KERNEL_RO);
+
+     printk ("Write protecting the kernel text data: %luk\n",
+                     (unsigned long)(_etext - _text) >> 10);
+     global_flush_tlb();
+}
+
 static int init(void * unused)
 {
      lock_kernel();

init/main.c 2005-11-12 02:50:45.000000000 +0800 static int init(void * unused)

Quoted 8 lines

init/main.c 2005-11-12 02:50:45.000000000 +0800 renamed from init/main.c 2005-11-11 22:34:21.000000000 +0800 +1 -1

@@ -716,6 +728,7 @@ static int init(void * unused)
       */
      free_initmem();
      unlock_kernel();
+     mark_text_ro();
      mark_rodata_ro();
      system_state = SYSTEM_RUNNING;
      numa_default_policy();
-

Assuming ix86, what is read-only? Certainly the text section needs to be marked execute!

So is it:
Execute-Only
Execute-Only, accessed
Execute/Read
Execute/Read, accessed
Execute-only, conforming Execute-only, conforming, accessed Execute/Read-Only, conforming Execute/Read-Only, conforming, accessed.
(all from page 5-12 of ix484 programmer's reference)

????

You need to WRITE to the text segment to be able to load executables (modules) and kernel threads. The user-mode code, children of init need to have writable .text segments to be able to exec().

Cheers,
Dick Johnson
Penguin : Linux version 2.6.13.4 on an i686 machine (5589.48 BogoMips). Warning : 98.36% of all statistics are fiction.
.

****************************************************************
The information transmitted in this message is confidential and may be privileged. Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited. If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to [email protected] - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Quoted 5 lines · Coywolf Qi Hunt

And we could also mark text section read-only and data/stack section noexec if NX is supported. But I doubt the whole thing would really help much. Kill the kernel thread? We can't. We only run into a panic. Anyway I'd attach a quick patch to mark text section read only in the next mail.

Overall I doubt it is worth it even as a debugging option. I so far cannot remember a single bug that was caused by overwriting kernel text.

-Andi

Andi Kleen writes:

[...]

Quoted 3 lines

Overall I doubt it is worth it even as a debugging option. I so far cannot remember a single bug that was caused by overwriting kernel text.

I wouldn't forget that one for a long time:

http://marc.theaimsgroup.com/?l=linux-kernel&m=106503116306729&w=2

Quoted 6 lines · Coywolf Qi Hunt

Quoted 5 lines

And we could also mark text section read-only and data/stack section noexec if NX is supported. But I doubt the whole thing would really help much. Kill the kernel thread? We can't. We only run into a panic. Anyway I'd attach a quick patch to mark text section read only in the next mail.

I think this whole thing is only usable as a debugging option. It shouldn't be used by default on production systems because it will increase TLB pressure by splitting up the large pages used by kernel. And TLB pressure is critical in many workloads.

It definitely shouldn't be on by default.

Then the text section will likely not be page aligned, so it would be surprising if it even worked.

At least on x86-64 it is pretty useless too because the .text section can be accessed over its alias in the direct mapping.

Overall I doubt it is worth it even as a debugging option. I so far cannot remember a single bug that was caused by overwriting kernel text.

2005/11/12, Andi Kleen <[email protected]>:

Quoted 14 lines · Coywolf Qi Hunt

Quoted 6 lines · Coywolf Qi Hunt

Quoted 5 lines

And we could also mark text section read-only and data/stack section noexec if NX is supported. But I doubt the whole thing would really help much. Kill the kernel thread? We can't. We only run into a panic. Anyway I'd attach a quick patch to mark text section read only in the next mail.

I think this whole thing is only usable as a debugging option. It shouldn't be used by default on production systems because it will increase TLB pressure by splitting up the large pages used by kernel. And TLB pressure is critical in many workloads.

Quoted 4 lines

It definitely shouldn't be on by default.

Then the text section will likely not be page aligned, so it would be surprising if it even worked.

It works. I have tested it with { c=_stext[0]; _stext[0]=c;}. No effect when it's disabled; panic when it's enabled.

The symbol _text' is always page aligned. _etext' is not, but we don't care.

(Bugs: It would conflict with kprobes.)

Quoted 3 lines

At least on x86-64 it is pretty useless too because the .text section can be accessed over its alias in the direct mapping.

OK, for x86 only then.

Quoted 3 lines

Overall I doubt it is worth it even as a debugging option. I so far cannot remember a single bug that was caused by overwriting kernel text.

I had the same concern basically. But I am convinced after seeing the bug Nikita Danilov points out.