2.6.22-rc5: known regressions with patches

PATCH In review Michal Piotrowski 10 participants 25 messages Jun 17, 2007 – Jun 21, 2007

lore

Share & tools

lore

Michal Piotrowski

author

2007-06-17 14:24 UTC #0 lore raw

PATCH 1/2 2.6.22-rc5: known regressions with patches

Hi all,

Here is a list of some known regressions in 2.6.22-rc5 with patches available.

Feel free to add new regressions/remove fixed etc.
http://kernelnewbies.org/known_regressions

Unclassified

Subject : CONFIG_DEBUG_RODATA prevents kprobes from working on 2.6.22-rc2
References : http://lkml.org/lkml/2007/5/23/202
Submitter : William Cohen <[email protected]>
Ian McDonald <[email protected]>

Handled-By S. P. Prasanna <[email protected]>

Patch : http://lkml.org/lkml/2007/6/7/2
Status : patch available

Subject : long freezes on thinkpad t60
References : http://lkml.org/lkml/2007/5/24/100
Submitter : Miklos Szeredi <[email protected]>

Handled-By Ingo Molnar <[email protected]>

Patch : http://lkml.org/lkml/2007/6/16/81
Status : patch was suggested

Subject : OProfile issues
References : http://lkml.org/lkml/2007/6/12/207
Submitter : Stephane Eranian <[email protected]>

Handled-By Björn Steinbrink <[email protected]>

Patch : http://lkml.org/lkml/2007/6/12/392
Status : patch was suggested

FBDEV

Subject : mach64 breakage in 2.6.22
References : http://lkml.org/lkml/2007/6/14/73
Submitter : Olaf Hering <[email protected]>

Handled-By Ville Syrjälä <[email protected]>

Patch : http://lkml.org/lkml/2007/6/14/273
Status : patch was suggested

HWMON

Subject : latest coretemp changes break s2ram
References : http://lkml.org/lkml/2007/6/16/173
Submitter : Soeren Sonnenburg <[email protected]>

Caused-By Rudolf Marek <[email protected]>

            commit 67f363b1f6a31cf5027a97372f64bcced4f05ba6

Handled-By Jean Delvare <[email protected]>

Patch : http://lkml.org/lkml/2007/6/16/177
Status : patch available

Regards,
Michal

--
LOG
http://www.stardust.webpages.pl/log/

Ingo Molnar

2007-06-17 15:16 UTC #1 lore raw

Quoted 6 lines · Michal Piotrowski

Subject : long freezes on thinkpad t60
References : http://lkml.org/lkml/2007/5/24/100
Submitter : Miklos Szeredi <[email protected]>
Handled-By Ingo Molnar <[email protected]>
Patch : http://lkml.org/lkml/2007/6/16/81
Status : patch was suggested

i think this isnt a new regression in .22, it existed in .21 too - or are you tracking all 2.6 regressions?

Ingo

Michal Piotrowski

author

2007-06-17 15:31 UTC #2 lore raw

Quoted 10 lines · Ingo Molnar

Quoted 6 lines · Michal Piotrowski
Subject : long freezes on thinkpad t60
References : http://lkml.org/lkml/2007/5/24/100
Submitter : Miklos Szeredi <[email protected]>
Handled-By Ingo Molnar <[email protected]>
Patch : http://lkml.org/lkml/2007/6/16/81
Status : patch was suggested

Quoted 2 lines

i think this isnt a new regression in .22, it existed in .21 too - or are you tracking all 2.6 regressions?

Unfortunately I don't have enough time, will and tools to track all 2.6 regressions.

Anyway, I'll leave it on the list. We've got a fix :)

Quoted 3 lines

Ingo

Regards,
Michal

Signature · 2 lines

LOG
http://www.stardust.webpages.pl/log/

Ian McDonald

2007-06-20 22:08 UTC #3 lore raw

In reply to Michal Piotrowski

Andrew,

Can this one be merged so that it makes it into 2.6.22?

Quoted 9 lines

Subject : CONFIG_DEBUG_RODATA prevents kprobes from working on 2.6.22-rc2
References : http://lkml.org/lkml/2007/5/23/202
Submitter : William Cohen <[email protected]>
Ian McDonald <[email protected]>
Handled-By S. P. Prasanna <[email protected]>
Patch : http://lkml.org/lkml/2007/6/7/2
Status : patch available

Regards,

Ian

Signature · 3 lines

Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

Chuck Ebbert

2007-06-20 22:32 UTC #4 lore raw

Quoted 13 lines · Ian McDonald

Andrew,
Can this one be merged so that it makes it into 2.6.22?
Quoted 9 lines
Subject : CONFIG_DEBUG_RODATA prevents kprobes from working on
2.6.22-rc2
References : http://lkml.org/lkml/2007/5/23/202
Submitter : William Cohen <[email protected]>
Ian McDonald <[email protected]>
Handled-By S. P. Prasanna <[email protected]>
Patch : http://lkml.org/lkml/2007/6/7/2
Status : patch available

Patch was just merged:

http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=55181000cd60334fe920c65ffbcdfe0e3f1de406

Ian McDonald

2007-06-20 22:38 UTC #5 lore raw

Quoted 9 lines · Chuck Ebbert

Quoted 8 lines
Quoted 8 lines
Subject : CONFIG_DEBUG_RODATA prevents kprobes from working on
2.6.22-rc2
References : http://lkml.org/lkml/2007/5/23/202
Submitter : William Cohen <[email protected]>
Ian McDonald <[email protected]>
Handled-By S. P. Prasanna <[email protected]>
Patch : http://lkml.org/lkml/2007/6/7/2
Status : patch available

Quoted 4 lines

Patch was just merged:
http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=55181000cd60334fe920c65ffbcdfe0e3f1de406

I see that just makes KPROBES and DEBUG_RODATA mutually exclusive which makes sense for 2.6.22

Michal - don't kill this one so we have the full fix in 2.6.23

Ian

Signature · 3 lines

Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

Linus Torvalds

2007-06-20 22:39 UTC #7 lore raw

In reply to Ian McDonald

Quoted 13 lines · Ian McDonald

Can this one be merged so that it makes it into 2.6.22?
Quoted 9 lines
Subject : CONFIG_DEBUG_RODATA prevents kprobes from working on 2.6.22-rc2
References : http://lkml.org/lkml/2007/5/23/202
Submitter : William Cohen <[email protected]>
Ian McDonald <[email protected]>
Handled-By S. P. Prasanna <[email protected]>
Patch : http://lkml.org/lkml/2007/6/7/2
Status : patch available

Quoted 1 line

Regards,

Well, for 2.6.22, Kprobes will just be disabled if you use DEBUG_RODATA.

And yes, that patch already got merged. However, the patch to *allow* Kprobes with DEBUG_RODATA is not, and will not be. It's not a regression, and quite frankly, I don't think I would even want that patch.

Kprobes fundamntally disagrees with DEBUG_RODATA, there's no point in "working around it". Better just admit it.

Linus

Ian McDonald

2007-06-20 22:41 UTC #8 lore raw

Quoted 11 lines · Linus Torvalds

Well, for 2.6.22, Kprobes will just be disabled if you use DEBUG_RODATA.
And yes, that patch already got merged. However, the patch to *allow* Kprobes with DEBUG_RODATA is not, and will not be. It's not a regression, and quite frankly, I don't think I would even want that patch.
Kprobes fundamntally disagrees with DEBUG_RODATA, there's no point in "working around it". Better just admit it.
Linus

It depends on the purpose of DEBUG_RODATA. If DEBUG_RODATA was for security reasons then I agree, but it seems to be more to catch accidental writes. Kprobes isn't an accidental write and I would suspect many developers would want to catch accidental writes and be able to insert kprobes.

Or is there something else I'm missing - i.e. you're saying the patch itself is crap?

Ian

Signature · 3 lines

Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

Linus Torvalds

2007-06-20 22:43 UTC #6 lore raw

In reply to Chuck Ebbert

Quoted 2 lines · Chuck Ebbert

Patch was just merged:

Well, that was the patch to make the conflict explicit, and just not allowing DEBUG_RODATA with KPROBES.

(And it has a "temporary" marker, which I actually think is wrong).

Linus

Linus Torvalds

2007-06-20 22:56 UTC #9 lore raw

In reply to Ian McDonald

Quoted 4 lines · Ian McDonald

It depends on the purpose of DEBUG_RODATA. If DEBUG_RODATA was for security reasons then I agree, but it seems to be more to catch accidental writes.

Well, I'd say that it is *one* tool for debugging.

Now, Kprobes is another tool - and I'm just saying that I don't see why you should really expect to break one tool with the other. They take different approaches.

And yes, sometimes debugging *does* change what it debugs. In timing, if nothing else, but also in the kinds of things you can do. For example, we don't allow slab redzoning on data structures that have alignment restrictions not compatible with the redzoning, and I'd argue that this is more of the same: we just should not do DEBUG_RODATA if you expect to change read-only data.

There's just no *point*.

Linus

Ian McDonald

2007-06-20 23:02 UTC #10 lore raw

Quoted 2 lines · Linus Torvalds

And yes, sometimes debugging *does* change what it debugs. In timing, if nothing else, but also in the kinds of things you can do.

Totally agree.

Quoted 6 lines

For example, we
don't allow slab redzoning on data structures that have alignment restrictions not compatible with the redzoning, and I'd argue that this is more of the same: we just should not do DEBUG_RODATA if you expect to change read-only data.

Well the description for DEBUG_RODATA is:
Mark the kernel read-only data as write-protected in the pagetables, in order to catch accidental (and incorrect) writes to such const data.

What we are doing with kprobes is neither accidental, nor incorrect.

Quoted 4 lines

There's just no *point*.
Linus

I understand your philosophical viewpoint here. Anyway it's no biggie as can keep the patch out of tree if I want it... Another option is to be able to enable and disable protecting read only data via a sysctl but that seems even uglier.

Anyway I'll stop wasting your time now.

Ian

Signature · 3 lines

Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

Linus Torvalds

2007-06-20 23:06 UTC #11 lore raw

In reply to Linus Torvalds

Quoted 2 lines · Linus Torvalds

There's just no *point*.

Put another way: we lived without DEBUG_RODATA for fifteen years, why should we now start adding complexity to work around code that doesn't accept the (fairly small) debugging it gives?

Has anybody actually found a bug using it?

As far as I know, the biggest reason to use DEBUG_RODATA is

(a) Hey, it's a cheap way to check one thing

(b) An added layer of security (which it's not that great for, but it might make sense to make it more of a real security feature).

and neither of them really seems to say "let's add more code to other pieces of the kernel to work around it" to me.

Linus

Dave Jones

2007-06-20 23:09 UTC #13 lore raw

In reply to Linus Torvalds

Quoted 6 lines · Linus Torvalds

And yes, that patch already got merged. However, the patch to *allow* Kprobes with DEBUG_RODATA is not, and will not be. It's not a regression, and quite frankly, I don't think I would even want that patch.
Kprobes fundamntally disagrees with DEBUG_RODATA, there's no point in "working around it". Better just admit it.

Surely the fundamental disagreement is only due to DEBUG_RODATA covering write-protection of both .text, and .rodata ? I can see value in having a kernel that supports kprobes, whilst at the same point, raising red flags if something writes into a const string. With my distro kernel maintainer hat on, I always hate these 'pick one' decisions, because I always get convincing arguments from proponents of both sides.

Was it always this way? I thought DEBUG_RODATA initially just
covered, well.. rodata. And kprobes only wants to change .text
doesn't it ?

Dave

Signature · 1 line

http://www.codemonkey.org.uk

Arjan van de Ven

2007-06-20 23:19 UTC #14 lore raw

Quoted 17 lines · Dave Jones

Quoted 6 lines · Linus Torvalds
And yes, that patch already got merged. However, the patch to *allow* Kprobes with DEBUG_RODATA is not, and will not be. It's not a regression, and quite frankly, I don't think I would even want that patch.
Kprobes fundamntally disagrees with DEBUG_RODATA, there's no point in "working around it". Better just admit it.
Surely the fundamental disagreement is only due to DEBUG_RODATA covering write-protection of both .text, and .rodata ? I can see value in having a kernel that supports kprobes, whilst at the same point, raising red flags if something writes into a const string. With my distro kernel maintainer hat on, I always hate these 'pick one' decisions, because I always get convincing arguments from proponents of both sides.

Quoted 3 lines

Was it always this way? I thought DEBUG_RODATA initially just
covered, well.. rodata. And kprobes only wants to change .text
doesn't it ?

no this got "fixed" recently. It used to only cover data. Andi merged a patch to make it cover text too.. imo we should reverse that, or make the check better and not have it cover text if kprobes is active. I can do the later if people are ok with that, it's approximately 3 lines of code.

Arjan van de Ven

2007-06-20 23:24 UTC #19 lore raw

In reply to Linus Torvalds

Quoted 3 lines

Kprobes fundamntally disagrees with DEBUG_RODATA, there's no point in "working around it". Better just admit it.

that's wrong. KPROBE fundamentally disagrees with TEXT being read only, which is a 2.6.22 new "feature".. and a buggy one at that.

the real fix would be something like this instead:

arch/x86_64/mm/init.c renamed from arch/x86_64/mm/init.c.org +5 -0

@@ -605,6 +605,11 @@ void mark_rodata_ro(void)
 	if (num_possible_cpus() > 1)
 		start = (unsigned long)_etext;
 #endif
+
+#ifdef CONFIG_KPROBES
+	start = (unsigned long)_etext;
+#endif
+	
 	end = (unsigned long)__end_rodata;
 	start = (start + PAGE_SIZE - 1) & PAGE_MASK;
 	end &= PAGE_MASK;

Dave Jones

2007-06-20 23:38 UTC #15 lore raw

In reply to Arjan van de Ven

Quoted 22 lines · Arjan van de Ven

Quoted 17 lines · Dave Jones
Quoted 6 lines · Linus Torvalds
And yes, that patch already got merged. However, the patch to *allow* Kprobes with DEBUG_RODATA is not, and will not be. It's not a regression, and quite frankly, I don't think I would even want that patch.
Kprobes fundamntally disagrees with DEBUG_RODATA, there's no point in "working around it". Better just admit it.
Surely the fundamental disagreement is only due to DEBUG_RODATA covering write-protection of both .text, and .rodata ? I can see value in having a kernel that supports kprobes, whilst at the same point, raising red flags if something writes into a const string. With my distro kernel maintainer hat on, I always hate these 'pick one' decisions, because I always get convincing arguments from proponents of both sides.
Quoted 3 lines
Was it always this way? I thought DEBUG_RODATA initially just
covered, well.. rodata. And kprobes only wants to change .text
doesn't it ?

Quoted 5 lines

no this got "fixed" recently. It used to only cover data. Andi merged a patch to make it cover text too.. imo we should reverse that, or make the check better and not have it cover text if kprobes is active. I can do the later if people are ok with that, it's approximately 3 lines of code.

Having the text as a separate option makes sense to me.
(Or at the least we should rename DEBUG_RODATA, as it's now misleading).

Dave

Signature · 1 line

http://www.codemonkey.org.uk

Linus Torvalds

2007-06-20 23:51 UTC #16 lore raw

In reply to Dave Jones

Quoted 3 lines · Dave Jones

Surely the fundamental disagreement is only due to DEBUG_RODATA covering write-protection of both .text, and .rodata ?

I agree that we could well split DEBUG_RODATA into something more fine-grained, and for example have it _only_ protect that .rodata thing when Kprobes are enabled, and both .text _and_ .rodata when Kprobes are not.

That would make lots of sense.

Linus

Linus Torvalds

2007-06-20 23:51 UTC #20 lore raw

In reply to Arjan van de Ven

Quoted 2 lines · Arjan van de Ven

the real fix would be something like this instead:

If people can test this, and confirm it works, please send a patch that not only does this ad undoes the Kconfig language. It looks like the right thing to do, but I won't touch it without somebody who actually tested these combinarions sending in a patch.

Hmm?

Linus

Arjan van de Ven

2007-06-21 05:27 UTC #21 lore raw

Quoted 9 lines · Linus Torvalds

Quoted 2 lines · Arjan van de Ven
the real fix would be something like this instead:
If people can test this, and confirm it works, please send a patch that not only does this ad undoes the Kconfig language. It looks like the right thing to do, but I won't touch it without somebody who actually tested these combinarions sending in a patch.

Hi,

I have tested this on x86_64, and without the config language, the original oopses, while with the patch below it works fine (as expected). I've not been able to test the i386 one (no 32 bit testboxes since 2 years) but the change is even simpler there, just an ifdef around the entire kernel text marking.

Do not mark the kernel text read only if KPROBES is in the kernel; kprobes needs to hot-patch the kernel text to insert it's instrumentation. In this case, only mark the .rodata segment as read only.

Signed-off-by Arjan van de Ven <[email protected]>

arch/i386/Kconfig.debug renamed from arch/i386/Kconfig.debug.org +0 -1

@@ -49,7 +49,6 @@ config DEBUG_PAGEALLOC
 config DEBUG_RODATA
 	bool "Write protect kernel read-only data structures"
 	depends on DEBUG_KERNEL
-	depends on !KPROBES # temporary for 2.6.22
 	help
 	  Mark the kernel read-only data as write-protected in the pagetables,
 	  in order to catch accidental (and incorrect) writes to such const

arch/x86_64/Kconfig.debug renamed from arch/x86_64/Kconfig.debug.org +0 -1

@@ -9,7 +9,6 @@ source "lib/Kconfig.debug"
 config DEBUG_RODATA
        bool "Write protect kernel read-only data structures"
        depends on DEBUG_KERNEL
-       depends on !KPROBES # temporary for 2.6.22
        help
 	 Mark the kernel read-only data as write-protected in the pagetables,
 	 in order to catch accidental (and incorrect) writes to such const data.

arch/i386/mm/init.c renamed from arch/i386/mm/init.c.org +2 -1

@@ -799,6 +799,7 @@ void mark_rodata_ro(void)
 	unsigned long start = PFN_ALIGN(_text);
 	unsigned long size = PFN_ALIGN(_etext) - start;
 
+#ifndef CONFIG_KPROBES
 #ifdef CONFIG_HOTPLUG_CPU
 	/* It must still be possible to apply SMP alternatives. */
 	if (num_possible_cpus() <= 1)

@@ -808,7 +809,7 @@ void mark_rodata_ro(void)
 		                 size >> PAGE_SHIFT, PAGE_KERNEL_RX);
 		printk("Write protecting the kernel text: %luk\n", size >> 10);
 	}
-
+#endif
 	start += size;
 	size = (unsigned long)__end_rodata - start;
 	change_page_attr(virt_to_page(start),

arch/x86_64/mm/init.c renamed from arch/x86_64/mm/init.c.org +5 -0

@@ -605,6 +605,11 @@ void mark_rodata_ro(void)
 	if (num_possible_cpus() > 1)
 		start = (unsigned long)_etext;
 #endif
+
+#ifdef CONFIG_KPROBES
+	start = (unsigned long)__start_rodata;
+#endif
+	
 	end = (unsigned long)__end_rodata;
 	start = (start + PAGE_SIZE - 1) & PAGE_MASK;
 	end &= PAGE_MASK;

Signature · 2 lines

if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

S. P. Prasanna

2007-06-21 07:11 UTC #22 lore raw

Quoted 11 lines · Arjan van de Ven

Quoted 9 lines · Linus Torvalds
Quoted 2 lines · Arjan van de Ven
the real fix would be something like this instead:
If people can test this, and confirm it works, please send a patch that not only does this ad undoes the Kconfig language. It looks like the right thing to do, but I won't touch it without somebody who actually tested these combinarions sending in a patch.

Quoted 7 lines

Hi,
I have tested this on x86_64, and without the config language, the original oopses, while with the patch below it works fine (as expected). I've not been able to test the i386 one (no 32 bit testboxes since 2 years) but the change is even simpler there, just an ifdef around the entire kernel text marking.

I tested this patch on i386 box, it seems to work fine.

Thanks
Prasanna

Quoted 67 lines

Do not mark the kernel text read only if KPROBES is in the kernel; kprobes needs to hot-patch the kernel text to insert it's instrumentation. In this case, only mark the .rodata segment as read only.

Signed-off-by Arjan van de Ven <[email protected]>

arch/i386/Kconfig.debug renamed from arch/i386/Kconfig.debug.org +0 -1

@@ -49,7 +49,6 @@ config DEBUG_PAGEALLOC
 config DEBUG_RODATA
 	bool "Write protect kernel read-only data structures"
 	depends on DEBUG_KERNEL
-	depends on !KPROBES # temporary for 2.6.22
 	help
 	  Mark the kernel read-only data as write-protected in the pagetables,
 	  in order to catch accidental (and incorrect) writes to such const

arch/x86_64/Kconfig.debug renamed from arch/x86_64/Kconfig.debug.org +0 -1

@@ -9,7 +9,6 @@ source "lib/Kconfig.debug"
 config DEBUG_RODATA
        bool "Write protect kernel read-only data structures"
        depends on DEBUG_KERNEL
-       depends on !KPROBES # temporary for 2.6.22
        help
 	 Mark the kernel read-only data as write-protected in the pagetables,
 	 in order to catch accidental (and incorrect) writes to such const data.

arch/i386/mm/init.c renamed from arch/i386/mm/init.c.org +2 -1

@@ -799,6 +799,7 @@ void mark_rodata_ro(void)
 	unsigned long start = PFN_ALIGN(_text);
 	unsigned long size = PFN_ALIGN(_etext) - start;
 
+#ifndef CONFIG_KPROBES
 #ifdef CONFIG_HOTPLUG_CPU
 	/* It must still be possible to apply SMP alternatives. */
 	if (num_possible_cpus() <= 1)

@@ -808,7 +809,7 @@ void mark_rodata_ro(void)
 		                 size >> PAGE_SHIFT, PAGE_KERNEL_RX);
 		printk("Write protecting the kernel text: %luk\n", size >> 10);
 	}
-
+#endif
 	start += size;
 	size = (unsigned long)__end_rodata - start;
 	change_page_attr(virt_to_page(start),

arch/x86_64/mm/init.c renamed from arch/x86_64/mm/init.c.org +5 -0

@@ -605,6 +605,11 @@ void mark_rodata_ro(void)
 	if (num_possible_cpus() > 1)
 		start = (unsigned long)_etext;
 #endif
+
+#ifdef CONFIG_KPROBES
+	start = (unsigned long)__start_rodata;
+#endif
+	
 	end = (unsigned long)__end_rodata;
 	start = (start + PAGE_SIZE - 1) & PAGE_MASK;
 	end &= PAGE_MASK;

Signature · 3 lines

-- 
if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

Signature · 6 lines

Thanks & Regards
Prasanna S.P.
Linux Technology Center
India Software Labs, IBM Bangalore
Email: [email protected]
Ph: 91-80-41776329

Andi Kleen

2007-06-21 09:20 UTC #17 lore raw

PATCH 1/1 Alternative fix for kprobes&DEBUG_RODATA was Re: [1/2] 2.6.22-rc5: known regressions with patches

In reply to Linus Torvalds

Quoted 6 lines · On Thursday 21 June 2007 01:48:43 Linus Torvalds wrote:

Quoted 3 lines · Dave Jones
Surely the fundamental disagreement is only due to DEBUG_RODATA covering write-protection of both .text, and .rodata ?

Quoted 6 lines

I agree that we could well split DEBUG_RODATA into something more fine-grained, and for example have it _only_ protect that .rodata thing when Kprobes are enabled, and both .text _and_ .rodata when Kprobes are not.
That would make lots of sense.

Ok, here's a patch to do this. With that 55181000cd60334fe920c65ffbcdfe0e3f1de406 should be reverted because it isn't needed anymore.

I still think in .23 it should be fixed properly, by either using c_p_a() as needed in the kprobes/alternatives code (Prasanna already had a patch) or perhaps better just doing a temporal ioremap() there.

-Andi

---

Disable kernel text protection when kprobes are enabled

To be done better in .23

Signed-off-by Andi Kleen <[email protected]>

arch/x86_64/mm/init.c +4 -0

Index: linux/arch/x86_64/mm/init.c
===================================================================
@@ -600,6 +600,10 @@ void mark_rodata_ro(void)
 {
 	unsigned long start = (unsigned long)_stext, end;
 
+#ifdef CONFIG_KPROBES
+	/* Kprobes code doesn't know yet how to unprotect. Temporary fix. */
+	start = (unsigned long)_etext;
+#endif
 #ifdef CONFIG_HOTPLUG_CPU
 	/* It must still be possible to apply SMP alternatives. */
 	if (num_possible_cpus() > 1)

arch/i386/mm/init.c +6 -2

Index: linux/arch/i386/mm/init.c
===================================================================
@@ -798,12 +798,16 @@ void mark_rodata_ro(void)
 {
 	unsigned long start = PFN_ALIGN(_text);
 	unsigned long size = PFN_ALIGN(_etext) - start;
+	int notext = 0;
 
+#ifdef CONFIG_KPROBES
+	notext = 1;
+#endif
 #ifdef CONFIG_HOTPLUG_CPU
 	/* It must still be possible to apply SMP alternatives. */
-	if (num_possible_cpus() <= 1)
+	notext = (num_possible_cpus() > 1);
 #endif
-	{
+	if (!notext) {
 		change_page_attr(virt_to_page(start),
 		                 size >> PAGE_SHIFT, PAGE_KERNEL_RX);
 		printk("Write protecting the kernel text: %luk\n", size >> 10);

Stefan Richter

2007-06-21 14:35 UTC #23 lore raw

In reply to Arjan van de Ven

Quoted 13 lines · Arjan van de Ven

arch/i386/Kconfig.debug renamed from arch/i386/Kconfig.debug.org +0 -1

@@ -49,7 +49,6 @@ config DEBUG_PAGEALLOC
 config DEBUG_RODATA
 	bool "Write protect kernel read-only data structures"
 	depends on DEBUG_KERNEL
-	depends on !KPROBES # temporary for 2.6.22
 	help
 	  Mark the kernel read-only data as write-protected in the pagetables,
 	  in order to catch accidental (and incorrect) writes to such const

arch/x86_64/Kconfig.debug source "lib/Kconfig.debug" · line 14

Quoted 7 lines

arch/x86_64/Kconfig.debug renamed from arch/x86_64/Kconfig.debug.org +0 -1

@@ -9,7 +9,6 @@ source "lib/Kconfig.debug"
 config DEBUG_RODATA
        bool "Write protect kernel read-only data structures"
        depends on DEBUG_KERNEL
-       depends on !KPROBES # temporary for 2.6.22
        help
 	 Mark the kernel read-only data as write-protected in the pagetables,
 	 in order to catch accidental (and incorrect) writes to such const data.

Shouldn't we add something to the help texts?

+ This option also marks kernel text pages as write-protected,
+ except if you enable KPROBES.

CMIIW.

As mentioned elsewhere in this thread, replacing CONFIG_DEBUG_RODATA by CONFIG_WRITEPROTECT_RODATA and CONFIG_WRITEPROTECT_TEXT might be a better idea. The latter should be mutual exclusive with CONFIG_KPROBES.

Signature · 3 lines

Stefan Richter
-=====-=-=== -==- =-=-=
http://arcgraph.de/sr/

Arjan van de Ven

2007-06-21 14:41 UTC #24 lore raw

Quoted 11 lines

Shouldn't we add something to the help texts?
(quoted diff) +2 -0
+	This option also marks kernel text pages as write-protected,
+	except if you enable KPROBES.
CMIIW.
As mentioned elsewhere in this thread, replacing CONFIG_DEBUG_RODATA by CONFIG_WRITEPROTECT_RODATA and CONFIG_WRITEPROTECT_TEXT might be a better idea. The latter should be mutual exclusive with CONFIG_KPROBES.

lets do the minimal now for 2.6.22, and do it right for .23 please

Signature · 2 lines

if you want to mail me at work (you don't), use arjan (at) linux.intel.com
Test the interaction between Linux and your BIOS via http://www.linuxfirmwarekit.org

Chuck Ebbert

2007-06-21 15:47 UTC #12 lore raw

In reply to Linus Torvalds

Quoted 7 lines · Linus Torvalds

As far as I know, the biggest reason to use DEBUG_RODATA is
(a) Hey, it's a cheap way to check one thing
(b) An added layer of security (which it's not that great for, but it might make sense to make it more of a real security feature).

And as a security feature it's pretty useless if kprobes are allowed.

Linus Torvalds

2007-06-21 16:05 UTC #18 lore raw

Re: [PATCH] Alternative fix for kprobes&DEBUG_RODATA was Re: [1/2] 2.6.22-rc5: known regressions with patches

In reply to Andi Kleen

Quoted 2 lines · Andi Kleen

Ok, here's a patch to do this. With that 55181000cd60334fe920c65ffbcdfe0e3f1de406 should be reverted because it isn't needed anymore.

This seems buggy:

arch/i386/mm/init.c void mark_rodata_ro(void) · line 809

Quoted 10 lines

arch/i386/mm/init.c +5 -1

+	int notext = 0;
 
+#ifdef CONFIG_KPROBES
+	notext = 1;
+#endif
 #ifdef CONFIG_HOTPLUG_CPU
 	/* It must still be possible to apply SMP alternatives. */
-	if (num_possible_cpus() <= 1)
+	notext = (num_possible_cpus() > 1);
 #endif

The CONFIG_HOTPLUG_CPU case will overwrite the CONFIG_KPROBES, and turn it back to zero for a single CPU.

Linus