Pavel Machek | 3 Mar 13:17 2008
Picon

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

Hi!

> > > Klaus S. Madsen wrote:
> > >> open("/dev/mem", O_RDWR)                = 5
> > >> mmap2(NULL, 1282, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_SHARED|MAP_FIXED, 5, 0) = 0
> > >> mmap2(0xa0000, 393216, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 5, 0xa0) = 0xa0000
> > >                          ^^^^^^^^^^^^^^^^^^^^
> > >> close(5)                                = 0
> > >> ioperm(0, 0x400, 0x1)                   = 0
> > >> iopl(0x3)                               = 0
> > >> access("/sys/bus/pci", R_OK)            = 0
> > >> write(1, "Calling get_mode\n", 17)      = 17
> > >> vm86(0x1, 0xb7f14ccc, 0xb7f14830, 0xc000, 0x18b6 <unfinished ...>
> > >> --- SIGSEGV (Segmentation fault)  <at>  0 (0) ---
> > >> +++ killed by SIGSEGV (core dumped) +++
> > >
> > > This is the VGA BIOS being mapped, it's mapped PROT_READ|PROT_WRITE, but  
> > > no PROT_EXEC; if the kernel is NX-capable it *should* segfault trying to 
> > > execute out of this area, which is exactly what will happen when vm86 
> > > executes INT 10h.
> > >
> > > If we can find that mmap() in the s2ram source code and add PROT_EXEC 
> > > to it, it would be interesting.
> > 
> > Klaus, could you send your .config as well? Lets make sure that NX is 
> > even relevant in this context.
> Allright. The mmap in question is in the x86-common.c file in libx86,
> and adding PROT_EXEC to it solves the problem.

Ok, sw should probably fix that in s2ram... can you mail a patch to
(Continue reading)

Klaus S. Madsen | 3 Mar 16:11 2008

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

On Mon, Mar 03, 2008 at 13:17:35 +0100, Pavel Machek wrote:
> Hi!
> 
> > > > Klaus S. Madsen wrote:
> > > >> open("/dev/mem", O_RDWR)                = 5
> > > >> mmap2(NULL, 1282, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_SHARED|MAP_FIXED, 5, 0) = 0
> > > >> mmap2(0xa0000, 393216, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 5, 0xa0) = 0xa0000
> > > >                          ^^^^^^^^^^^^^^^^^^^^
> > > >> close(5)                                = 0
> > > >> ioperm(0, 0x400, 0x1)                   = 0
> > > >> iopl(0x3)                               = 0
> > > >> access("/sys/bus/pci", R_OK)            = 0
> > > >> write(1, "Calling get_mode\n", 17)      = 17
> > > >> vm86(0x1, 0xb7f14ccc, 0xb7f14830, 0xc000, 0x18b6 <unfinished ...>
> > > >> --- SIGSEGV (Segmentation fault)  <at>  0 (0) ---
> > > >> +++ killed by SIGSEGV (core dumped) +++
> > > >
> > > > This is the VGA BIOS being mapped, it's mapped
> > > > PROT_READ|PROT_WRITE, but  no PROT_EXEC; if the kernel is
> > > > NX-capable it *should* segfault trying to execute out of this
> > > > area, which is exactly what will happen when vm86 executes INT
> > > > 10h.
> > > >
> > > > If we can find that mmap() in the s2ram source code and add
> > > > PROT_EXEC to it, it would be interesting.
> > > 
> > > Klaus, could you send your .config as well? Lets make sure that NX is 
> > > even relevant in this context.
> > Allright. The mmap in question is in the x86-common.c file in libx86,
> > and adding PROT_EXEC to it solves the problem.
(Continue reading)

Rafael J. Wysocki | 3 Mar 16:40 2008
Picon

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

On Monday, 3 of March 2008, Pavel Machek wrote:
> Hi!
> 
> > > > Klaus S. Madsen wrote:
> > > >> open("/dev/mem", O_RDWR)                = 5
> > > >> mmap2(NULL, 1282, PROT_READ|PROT_WRITE|PROT_EXEC, MAP_SHARED|MAP_FIXED, 5, 0) = 0
> > > >> mmap2(0xa0000, 393216, PROT_READ|PROT_WRITE, MAP_SHARED|MAP_FIXED, 5, 0xa0) = 0xa0000
> > > >                          ^^^^^^^^^^^^^^^^^^^^
> > > >> close(5)                                = 0
> > > >> ioperm(0, 0x400, 0x1)                   = 0
> > > >> iopl(0x3)                               = 0
> > > >> access("/sys/bus/pci", R_OK)            = 0
> > > >> write(1, "Calling get_mode\n", 17)      = 17
> > > >> vm86(0x1, 0xb7f14ccc, 0xb7f14830, 0xc000, 0x18b6 <unfinished ...>
> > > >> --- SIGSEGV (Segmentation fault)  <at>  0 (0) ---
> > > >> +++ killed by SIGSEGV (core dumped) +++
> > > >
> > > > This is the VGA BIOS being mapped, it's mapped PROT_READ|PROT_WRITE, but  
> > > > no PROT_EXEC; if the kernel is NX-capable it *should* segfault trying to 
> > > > execute out of this area, which is exactly what will happen when vm86 
> > > > executes INT 10h.
> > > >
> > > > If we can find that mmap() in the s2ram source code and add PROT_EXEC 
> > > > to it, it would be interesting.
> > > 
> > > Klaus, could you send your .config as well? Lets make sure that NX is 
> > > even relevant in this context.
> > Allright. The mmap in question is in the x86-common.c file in libx86,
> > and adding PROT_EXEC to it solves the problem.
> 
(Continue reading)

H. Peter Anvin | 3 Mar 18:10 2008

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

Pavel Machek wrote:
>>
>> The only thing I don't understand is why this is suddenly a problem with
>> 2.6.25, and not with 2.6.24? Is there a bug in 2.6.24 and previously
>> that allows real-mode execution of non-executable pages?
> 
> It is strange indeed... Should it be traced as an regression?
> 									Pavel

I'd like to understand what the heck happened, but as far as we can 
observe right now, it's a *progression*, not a regression, since 
executing out of a non-PROT_EXEC area isn't *supposed* to work...

	-hpa

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
Pavel Machek | 3 Mar 18:47 2008
Picon

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

On Mon 2008-03-03 09:10:35, H. Peter Anvin wrote:
> Pavel Machek wrote:
>>>
>>> The only thing I don't understand is why this is suddenly a problem with
>>> 2.6.25, and not with 2.6.24? Is there a bug in 2.6.24 and previously
>>> that allows real-mode execution of non-executable pages?
>>
>> It is strange indeed... Should it be traced as an regression?
>
> I'd like to understand what the heck happened, but as far as we can observe 
> right now, it's a *progression*, not a regression, since executing out of a 
> non-PROT_EXEC area isn't *supposed* to work...

Okay, I guess this depends on the eye of the beholder... because s2ram
*is* supposed to work ;-).

Ideally, I'd like to keep 2.6.24 behaviour for at least a while, so we
can try to fix the libx86 out there or something...
									Pavel
PS: Matthew, there's problem in libx86: it tries to execute from area
not marked as PROT_EXEC.
--

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/
(Continue reading)

Ingo Molnar | 3 Mar 18:48 2008
Picon
Picon

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending


* Klaus S. Madsen <ksm <at> hjernemadsen.org> wrote:

> The following patch solves the segfault, by changing the mmap flags of 
> the video memory area, to allow execution. The patch is against 
> libx86-0.99 available from http://www.codon.org.uk/~mjg59/libx86/
> 
> --- libx86-0.99/x86-common.c	2006-09-08 00:44:27.000000000 +0200
> +++ libx86-0.99.new/x86-common.c	2008-03-01 10:08:25.000000000 +0100
>  <at>  <at>  -232,7 +232,7  <at>  <at> 
>  	}
>  
>  	m = mmap((void *)0xa0000, 0x100000 - 0xa0000,
> -	 PROT_READ | PROT_WRITE,
> +	 PROT_READ | PROT_WRITE | PROT_EXEC,

are you sure you ID-ed the right commit that broke things?

while requiring PROT_EXEC is fine, breaking existing user-space apps 
over that is not fine. So are you absolutely sure that by reverting that 
PWT|PCD commit, s2ram again starts to work? That's utmost weird...

perhaps there's some CPU bug that causes NX to _NOT_ work if only PCD is 
used (not PCD|PWT). Seems like a pretty unlikely scenario though.

	Ingo

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
(Continue reading)

H. Peter Anvin | 3 Mar 18:50 2008

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

Pavel Machek wrote:
> On Mon 2008-03-03 09:10:35, H. Peter Anvin wrote:
>> Pavel Machek wrote:
>>>> The only thing I don't understand is why this is suddenly a problem with
>>>> 2.6.25, and not with 2.6.24? Is there a bug in 2.6.24 and previously
>>>> that allows real-mode execution of non-executable pages?
>>> It is strange indeed... Should it be traced as an regression?
>> I'd like to understand what the heck happened, but as far as we can observe 
>> right now, it's a *progression*, not a regression, since executing out of a 
>> non-PROT_EXEC area isn't *supposed* to work...
> 
> Okay, I guess this depends on the eye of the beholder... because s2ram
> *is* supposed to work ;-).
> 
> Ideally, I'd like to keep 2.6.24 behaviour for at least a while, so we
> can try to fix the libx86 out there or something...
> 									Pavel
> PS: Matthew, there's problem in libx86: it tries to execute from area
> not marked as PROT_EXEC.

Allowing execution of a PROT_EXEC area is a security hole.  The fact 
that you happened to benefit from it doesn't change its nature as a 
security hole.

	-hpa
H. Peter Anvin | 3 Mar 18:53 2008

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

Ingo Molnar wrote:
> * Klaus S. Madsen <ksm <at> hjernemadsen.org> wrote:
> 
>> The following patch solves the segfault, by changing the mmap flags of 
>> the video memory area, to allow execution. The patch is against 
>> libx86-0.99 available from http://www.codon.org.uk/~mjg59/libx86/
>>
>> --- libx86-0.99/x86-common.c	2006-09-08 00:44:27.000000000 +0200
>> +++ libx86-0.99.new/x86-common.c	2008-03-01 10:08:25.000000000 +0100
>>  <at>  <at>  -232,7 +232,7  <at>  <at> 
>>  	}
>>  
>>  	m = mmap((void *)0xa0000, 0x100000 - 0xa0000,
>> -	 PROT_READ | PROT_WRITE,
>> +	 PROT_READ | PROT_WRITE | PROT_EXEC,
> 
> are you sure you ID-ed the right commit that broke things?
> 
> while requiring PROT_EXEC is fine, breaking existing user-space apps 
> over that is not fine. So are you absolutely sure that by reverting that 
> PWT|PCD commit, s2ram again starts to work? That's utmost weird...
> 
> perhaps there's some CPU bug that causes NX to _NOT_ work if only PCD is 
> used (not PCD|PWT). Seems like a pretty unlikely scenario though.
> 

It really does.  What would be much more likely is that the PCD -> 
(PCD|PWT) triggered something in the kernel proper.

	-hpa
(Continue reading)

Ingo Molnar | 3 Mar 18:53 2008
Picon
Picon

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending


* H. Peter Anvin <hpa <at> zytor.com> wrote:

>> It is strange indeed... Should it be traced as an regression?
>
> I'd like to understand what the heck happened, but as far as we can 
> observe right now, it's a *progression*, not a regression, since 
> executing out of a non-PROT_EXEC area isn't *supposed* to work...

if s2ram worked before, and breaks with v2.6.25 then it's a regression. 
It doesnt really matter that this segfault is the right thing to do ...

what would be _really_ nice to know how this all links up to the PCD -> 
PCD|PWT commit. I.e. how did the commit below suddenly _trigger_ this 
segfault? Are we absolutely sure that the bisection is correct, that 
it's this commit which broke things? [i.e. if this commit is unapplied, 
does s2ram work?]

there are a few "nearby" commits in the bisection tree:

 commit 6c3866558213ff706d8331053386915371ad63ec
 Author: Jeremy Fitzhardinge <jeremy <at> goop.org>
 Date:   Wed Jan 30 13:32:55 2008 +0100

     x86: move all asm/pgtable constants into one place

 commit 61f38226def55d972cfd0e789971e952525ff8e5
 Author: Ingo Molnar <mingo <at> elte.hu>
 Date:   Wed Jan 30 13:32:55 2008 +0100

(Continue reading)

H. Peter Anvin | 3 Mar 18:58 2008

Re: Regression in 2.6.25-rc3: s2ram segfaults before suspending

Ingo Molnar wrote:
> 
> if s2ram worked before, and breaks with v2.6.25 then it's a regression. 
> It doesnt really matter that this segfault is the right thing to do ...
> 

I'm sorry, I have to disagree with this one, simply because it's a 
security model violation.  We can't just say "well, app X does this even 
though it's forbidden, well, let's just put in a security hole for it."

	-hpa

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

Gmane