public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* RE: Non-Exec stack patches
@ 2004-04-14  7:28 Siddha, Suresh B
  2004-04-14  8:23 ` Jamie Lokier
                   ` (2 more replies)
  0 siblings, 3 replies; 20+ messages in thread
From: Siddha, Suresh B @ 2004-04-14  7:28 UTC (permalink / raw)
  To: Andrew Morton, Kurt Garloff; +Cc: linux-kernel, mingo

[-- Attachment #1: Type: text/plain, Size: 1077 bytes --]

> -----Original Message-----
> From: linux-kernel-owner@vger.kernel.org
> [mailto:linux-kernel-owner@vger.kernel.org]On Behalf Of Andrew Morton
> Sent: Tuesday, March 23, 2004 4:41 PM
> To: Kurt Garloff
> Cc: linux-kernel@vger.kernel.org; mingo@redhat.com
> Subject: Re: Non-Exec stack patches
> 
> 
> Kurt Garloff <garloff@suse.de> wrote:
> >
> > > Which architectures are currently making their pre-page 
> execute permissions
> > > depend upon VM_EXEC?  Would additional arch patches be 
> needed for this?
> > 
> > It works on AMD64 (not ia32e), both for 64bit and 32bit binaries.
> > I have not yet tested other archs.
> > 
> > If the values in the protection_map are different depending 
> on bit 2,
> > the patch will be effecitve. (OK, the CPU/MMU needs to honour the
> > setting of course.) Most likely, the values for 
> > protection_map[7] is PAGE_COPY_EXEC and of protection_map[3] is
> > PAGE_COPY.
> 
> OK.
> 

Recent ia64 mm trees are broken because of this issue. Attached patch fixes protection_map[7] in IA64.

thanks,
suresh

[-- Attachment #2: noexec-ia64.fix --]
[-- Type: application/octet-stream, Size: 458 bytes --]

--- linux-265mm5/include/asm-ia64/pgtable.h~	2004-04-14 00:09:04.000000000 -0700
+++ linux-265mm5/include/asm-ia64/pgtable.h	2004-04-13 23:45:29.000000000 -0700
@@ -148,7 +148,7 @@
 #define __P100	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
 #define __P101	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
 #define __P110	PAGE_COPY
-#define __P111	PAGE_COPY
+#define __P111	PAGE_COPY_EXEC
 
 #define __S000	PAGE_NONE
 #define __S001	PAGE_READONLY

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Non-Exec stack patches
  2004-04-14  7:28 Non-Exec stack patches Siddha, Suresh B
@ 2004-04-14  8:23 ` Jamie Lokier
  2004-04-14  8:35   ` PowerPC exec page protection Jamie Lokier
  2004-04-14 11:37   ` [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h> Jamie Lokier
  2004-04-14  9:47 ` Non-Exec stack patches Jamie Lokier
  2004-04-14 18:35 ` Kurt Garloff
  2 siblings, 2 replies; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14  8:23 UTC (permalink / raw)
  To: Siddha, Suresh B; +Cc: Andrew Morton, Kurt Garloff, linux-kernel, mingo

Siddha, Suresh B wrote:
> Recent ia64 mm trees are broken because of this issue. Attached
> patch fixes protection_map[7] in IA64.

> --- linux-265mm5/include/asm-ia64/pgtable.h~    2004-04-14 00:09:04.000000000 -0700
> +++ linux-265mm5/include/asm-ia64/pgtable.h     2004-04-13 23:45:29.000000000 -0700
> @@ -148,7 +148,7 @@
>  #define __P100 __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
>  #define __P101 __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
>  #define __P110 PAGE_COPY
> -#define __P111 PAGE_COPY
> +#define __P111 PAGE_COPY_EXEC
>   
>  #define __S000 PAGE_NONE
>  #define __S001 PAGE_READONLY

I'm looking at 2.6.5, and it doesn't define PAGE_COPY_EXEC.  In 2.6.5,
asm-ia64/pgtable.h, PAGE_COPY is executable, and PAGE_READONLY is
non-executable.

Yes I know the naming is different from all the other asm-* files, but
that's what it says.  All of those definitions have confused names on
ia64, although they seem to have the rights bits set in the end.

Has the meaning of PAGE_COPY in asm-ia64/pgtable.h changed in 2.6.5-mm5?

If so, you want to change __P110 as well as __P111.

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* PowerPC exec page protection
  2004-04-14  8:23 ` Jamie Lokier
@ 2004-04-14  8:35   ` Jamie Lokier
  2004-04-14  8:44     ` Anton Blanchard
  2004-04-14 11:37   ` [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h> Jamie Lokier
  1 sibling, 1 reply; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14  8:35 UTC (permalink / raw)
  To: Paul Mackerras, Anton Blanchard
  Cc: Siddha, Suresh B, Andrew Morton, Kurt Garloff, linux-kernel,
	mingo

<asm-ppc/pgtable.h> and <asm-ppc64/pgtable.h> both define the
following map of protection bits:

    #define __P000  PAGE_NONE
    #define __P001  PAGE_READONLY_X
    #define __P010  PAGE_COPY
    #define __P011  PAGE_COPY_X
    #define __P100  PAGE_READONLY
    #define __P101  PAGE_READONLY_X
    #define __P110  PAGE_COPY
    #define __P111  PAGE_COPY_X

    #define __S000  PAGE_NONE
    #define __S001  PAGE_READONLY_X
    #define __S010  PAGE_SHARED
    #define __S011  PAGE_SHARED_X
    #define __S100  PAGE_READONLY
    #define __S101  PAGE_READONLY_X
    #define __S110  PAGE_SHARED
    #define __S111  PAGE_SHARED_X

The _X flags seem wrongly placed, as bit 2 is the PROT_EXEC bit, not
bit 0.  Is the above intentional?

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: PowerPC exec page protection
  2004-04-14  8:35   ` PowerPC exec page protection Jamie Lokier
@ 2004-04-14  8:44     ` Anton Blanchard
  2004-04-14  9:35       ` Jamie Lokier
  0 siblings, 1 reply; 20+ messages in thread
From: Anton Blanchard @ 2004-04-14  8:44 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: Paul Mackerras, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo


Hi,

> <asm-ppc/pgtable.h> and <asm-ppc64/pgtable.h> both define the
> following map of protection bits:
> 
>     #define __P000  PAGE_NONE
>     #define __P001  PAGE_READONLY_X
>     #define __P010  PAGE_COPY
>     #define __P011  PAGE_COPY_X
>     #define __P100  PAGE_READONLY
>     #define __P101  PAGE_READONLY_X
>     #define __P110  PAGE_COPY
>     #define __P111  PAGE_COPY_X
> 
>     #define __S000  PAGE_NONE
>     #define __S001  PAGE_READONLY_X
>     #define __S010  PAGE_SHARED
>     #define __S011  PAGE_SHARED_X
>     #define __S100  PAGE_READONLY
>     #define __S101  PAGE_READONLY_X
>     #define __S110  PAGE_SHARED
>     #define __S111  PAGE_SHARED_X
> 
> The _X flags seem wrongly placed, as bit 2 is the PROT_EXEC bit, not
> bit 0.  Is the above intentional?

Its backwards and we know it :) Ive got a patch to implement per page
execute on ppc64 and that did pop up.

Thanks for pointing it out, are you looking at ppc* page protection or
just chanced upon it?

Anton

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: PowerPC exec page protection
  2004-04-14  8:44     ` Anton Blanchard
@ 2004-04-14  9:35       ` Jamie Lokier
  0 siblings, 0 replies; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14  9:35 UTC (permalink / raw)
  To: Anton Blanchard
  Cc: Paul Mackerras, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

Anton Blanchard wrote:
> Thanks for pointing it out, are you looking at ppc* page protection or
> just chanced upon it?

Just chanced.

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Non-Exec stack patches
  2004-04-14  7:28 Non-Exec stack patches Siddha, Suresh B
  2004-04-14  8:23 ` Jamie Lokier
@ 2004-04-14  9:47 ` Jamie Lokier
  2004-04-14 18:30   ` Kurt Garloff
  2004-04-14 18:35 ` Kurt Garloff
  2 siblings, 1 reply; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14  9:47 UTC (permalink / raw)
  To: Siddha, Suresh B; +Cc: Andrew Morton, Kurt Garloff, linux-kernel, mingo

People looking at PROT_EXEC page table flags might want to be aware
that <asm-um/pgtable.h> mimics the behaviour of i386: read implies and
is implied by exec, write implies read.

That might mean user-mode linux doesn't provide no-exec-stack
protection even when the underlying kernel does offer it.  I'm not sure.

-- Jamie


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14  8:23 ` Jamie Lokier
  2004-04-14  8:35   ` PowerPC exec page protection Jamie Lokier
@ 2004-04-14 11:37   ` Jamie Lokier
  2004-04-14 16:07     ` David Mosberger
  1 sibling, 1 reply; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14 11:37 UTC (permalink / raw)
  To: David Mosberger-Tang, linux-ia64
  Cc: Siddha, Suresh B, Andrew Morton, Kurt Garloff, linux-kernel,
	mingo

Patch: ia64-pgtable-2.6.5.patch

That mixture of PAGE_* and __pgprot() definitions for the __[PS]*
macros in asm-ia64/pgtable.h is really ugly and just makes the code
unnecessarily confusing:

	#define __P000  PAGE_NONE
	#define __P001  PAGE_READONLY
	#define __P010  PAGE_READONLY
	#define __P011  PAGE_READONLY
	#define __P100  __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
	#define __P101  __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
	#define __P110  PAGE_COPY
	#define __P111  PAGE_COPY

The PAGE_* macros which are used in __[PS]* aren't used anywhere else:
their entire reason for existing is to make the __[PS]* macros
clearer.  It looks as though the people who implemented the IA-64 port
didn't realise that.

Here is a page (untested) which cleans up those definitions.  It was
made from 2.6.5.

Enjoy,
-- Jamie

diff -ur orig-2.6.5/include/asm-ia64/pgtable.h ia64-2.6.5/include/asm-ia64/pgtable.h
--- orig-2.6.5/include/asm-ia64/pgtable.h	2004-04-12 21:45:39.000000000 +0100
+++ ia64-2.6.5/include/asm-ia64/pgtable.h	2004-04-14 12:29:01.000000000 +0100
@@ -116,13 +116,17 @@
  * they are used, the page is accessed. They are cleared only by the
  * page-out routines.
  */
-#define PAGE_NONE	__pgprot(_PAGE_PROTNONE | _PAGE_A)
-#define PAGE_SHARED	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RW)
-#define PAGE_READONLY	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_R)
-#define PAGE_COPY	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
-#define PAGE_GATE	__pgprot(__ACCESS_BITS | _PAGE_PL_0 | _PAGE_AR_X_RX)
-#define PAGE_KERNEL	__pgprot(__DIRTY_BITS  | _PAGE_PL_0 | _PAGE_AR_RWX)
-#define PAGE_KERNELRX	__pgprot(__ACCESS_BITS | _PAGE_PL_0 | _PAGE_AR_RX)
+#define PAGE_NONE	   __pgprot(_PAGE_PROTNONE | _PAGE_A)
+#define PAGE_SHARED	   __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RW)
+#define PAGE_SHARED_EXEC   __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RWX)
+#define PAGE_READONLY	   __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_R)
+#define PAGE_READONLY_EXEC __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
+#define PAGE_COPY	   __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_R)
+#define PAGE_COPY_EXEC	   __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
+#define PAGE_EXEC	   __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
+#define PAGE_GATE	   __pgprot(__ACCESS_BITS | _PAGE_PL_0 | _PAGE_AR_X_RX)
+#define PAGE_KERNEL	   __pgprot(__DIRTY_BITS  | _PAGE_PL_0 | _PAGE_AR_RWX)
+#define PAGE_KERNELRX	   __pgprot(__ACCESS_BITS | _PAGE_PL_0 | _PAGE_AR_RX)
 
 # ifndef __ASSEMBLY__
 
@@ -142,21 +146,21 @@
 	/* xwr */
 #define __P000	PAGE_NONE
 #define __P001	PAGE_READONLY
-#define __P010	PAGE_READONLY	/* write to priv pg -> copy & make writable */
-#define __P011	PAGE_READONLY	/* ditto */
-#define __P100	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
-#define __P101	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
-#define __P110	PAGE_COPY
-#define __P111	PAGE_COPY
+#define __P010	PAGE_COPY
+#define __P011	PAGE_COPY
+#define __P100	PAGE_EXEC
+#define __P101	PAGE_READONLY_EXEC
+#define __P110	PAGE_COPY_EXEC
+#define __P111	PAGE_COPY_EXEC
 
 #define __S000	PAGE_NONE
 #define __S001	PAGE_READONLY
 #define __S010	PAGE_SHARED	/* we don't have (and don't need) write-only */
 #define __S011	PAGE_SHARED
-#define __S100	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
-#define __S101	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
-#define __S110	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RWX)
-#define __S111	__pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RWX)
+#define __S100	PAGE_EXEC
+#define __S101	PAGE_READONLY_EXEC
+#define __S110	PAGE_SHARED_EXEC
+#define __S111	PAGE_SHARED_EXEC
 
 #define pgd_ERROR(e)	printk("%s:%d: bad pgd %016lx.\n", __FILE__, __LINE__, pgd_val(e))
 #define pmd_ERROR(e)	printk("%s:%d: bad pmd %016lx.\n", __FILE__, __LINE__, pmd_val(e))

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 11:37   ` [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h> Jamie Lokier
@ 2004-04-14 16:07     ` David Mosberger
  2004-04-14 18:46       ` Jamie Lokier
  0 siblings, 1 reply; 20+ messages in thread
From: David Mosberger @ 2004-04-14 16:07 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: David Mosberger-Tang, linux-ia64, Siddha, Suresh B, Andrew Morton,
	Kurt Garloff, linux-kernel, mingo



 Jamie> Patch: ia64-pgtable-2.6.5.patch
 Jamie> That mixture of PAGE_* and __pgprot() definitions for the __[PS]*
 Jamie> macros in asm-ia64/pgtable.h is really ugly and just makes the code
 Jamie> unnecessarily confusing:

 Jamie> #define __P000  PAGE_NONE
 Jamie> #define __P001  PAGE_READONLY
 Jamie> #define __P010  PAGE_READONLY
 Jamie> #define __P011  PAGE_READONLY
 Jamie> #define __P100  __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_X_RX)
 Jamie> #define __P101  __pgprot(__ACCESS_BITS | _PAGE_PL_3 | _PAGE_AR_RX)
 Jamie> #define __P110  PAGE_COPY
 Jamie> #define __P111  PAGE_COPY

 Jamie> The PAGE_* macros which are used in __[PS]* aren't used
 Jamie> anywhere else: their entire reason for existing is to make the
 Jamie> __[PS]* macros clearer.  It looks as though the people who
 Jamie> implemented the IA-64 port didn't realise that.

Huh?  You haven't actually checked, have you?
I don't pollute namespace for no good reason.

 Jamie> Here is a page (untested) which cleans up those definitions.
 Jamie> It was made from 2.6.5.

If the same names are adopted by the other platforms, I'm fine with it.
Otherwise, see my comment above.

	--david

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Non-Exec stack patches
  2004-04-14  9:47 ` Non-Exec stack patches Jamie Lokier
@ 2004-04-14 18:30   ` Kurt Garloff
  2004-04-14 20:54     ` Jeff Dike
  0 siblings, 1 reply; 20+ messages in thread
From: Kurt Garloff @ 2004-04-14 18:30 UTC (permalink / raw)
  To: linux-kernel

[-- Attachment #1: Type: text/plain, Size: 691 bytes --]

On Wed, Apr 14, 2004 at 10:47:02AM +0100, Jamie Lokier wrote:
> People looking at PROT_EXEC page table flags might want to be aware
> that <asm-um/pgtable.h> mimics the behaviour of i386: read implies and
> is implied by exec, write implies read.
> 
> That might mean user-mode linux doesn't provide no-exec-stack
> protection even when the underlying kernel does offer it.  I'm not sure.

I thought UML only runs on i386.
And on i386, you have no NX feature.
You can run i386 UML on AMD64 (with 64bit kernel) though.

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: RE: Non-Exec stack patches
  2004-04-14  7:28 Non-Exec stack patches Siddha, Suresh B
  2004-04-14  8:23 ` Jamie Lokier
  2004-04-14  9:47 ` Non-Exec stack patches Jamie Lokier
@ 2004-04-14 18:35 ` Kurt Garloff
  2 siblings, 0 replies; 20+ messages in thread
From: Kurt Garloff @ 2004-04-14 18:35 UTC (permalink / raw)
  To: Siddha, Suresh B; +Cc: Andrew Morton, linux-kernel, mingo

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Wed, Apr 14, 2004 at 12:28:24AM -0700, Suresh B Siddha wrote:
> Recent ia64 mm trees are broken because of this issue. Attached patch 
> fixes protection_map[7] in IA64.

Ah, sorry. ia64 seems to be strangely different here.
My understanding is that it support NX page protection. And that you
don't have executable stacks in the ia64 ABI at all. But for i386
emulation you should be able to enable and disable executable stacks.
For some reason the needed defines are missing ...

Regards,
-- 
Kurt Garloff  <garloff@suse.de>                            Cologne, DE 
SUSE LINUX AG, Nuernberg, DE                          SUSE Labs (Head)

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 16:07     ` David Mosberger
@ 2004-04-14 18:46       ` Jamie Lokier
  2004-04-14 19:02         ` David Mosberger
  0 siblings, 1 reply; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14 18:46 UTC (permalink / raw)
  To: davidm
  Cc: linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

David Mosberger wrote:
> Huh?  You haven't actually checked, have you?

Yes I have.  Quite thoroughly.

> I don't pollute namespace for no good reason.

In this instance, there is a certain amount of arch variation.  parisc
defines PAGE_EXECREAD and PAGE_RWX.  ppc defines PAGE_COPY_X,
PAGE_SHARED_X, PAGE_READONLY_X (X is for exec).  m68k defines
PAGE_COPY_C, PAGE_READONLY_C, PAGE_SHARED_C.  x86_64 defines
PAGE_COPY_EXEC, PAGE_SHARED_EXEC and PAGE_READONLY_EXEC.

Those are all arch-private names, some of them used in code in arch/*,
some just to define the __[PS]* constants.

>  Jamie> Here is a page (untested) which cleans up those definitions.
>  Jamie> It was made from 2.6.5.
> 
> If the same names are adopted by the other platforms, I'm fine with it.
> Otherwise, see my comment above.

I copied <asm-x86_64/pgtable.h>, which closely matches ia64, except
for PAGE_EXEC which I named because nothing else has it.  That name
isn't used anywhere.  On reflection, a better name for it is
PAGE_EXECONLY (like PAGE_READONLY).

In theory the Alpha can do exec-only pages, but it's __[PS]* map
always gives read permission when there's execute permission.  I'm not
sure if there's a reason for that, or if it just historically copied
the i386 behaviour (Alpha was the first port).

The constants PAGE_KERNEL and PAGE_READONLY are used in
arch-independent code with a common meaning.  Otherwise, the constants
are used only to defined __[PS]* and a few in arch-dependent
initialisation code.

I agree it is best to avoid namespace pollution.  However this is one
area where ia64 sticks out because it's approach is different from
other ports.  All of them except Alpha used PAGE_* names to clarify
the __[PS]* map, defining additional names as needed.

The Alpha is quite clean in a different way, or looks like it until
you realise the _PAGE_P() macro is equivalent to identity so just
obfuscates the code.  (The _PAGE_P() macro which is absurd because
it's a complicated expression that's equivalent to identity).

The thing I don't like about the ia64 file is the mixing of two
different styles of definition in the same table.  When I had to read
all the arch pgtable.h files to discover what is Linux's mmap()
behaviour on each one, ia64 stood out awkwardly.

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 18:46       ` Jamie Lokier
@ 2004-04-14 19:02         ` David Mosberger
  2004-04-14 19:14           ` Jamie Lokier
  2004-04-14 19:28           ` Jamie Lokier
  0 siblings, 2 replies; 20+ messages in thread
From: David Mosberger @ 2004-04-14 19:02 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: davidm, linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

>>>>> On Wed, 14 Apr 2004 19:46:03 +0100, Jamie Lokier <jamie@shareable.org> said:

  Jamie> David Mosberger wrote:
  >> Huh?  You haven't actually checked, have you?

  Jamie> Yes I have.  Quite thoroughly.

Then you should have noticed that drivers/char/mem.c is using PAGE_COPY.
Various architecture-dependent code is also using PAGE_foo macros.

  Jamie> In theory the Alpha can do exec-only pages, but it's __[PS]*
  Jamie> map always gives read permission when there's execute
  Jamie> permission.  I'm not sure if there's a reason for that, or if
  Jamie> it just historically copied the i386 behaviour (Alpha was the
  Jamie> first port).

I know why: back in those days, GCC emitted code for nested C
functions that assumed an executable stack.  Also, Linus wasn't
terribly eager to turn off execute-permission on data/stacks.  Even on
ia64 we started out that way, until I saw the error in my ways.

  Jamie> I agree it is best to avoid namespace pollution.  However
  Jamie> this is one area where ia64 sticks out because it's approach
  Jamie> is different from other ports.  All of them except Alpha used
  Jamie> PAGE_* names to clarify the __[PS]* map, defining additional
  Jamie> names as needed.

The reality is that whenever you introduce a globally visible name
that is not used on x86, there is a very definite risk that someone
will use that same name and cause a conflict.  We have had that happen
several times and that's the reason I'm normally religious about
prefixing all ia64-specific names with a "ia64_" or "IA64_" (yes, this
makes code a bit uglier, but you can't have it both ways).  Your
argument that the Alpha and other ports are doing something different
doesn't buy me anything.  If the ia64 break builds, the Alpha
maintainer won't fix it up for me, after all.

	--david

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 19:02         ` David Mosberger
@ 2004-04-14 19:14           ` Jamie Lokier
  2004-04-14 19:28           ` Jamie Lokier
  1 sibling, 0 replies; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14 19:14 UTC (permalink / raw)
  To: davidm
  Cc: linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

David Mosberger wrote:
> If the ia64 break builds, the Alpha maintainer won't fix it up for
> me, after all.

Ok.  In this case PAGE_COPY_EXEC, PAGE_SHARED_EXEC and
PAGE_READONLY_EXEC are in x86_64, so those names are fairly safe.

You could write the other one as IA64_PAGE_EXECONLY to be very safe.

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 19:02         ` David Mosberger
  2004-04-14 19:14           ` Jamie Lokier
@ 2004-04-14 19:28           ` Jamie Lokier
  2004-04-14 20:05             ` David Mosberger
  1 sibling, 1 reply; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14 19:28 UTC (permalink / raw)
  To: davidm
  Cc: linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

David Mosberger wrote:
>   Jamie> Yes I have.  Quite thoroughly.
> 
> Then you should have noticed that drivers/char/mem.c is using PAGE_COPY.

That's an interesting bug in drivers/char/mem.c.  PAGE_COPY is wrong,
for archs with separate execute permission or write-only mappings.

The correct argument in drivers/char/mem.c should be vma->vm_page_prot.

>   Jamie> In theory the Alpha can do exec-only pages, but it's __[PS]*
>   Jamie> map always gives read permission when there's execute
>   Jamie> permission.  I'm not sure if there's a reason for that, or if
>   Jamie> it just historically copied the i386 behaviour (Alpha was the
>   Jamie> first port).
> 
> I know why: back in those days, GCC emitted code for nested C
> functions that assumed an executable stack.  Also, Linus wasn't
> terribly eager to turn off execute-permission on data/stacks.  Even on
> ia64 we started out that way, until I saw the error in my ways.

We're both wrong.  I misread the Alpha code: it does have exec-only
pages.  What it doesn't have is write-only.  And exec-only pages
aren't relevant to GCC's requirements, which used to be
read-implies-exec (exec-only breaks exec-implies-read).

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 19:28           ` Jamie Lokier
@ 2004-04-14 20:05             ` David Mosberger
  2004-04-14 21:05               ` Jamie Lokier
  0 siblings, 1 reply; 20+ messages in thread
From: David Mosberger @ 2004-04-14 20:05 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: davidm, linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

>>>>> On Wed, 14 Apr 2004 20:28:44 +0100, Jamie Lokier <jamie@shareable.org> said:

  >>  I know why: back in those days, GCC emitted code for nested C
  >> functions that assumed an executable stack.  Also, Linus wasn't
  >> terribly eager to turn off execute-permission on data/stacks.
  >> Even on ia64 we started out that way, until I saw the error in my
  >> ways.

  Jamie> We're both wrong.

No, Alpha Linux didn't map data without execute permission.

  Jamie> What it doesn't have is write-only.

Yes, that one is because earlier Alphas couldn't do subword stores, so
WRITE-permission necessitated READ-permission.

	--david

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: Non-Exec stack patches
  2004-04-14 18:30   ` Kurt Garloff
@ 2004-04-14 20:54     ` Jeff Dike
  0 siblings, 0 replies; 20+ messages in thread
From: Jeff Dike @ 2004-04-14 20:54 UTC (permalink / raw)
  To: Kurt Garloff, Jamie Lokier; +Cc: linux-kernel

garloff@suse.de said:
> I thought UML only runs on i386. And on i386, you have no NX feature.
> You can run i386 UML on AMD64 (with 64bit kernel) though. 

It also runs natively on and64 (i.e. a 64 bit UML).  The UML page table stuff
is going to need to be fixed for NX, but there's no reason that it won't work.

				Jeff


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 20:05             ` David Mosberger
@ 2004-04-14 21:05               ` Jamie Lokier
  2004-04-14 22:34                 ` David Mosberger
  0 siblings, 1 reply; 20+ messages in thread
From: Jamie Lokier @ 2004-04-14 21:05 UTC (permalink / raw)
  To: davidm
  Cc: linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

David Mosberger wrote:
> No, Alpha Linux didn't map data without execute permission.

That was true from Linux 1.1.67 (when Alpha was introduced) to 1.1.84
(when __[PS]* was introduced).  I'm not sure the Alpha target even
worked during those versions.  Since Linux 1.1.84, it has mapped pages
on the Alpha without execute permission: the _PAGE_FOE (fault on exec)
bit is set for mappings which don't have PROT_EXEC.

Btw, they used PAGE_EXECONLY in those days :)

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 21:05               ` Jamie Lokier
@ 2004-04-14 22:34                 ` David Mosberger
  2004-04-15 15:26                   ` Jamie Lokier
  0 siblings, 1 reply; 20+ messages in thread
From: David Mosberger @ 2004-04-14 22:34 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: davidm, linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

>>>>> On Wed, 14 Apr 2004 22:05:38 +0100, Jamie Lokier <jamie@shareable.org> said:

  Jamie> David Mosberger wrote:

  >> No, Alpha Linux didn't map data without execute permission.

  Jamie> That was true from Linux 1.1.67 (when Alpha was introduced)
  Jamie> to 1.1.84 (when __[PS]* was introduced).  I'm not sure the
  Jamie> Alpha target even worked during those versions.  Since Linux
  Jamie> 1.1.84, it has mapped pages on the Alpha without execute
  Jamie> permission: the _PAGE_FOE (fault on exec) bit is set for
  Jamie> mappings which don't have PROT_EXEC.

$ uname -a
Linux hostname 2.2.20 #2 Wed Mar 20 19:57:28 EST 2002 alpha GNU/Linux
$ cat /proc/self/maps | grep cat
0000000120000000-0000000120006000 r-xp 0000000000000000 08:01 309740     /bin/cat
0000000120014000-0000000120016000 rwxp 0000000000004000 08:01 309740     /bin/cat

As you can see, the data-segment is mapped with EXEC bit turned on.
Ditto for the stack segment, which I think is this one:

000000011fffe000-0000000120000000 rwxp 0000000000000000 00:00 0

	--david

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-14 22:34                 ` David Mosberger
@ 2004-04-15 15:26                   ` Jamie Lokier
  2004-04-15 17:45                     ` David Mosberger
  0 siblings, 1 reply; 20+ messages in thread
From: Jamie Lokier @ 2004-04-15 15:26 UTC (permalink / raw)
  To: davidm
  Cc: linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

David Mosberger wrote:
>   >> No, Alpha Linux didn't map data without execute permission.
> 
>   Jamie> That was true from Linux 1.1.67 (when Alpha was introduced)
>   Jamie> to 1.1.84 (when __[PS]* was introduced).  I'm not sure the
>   Jamie> Alpha target even worked during those versions.  Since Linux
>   Jamie> 1.1.84, it has mapped pages on the Alpha without execute
>   Jamie> permission: the _PAGE_FOE (fault on exec) bit is set for
>   Jamie> mappings which don't have PROT_EXEC.
> 
> $ uname -a
> Linux hostname 2.2.20 #2 Wed Mar 20 19:57:28 EST 2002 alpha GNU/Linux
> $ cat /proc/self/maps | grep cat
> 0000000120000000-0000000120006000 r-xp 0000000000000000 08:01 309740     /bin/cat
> 0000000120014000-0000000120016000 rwxp 0000000000004000 08:01 309740     /bin/cat
> 
> As you can see, the data-segment is mapped with EXEC bit turned on.
> Ditto for the stack segment, which I think is this one:
> 
> 000000011fffe000-0000000120000000 rwxp 0000000000000000 00:00 0

Lest we get more wires crossed, the "x" in /proc/self/maps indicates
that userspace _requested_ executable mapping with PROT_EXEC.

It is independent of whether the kernel and hardware can grant a
non-executable mapping.  On my Athlon:

08048000-0804c000 r-xp 00000000 03:02 95153      /bin/cat
0804c000-0804d000 rw-p 00003000 03:02 95153      /bin/cat
bfffe000-c0000000 rwxp fffff000 00:00 0

The stack is mapped executable, and the data segment is mapped
non-executable, according to /proc/self/maps which reflects the
PROT_EXEC request.  But in fact because of hardware limitations,
despite the "rw-p" my data segment is actually executable.

If you map a segment without PROT_EXEC on Alpha, using a kernel newer
than 1.1.84, you'll get a non-executable mapping.  That's what I mean
when I say the Alpha maps data without executable permission.  The ELF
loader appears to ask for exec permission anyway, which is another
thing entirely.

-- Jamie

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h>
  2004-04-15 15:26                   ` Jamie Lokier
@ 2004-04-15 17:45                     ` David Mosberger
  0 siblings, 0 replies; 20+ messages in thread
From: David Mosberger @ 2004-04-15 17:45 UTC (permalink / raw)
  To: Jamie Lokier
  Cc: davidm, linux-ia64, Siddha, Suresh B, Andrew Morton, Kurt Garloff,
	linux-kernel, mingo

>>>>> On Thu, 15 Apr 2004 16:26:00 +0100, Jamie Lokier <jamie@shareable.org> said:

  >> As you can see, the data-segment is mapped with EXEC bit turned on.
  >> Ditto for the stack segment, which I think is this one:

  >> 000000011fffe000-0000000120000000 rwxp 0000000000000000 00:00 0

  Jamie> The stack is mapped executable, and the data segment is mapped
  Jamie> non-executable, according to /proc/self/maps which reflects the
  Jamie> PROT_EXEC request.  But in fact because of hardware limitations,
  Jamie> despite the "rw-p" my data segment is actually executable.

Yes, but Alpha doesn't have this limitation.

  Jamie> If you map a segment without PROT_EXEC on Alpha, using a kernel newer
  Jamie> than 1.1.84, you'll get a non-executable mapping.

Sure.

  Jamie> That's what I mean when I say the Alpha maps data without
  Jamie> executable permission.  The ELF loader appears to ask for
  Jamie> exec permission anyway, which is another thing entirely.

It's not the ELF loader, it's the kernel.  See VM_DATA_DEFAULT_FLAGS
in asm-alpha/page.h.

	--david

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2004-04-15 17:47 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-04-14  7:28 Non-Exec stack patches Siddha, Suresh B
2004-04-14  8:23 ` Jamie Lokier
2004-04-14  8:35   ` PowerPC exec page protection Jamie Lokier
2004-04-14  8:44     ` Anton Blanchard
2004-04-14  9:35       ` Jamie Lokier
2004-04-14 11:37   ` [PATCH] (IA64) Fix ugly __[PS]* macros in <asm-ia64/pgtable.h> Jamie Lokier
2004-04-14 16:07     ` David Mosberger
2004-04-14 18:46       ` Jamie Lokier
2004-04-14 19:02         ` David Mosberger
2004-04-14 19:14           ` Jamie Lokier
2004-04-14 19:28           ` Jamie Lokier
2004-04-14 20:05             ` David Mosberger
2004-04-14 21:05               ` Jamie Lokier
2004-04-14 22:34                 ` David Mosberger
2004-04-15 15:26                   ` Jamie Lokier
2004-04-15 17:45                     ` David Mosberger
2004-04-14  9:47 ` Non-Exec stack patches Jamie Lokier
2004-04-14 18:30   ` Kurt Garloff
2004-04-14 20:54     ` Jeff Dike
2004-04-14 18:35 ` Kurt Garloff

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox