2007-06-17 14:24:46

by Michal Piotrowski

[permalink] [raw]
Subject: [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/


2007-06-17 15:16:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches


* Michal Piotrowski <[email protected]> wrote:

> 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

2007-06-17 15:31:27

by Michal Piotrowski

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On 17/06/07, Ingo Molnar <[email protected]> wrote:
>
> * Michal Piotrowski <[email protected]> wrote:
>
> > 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?

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

>
> Ingo
>

Regards,
Michal

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

2007-06-20 22:08:33

by Ian McDonald

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

Andrew,

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

>
> 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
--
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

2007-06-20 22:32:35

by Chuck Ebbert

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On 06/20/2007 06:08 PM, Ian McDonald wrote:
> Andrew,
>
> Can this one be merged so that it makes it into 2.6.22?
>
>>
>> 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

2007-06-20 22:38:44

by Ian McDonald

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On 6/21/07, Chuck Ebbert <[email protected]> wrote:
> >> 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
>

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
--
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

2007-06-20 22:39:53

by Linus Torvalds

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches



On Thu, 21 Jun 2007, Ian McDonald wrote:
>
> Can this one be merged so that it makes it into 2.6.22?
>
> >
> > 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,

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

2007-06-20 22:41:51

by Ian McDonald

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On 6/21/07, Linus Torvalds <[email protected]> wrote:
> 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

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

2007-06-20 22:43:47

by Linus Torvalds

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches



On Wed, 20 Jun 2007, Chuck Ebbert wrote:
>
> 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

2007-06-20 22:56:13

by Linus Torvalds

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches



On Thu, 21 Jun 2007, Ian McDonald wrote:
>
> 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

2007-06-20 23:02:54

by Ian McDonald

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On 6/21/07, Linus Torvalds <[email protected]> wrote:
> 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.

> 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.

> 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
--
Web: http://wand.net.nz/~iam4/
Blog: http://iansblog.jandi.co.nz
WAND Network Research Group

2007-06-20 23:06:35

by Linus Torvalds

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches



On Wed, 20 Jun 2007, Linus Torvalds wrote:
>
> 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

2007-06-20 23:09:42

by Dave Jones

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On Wed, Jun 20, 2007 at 03:38:06PM -0700, Linus Torvalds wrote:

> 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

--
http://www.codemonkey.org.uk

2007-06-20 23:19:58

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On Wed, 2007-06-20 at 19:07 -0400, Dave Jones wrote:
> On Wed, Jun 20, 2007 at 03:38:06PM -0700, Linus Torvalds wrote:
>
> > 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 ?

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.


2007-06-20 23:24:52

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches


> 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:

--- linux-2.6.22-rc5/arch/x86_64/mm/init.c.org 2007-06-21 04:20:10.000000000 -0700
+++ linux-2.6.22-rc5/arch/x86_64/mm/init.c 2007-06-21 04:21:01.000000000 -0700
@@ -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;


2007-06-20 23:38:18

by Dave Jones

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On Wed, Jun 20, 2007 at 04:15:53PM -0700, Arjan van de Ven wrote:
> On Wed, 2007-06-20 at 19:07 -0400, Dave Jones wrote:
> > On Wed, Jun 20, 2007 at 03:38:06PM -0700, Linus Torvalds wrote:
> >
> > > 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 ?
>
> 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

--
http://www.codemonkey.org.uk

2007-06-20 23:51:44

by Linus Torvalds

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches



On Wed, 20 Jun 2007, Dave Jones wrote:
>
> 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

2007-06-20 23:51:58

by Linus Torvalds

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches



On Wed, 20 Jun 2007, Arjan van de Ven wrote:
>
> 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

2007-06-21 05:27:20

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On Wed, 2007-06-20 at 16:50 -0700, Linus Torvalds wrote:
>
> On Wed, 20 Jun 2007, Arjan van de Ven wrote:
> >
> > 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]>

--- linux-2.6.22-rc5/arch/i386/Kconfig.debug.org 2007-06-20 22:20:30.000000000 -0700
+++ linux-2.6.22-rc5/arch/i386/Kconfig.debug 2007-06-20 22:20:55.000000000 -0700
@@ -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
--- linux-2.6.22-rc5/arch/x86_64/Kconfig.debug.org 2007-06-20 22:20:28.000000000 -0700
+++ linux-2.6.22-rc5/arch/x86_64/Kconfig.debug 2007-06-20 22:20:44.000000000 -0700
@@ -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.
--- linux-2.6.22-rc5/arch/i386/mm/init.c.org 2007-06-20 22:18:40.000000000 -0700
+++ linux-2.6.22-rc5/arch/i386/mm/init.c 2007-06-20 22:19:45.000000000 -0700
@@ -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),
--- linux-2.6.22-rc5/arch/x86_64/mm/init.c.org 2007-06-20 21:44:15.000000000 -0700
+++ linux-2.6.22-rc5/arch/x86_64/mm/init.c 2007-06-20 22:17:45.000000000 -0700
@@ -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;

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

2007-06-21 07:11:19

by S. P. Prasanna

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On Wed, Jun 20, 2007 at 10:23:21PM -0700, Arjan van de Ven wrote:
> On Wed, 2007-06-20 at 16:50 -0700, Linus Torvalds wrote:
> >
> > On Wed, 20 Jun 2007, Arjan van de Ven wrote:
> > >
> > > 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.

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

Thanks
Prasanna
>
>
>
> 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]>
>
> --- linux-2.6.22-rc5/arch/i386/Kconfig.debug.org 2007-06-20 22:20:30.000000000 -0700
> +++ linux-2.6.22-rc5/arch/i386/Kconfig.debug 2007-06-20 22:20:55.000000000 -0700
> @@ -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
> --- linux-2.6.22-rc5/arch/x86_64/Kconfig.debug.org 2007-06-20 22:20:28.000000000 -0700
> +++ linux-2.6.22-rc5/arch/x86_64/Kconfig.debug 2007-06-20 22:20:44.000000000 -0700
> @@ -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.
> --- linux-2.6.22-rc5/arch/i386/mm/init.c.org 2007-06-20 22:18:40.000000000 -0700
> +++ linux-2.6.22-rc5/arch/i386/mm/init.c 2007-06-20 22:19:45.000000000 -0700
> @@ -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),
> --- linux-2.6.22-rc5/arch/x86_64/mm/init.c.org 2007-06-20 21:44:15.000000000 -0700
> +++ linux-2.6.22-rc5/arch/x86_64/mm/init.c 2007-06-20 22:17:45.000000000 -0700
> @@ -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;
>
> --
> 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

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

2007-06-21 09:20:13

by Andi Kleen

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

On Thursday 21 June 2007 01:48:43 Linus Torvalds wrote:
>
> On Wed, 20 Jun 2007, Dave Jones wrote:
> >
> > 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.

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]>

Index: linux/arch/x86_64/mm/init.c
===================================================================
--- linux.orig/arch/x86_64/mm/init.c
+++ 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)
Index: linux/arch/i386/mm/init.c
===================================================================
--- linux.orig/arch/i386/mm/init.c
+++ 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);

2007-06-21 14:35:27

by Stefan Richter

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

Arjan van de Ven wrote:
> --- linux-2.6.22-rc5/arch/i386/Kconfig.debug.org 2007-06-20 22:20:30.000000000 -0700
> +++ linux-2.6.22-rc5/arch/i386/Kconfig.debug 2007-06-20 22:20:55.000000000 -0700
> @@ -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
> --- linux-2.6.22-rc5/arch/x86_64/Kconfig.debug.org 2007-06-20 22:20:28.000000000 -0700
> +++ linux-2.6.22-rc5/arch/x86_64/Kconfig.debug 2007-06-20 22:20:44.000000000 -0700
> @@ -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.
--
Stefan Richter
-=====-=-=== -==- =-=-=
http://arcgraph.de/sr/

2007-06-21 14:41:21

by Arjan van de Ven

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches


> 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.


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

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

2007-06-21 15:47:35

by Chuck Ebbert

[permalink] [raw]
Subject: Re: [1/2] 2.6.22-rc5: known regressions with patches

On 06/20/2007 07:04 PM, Linus Torvalds wrote:
> 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.

2007-06-21 16:05:23

by Linus Torvalds

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



On Thu, 21 Jun 2007, Andi Kleen wrote:

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

This seems buggy:

> + 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