[PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

This patch adds support for the PROT_FINAL flag to
the mmap() and mprotect() syscalls.

The PROT_FINAL flag indicates that the requested set
of protection bits should be final, i.e., it shall
not be allowed for a subsequent mprotect call to
set protection bits that were not set already.

This is mainly intended for the dynamic linker,
which sets up the address space on behalf of
dynamic binaries. By using this flag, it can
prevent exploited code from remapping read-only
executable code or data sections read-write.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@gmail.com>
---
 arch/powerpc/include/asm/mman.h   |    3 ++-
 include/asm-generic/mman-common.h |    1 +
 include/linux/mman.h              |    3 ++-
 mm/mmap.c                         |    9 +++++++++
 mm/mprotect.c                     |    8 ++++++++
 5 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index d4a7f64..c0014eb 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -52,7 +52,8 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
 
 static inline int arch_validate_prot(unsigned long prot)
 {
-	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
+	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM
+		     | PROT_SAO | PROT_FINAL))
 		return 0;
 	if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO))
 		return 0;
diff --git a/include/asm-generic/mman-common.h b/include/asm-generic/mman-common.h
index d030d2c..5687993 100644
--- a/include/asm-generic/mman-common.h
+++ b/include/asm-generic/mman-common.h
@@ -10,6 +10,7 @@
 #define PROT_WRITE	0x2		/* page can be written */
 #define PROT_EXEC	0x4		/* page can be executed */
 #define PROT_SEM	0x8		/* page may be used for atomic ops */
+#define PROT_FINAL	0x80		/* unset page prot bits cannot be set later */
 #define PROT_NONE	0x0		/* page can not be accessed */
 #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
 #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
diff --git a/include/linux/mman.h b/include/linux/mman.h
index 8b74e9b..c11b1ab 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -51,7 +51,8 @@ static inline void vm_unacct_memory(long pages)
  */
 static inline int arch_validate_prot(unsigned long prot)
 {
-	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
+	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC
+			 | PROT_SEM | PROT_FINAL)) == 0;
 }
 #define arch_validate_prot arch_validate_prot
 #endif
diff --git a/mm/mmap.c b/mm/mmap.c
index e3e8691..a043fa9 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1101,6 +1101,15 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
 		}
 	}
 
+	/*
+	 * PROT_FINAL indicates that prot bits not requested by this
+	 * mmap() call cannot be added later
+	 */
+	if (prot & PROT_FINAL)
+		vm_flags &= ~(VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+			    | (vm_flags << 4);
+
+
 	return mmap_region(file, addr, len, flags, vm_flags, pgoff);
 }
 
diff --git a/mm/mprotect.c b/mm/mprotect.c
index a409926..7a39f73 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -301,6 +301,14 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 			goto out;
 		}
 
+		/*
+		 * PROT_FINAL indicates that prot bits removed by this
+		 * mprotect() call cannot be added later
+		 */
+		if (prot & PROT_FINAL)
+			newflags &= ~(VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+				    | (newflags << 4);
+
 		error = security_file_mprotect(vma, reqprot, prot);
 		if (error)
 			goto out;
-- 
1.7.9.5

Re: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Kees Cook]

Hi,

On Tue, Aug 14, 2012 at 09:11:11PM +0200, Ard Biesheuvel wrote:
> This patch adds support for the PROT_FINAL flag to
> the mmap() and mprotect() syscalls.
> 
> The PROT_FINAL flag indicates that the requested set
> of protection bits should be final, i.e., it shall
> not be allowed for a subsequent mprotect call to
> set protection bits that were not set already.
> 
> This is mainly intended for the dynamic linker,
> which sets up the address space on behalf of
> dynamic binaries. By using this flag, it can
> prevent exploited code from remapping read-only
> executable code or data sections read-write.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@gmail.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

This seems like a good idea to me. It would allow more than just the
loader to harden userspace allocations. It's a more direct version of
PaX's "MPROTECT" feature[1]. That feature hardens existing loaders,
but doesn't play nice with JITs (like Java), but this lets a loader
(or JIT) opt-in to the protection and have some direct control over it.

It seems like there needs to be a sensible way to detect that this flag is
available, though.

-Kees

[1] http://pax.grsecurity.net/docs/mprotect.txt

-- 
Kees Cook                                            @outflux.net

Re: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

> This seems like a good idea to me. It would allow more than just the
> loader to harden userspace allocations. It's a more direct version of
> PaX's "MPROTECT" feature[1]. That feature hardens existing loaders,
> but doesn't play nice with JITs (like Java), but this lets a loader
> (or JIT) opt-in to the protection and have some direct control over it.
>

If desired, additional restrictions can be imposed by using the
security framework, e.g,, disallow non-final r-x mappings.

> It seems like there needs to be a sensible way to detect that this flag is
> available, though.
>

I am open for suggestions to address this. Our particular
implementation of the loader (on an embedded system) tries to set it
on the first mmap invocation, and stops trying if it fails. Not the
most elegant approach, I know ...

-- 
Ard.


> -Kees
>
> [1] http://pax.grsecurity.net/docs/mprotect.txt
>
> --
> Kees Cook                                            @outflux.net

Re: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Kees Cook]

Hi,

On Mon, Aug 20, 2012 at 2:48 PM, Ard Biesheuvel
<ard.biesheuvel@gmail.com> wrote:
>> This seems like a good idea to me. It would allow more than just the
>> loader to harden userspace allocations. It's a more direct version of
>> PaX's "MPROTECT" feature[1]. That feature hardens existing loaders,
>> but doesn't play nice with JITs (like Java), but this lets a loader
>> (or JIT) opt-in to the protection and have some direct control over it.
>
> If desired, additional restrictions can be imposed by using the
> security framework, e.g,, disallow non-final r-x mappings.

Interesting; what kind of interface did you have in mind?

>> It seems like there needs to be a sensible way to detect that this flag is
>> available, though.
>
> I am open for suggestions to address this. Our particular
> implementation of the loader (on an embedded system) tries to set it
> on the first mmap invocation, and stops trying if it fails. Not the
> most elegant approach, I know ...

Actually, that seems easiest.

Has there been any more progress on this patch over-all?

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

2012/10/2 Kees Cook <keescook@chromium.org>:
>> If desired, additional restrictions can be imposed by using the
>> security framework, e.g,, disallow non-final r-x mappings.
>
> Interesting; what kind of interface did you have in mind?
>

The 'interface' we use is a LSM .ko which registers handlers for
mmap() and mprotect() that fail the respective invocations if the
passed arguments do not adhere to the policy.

>>> It seems like there needs to be a sensible way to detect that this flag is
>>> available, though.
>>
>> I am open for suggestions to address this. Our particular
>> implementation of the loader (on an embedded system) tries to set it
>> on the first mmap invocation, and stops trying if it fails. Not the
>> most elegant approach, I know ...
>
> Actually, that seems easiest.
>
> Has there been any more progress on this patch over-all?
>

No progress.

-- 
Ard.

Re: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Kees Cook]

On Tue, Oct 2, 2012 at 2:41 PM, Ard Biesheuvel <ard.biesheuvel@gmail.com> wrote:
> 2012/10/2 Kees Cook <keescook@chromium.org>:
>>> If desired, additional restrictions can be imposed by using the
>>> security framework, e.g,, disallow non-final r-x mappings.
>>
>> Interesting; what kind of interface did you have in mind?
>
> The 'interface' we use is a LSM .ko which registers handlers for
> mmap() and mprotect() that fail the respective invocations if the
> passed arguments do not adhere to the policy.

Seems reasonable.

>>>> It seems like there needs to be a sensible way to detect that this flag is
>>>> available, though.
>>>
>>> I am open for suggestions to address this. Our particular
>>> implementation of the loader (on an embedded system) tries to set it
>>> on the first mmap invocation, and stops trying if it fails. Not the
>>> most elegant approach, I know ...
>>
>> Actually, that seems easiest.
>>
>> Has there been any more progress on this patch over-all?
>
> No progress.

Al, Andrew, anyone? Thoughts on this?
(First email is https://lkml.org/lkml/2012/8/14/448)

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Andrew Morton]

On Tue, 2 Oct 2012 15:10:56 -0700
Kees Cook <keescook@chromium.org> wrote:

> >> Has there been any more progress on this patch over-all?
> >
> > No progress.
> 
> Al, Andrew, anyone? Thoughts on this?
> (First email is https://lkml.org/lkml/2012/8/14/448)

Wasn't cc'ed, missed it.

The patch looks straightforward enough.  Have the maintainers of the
runtime linker (I guess that's glibc) provided any feedback on the
proposal?

Re: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Hugh Dickins]

On Tue, 2 Oct 2012, Andrew Morton wrote:
> On Tue, 2 Oct 2012 15:10:56 -0700
> Kees Cook <keescook@chromium.org> wrote:
> 
> > >> Has there been any more progress on this patch over-all?
> > >
> > > No progress.
> > 
> > Al, Andrew, anyone? Thoughts on this?
> > (First email is https://lkml.org/lkml/2012/8/14/448)
> 
> Wasn't cc'ed, missed it.
> 
> The patch looks straightforward enough.  Have the maintainers of the
> runtime linker (I guess that's glibc) provided any feedback on the
> proposal?

It looks reasonable to me too.  I checked through VM_MAYflag handling
and don't expect surprises (a few places already turn off VM_MAYWRITE
in much the same way that this does, I hadn't realized).

I'm disappointed to find that our mmap() is lax about checking its
PROT and MAP args, so old kernels will accept PROT_FINAL but do
nothing with it.  Luckily mprotect() is stricter, so that can be
used to check for whether it's supported.

The patch does need to be slightly extended though: alpha, mips,
parisc and xtensa have their own include/asm/mman.h, which does
not include asm-generic/mman-common.h at all.

Hugh

Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

This patch adds support for the PROT_FINAL flag to
the mmap() and mprotect() syscalls.

The PROT_FINAL flag indicates that the requested set
of protection bits should be final, i.e., it shall
not be allowed for a subsequent mprotect call to
set protection bits that were not set already.

This is mainly intended for the dynamic linker,
which sets up the address space on behalf of
dynamic binaries. By using this flag, it can
prevent exploited code from remapping read-only
executable code or data sections read-write.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@gmail.com>
---
 arch/alpha/include/asm/mman.h     |    1 +
 arch/mips/include/asm/mman.h      |    1 +
 arch/parisc/include/asm/mman.h    |    1 +
 arch/powerpc/include/asm/mman.h   |    3 ++-
 arch/xtensa/include/asm/mman.h    |    1 +
 include/asm-generic/mman-common.h |    1 +
 include/linux/mman.h              |    3 ++-
 mm/mmap.c                         |    8 ++++++++
 mm/mprotect.c                     |    8 ++++++++
 9 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/arch/alpha/include/asm/mman.h b/arch/alpha/include/asm/mman.h
index cbeb361..ab46252 100644
--- a/arch/alpha/include/asm/mman.h
+++ b/arch/alpha/include/asm/mman.h
@@ -6,6 +6,7 @@
 #define PROT_EXEC	0x4		/* page can be executed */
 #define PROT_SEM	0x8		/* page may be used for atomic ops */
 #define PROT_NONE	0x0		/* page can not be accessed */
+#define PROT_FINAL	0x80	/* unset page prot bits cannot be set later */
 #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
 #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
 
diff --git a/arch/mips/include/asm/mman.h b/arch/mips/include/asm/mman.h
index 46d3da0..9cd50e4 100644
--- a/arch/mips/include/asm/mman.h
+++ b/arch/mips/include/asm/mman.h
@@ -20,6 +20,7 @@
 #define PROT_EXEC	0x04		/* page can be executed */
 /*			0x08		   reserved for PROT_EXEC_NOFLUSH */
 #define PROT_SEM	0x10		/* page may be used for atomic ops */
+#define PROT_FINAL	0x80		/* unset page prot bits cannot be set later */
 #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
 #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
 
diff --git a/arch/parisc/include/asm/mman.h b/arch/parisc/include/asm/mman.h
index 12219eb..0cf18e3 100644
--- a/arch/parisc/include/asm/mman.h
+++ b/arch/parisc/include/asm/mman.h
@@ -6,6 +6,7 @@
 #define PROT_EXEC	0x4		/* page can be executed */
 #define PROT_SEM	0x8		/* page may be used for atomic ops */
 #define PROT_NONE	0x0		/* page can not be accessed */
+#define PROT_FINAL	0x80	/* unset page prot bits cannot be set later */
 #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
 #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
 
diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
index d4a7f64..c0014eb 100644
--- a/arch/powerpc/include/asm/mman.h
+++ b/arch/powerpc/include/asm/mman.h
@@ -52,7 +52,8 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
 
 static inline int arch_validate_prot(unsigned long prot)
 {
-	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
+	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM
+		     | PROT_SAO | PROT_FINAL))
 		return 0;
 	if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO))
 		return 0;
diff --git a/arch/xtensa/include/asm/mman.h b/arch/xtensa/include/asm/mman.h
index 25bc6c1..680b3c4 100644
--- a/arch/xtensa/include/asm/mman.h
+++ b/arch/xtensa/include/asm/mman.h
@@ -27,6 +27,7 @@
 #define PROT_EXEC	0x4		/* page can be executed */
 
 #define PROT_SEM	0x10		/* page may be used for atomic ops */
+#define PROT_FINAL	0x80		/* unset page prot bits cannot be set later */
 #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
 #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end fo growsup vma */
 
diff --git a/include/asm-generic/mman-common.h b/include/asm-generic/mman-common.h
index d030d2c..5687993 100644
--- a/include/asm-generic/mman-common.h
+++ b/include/asm-generic/mman-common.h
@@ -10,6 +10,7 @@
 #define PROT_WRITE	0x2		/* page can be written */
 #define PROT_EXEC	0x4		/* page can be executed */
 #define PROT_SEM	0x8		/* page may be used for atomic ops */
+#define PROT_FINAL	0x80		/* unset page prot bits cannot be set later */
 #define PROT_NONE	0x0		/* page can not be accessed */
 #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
 #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
diff --git a/include/linux/mman.h b/include/linux/mman.h
index 8b74e9b..c11b1ab 100644
--- a/include/linux/mman.h
+++ b/include/linux/mman.h
@@ -51,7 +51,8 @@ static inline void vm_unacct_memory(long pages)
  */
 static inline int arch_validate_prot(unsigned long prot)
 {
-	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
+	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC
+			 | PROT_SEM | PROT_FINAL)) == 0;
 }
 #define arch_validate_prot arch_validate_prot
 #endif
diff --git a/mm/mmap.c b/mm/mmap.c
index 872441e..292f988 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1101,6 +1101,14 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
 		}
 	}
 
+	/*
+	 * PROT_FINAL indicates that prot bits not requested by this
+	 * mmap() call cannot be added later
+	 */
+	if (prot & PROT_FINAL)
+		vm_flags &= ~(VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+			    | (vm_flags << 4);
+
 	return mmap_region(file, addr, len, flags, vm_flags, pgoff);
 }
 
diff --git a/mm/mprotect.c b/mm/mprotect.c
index a409926..7a39f73 100644
--- a/mm/mprotect.c
+++ b/mm/mprotect.c
@@ -301,6 +301,14 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
 			goto out;
 		}
 
+		/*
+		 * PROT_FINAL indicates that prot bits removed by this
+		 * mprotect() call cannot be added later
+		 */
+		if (prot & PROT_FINAL)
+			newflags &= ~(VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
+				    | (newflags << 4);
+
 		error = security_file_mprotect(vma, reqprot, prot);
 		if (error)
 			goto out;
-- 
1.7.9.5

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Kees Cook]

On Wed, Oct 3, 2012 at 7:43 AM, Ard Biesheuvel <ard.biesheuvel@gmail.com> wrote:
> This patch adds support for the PROT_FINAL flag to
> the mmap() and mprotect() syscalls.
>
> The PROT_FINAL flag indicates that the requested set
> of protection bits should be final, i.e., it shall
> not be allowed for a subsequent mprotect call to
> set protection bits that were not set already.
>
> This is mainly intended for the dynamic linker,
> which sets up the address space on behalf of
> dynamic binaries. By using this flag, it can
> prevent exploited code from remapping read-only
> executable code or data sections read-write.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@gmail.com>

Reviewed-by: Kees Cook <keescook@chromium.org>

If it wasn't clear before, I like this idea. :)

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Hugh Dickins]

On Wed, 3 Oct 2012, Ard Biesheuvel wrote:

> This patch adds support for the PROT_FINAL flag to
> the mmap() and mprotect() syscalls.
> 
> The PROT_FINAL flag indicates that the requested set
> of protection bits should be final, i.e., it shall
> not be allowed for a subsequent mprotect call to
> set protection bits that were not set already.
> 
> This is mainly intended for the dynamic linker,
> which sets up the address space on behalf of
> dynamic binaries. By using this flag, it can
> prevent exploited code from remapping read-only
> executable code or data sections read-write.
> 
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@gmail.com>

Acked-by: Hugh Dickins <hughd@google.com>

(and I rather enjoy the way you ask PROT_GROWSDOWN for an alibi, in case
someone accuses the PROT_FINAL comment of a checkpatch linelength crime.)

> ---
>  arch/alpha/include/asm/mman.h     |    1 +
>  arch/mips/include/asm/mman.h      |    1 +
>  arch/parisc/include/asm/mman.h    |    1 +
>  arch/powerpc/include/asm/mman.h   |    3 ++-
>  arch/xtensa/include/asm/mman.h    |    1 +
>  include/asm-generic/mman-common.h |    1 +
>  include/linux/mman.h              |    3 ++-
>  mm/mmap.c                         |    8 ++++++++
>  mm/mprotect.c                     |    8 ++++++++
>  9 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/alpha/include/asm/mman.h b/arch/alpha/include/asm/mman.h
> index cbeb361..ab46252 100644
> --- a/arch/alpha/include/asm/mman.h
> +++ b/arch/alpha/include/asm/mman.h
> @@ -6,6 +6,7 @@
>  #define PROT_EXEC	0x4		/* page can be executed */
>  #define PROT_SEM	0x8		/* page may be used for atomic ops */
>  #define PROT_NONE	0x0		/* page can not be accessed */
> +#define PROT_FINAL	0x80	/* unset page prot bits cannot be set later */
>  #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
>  #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
>  
> diff --git a/arch/mips/include/asm/mman.h b/arch/mips/include/asm/mman.h
> index 46d3da0..9cd50e4 100644
> --- a/arch/mips/include/asm/mman.h
> +++ b/arch/mips/include/asm/mman.h
> @@ -20,6 +20,7 @@
>  #define PROT_EXEC	0x04		/* page can be executed */
>  /*			0x08		   reserved for PROT_EXEC_NOFLUSH */
>  #define PROT_SEM	0x10		/* page may be used for atomic ops */
> +#define PROT_FINAL	0x80		/* unset page prot bits cannot be set later */
>  #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
>  #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
>  
> diff --git a/arch/parisc/include/asm/mman.h b/arch/parisc/include/asm/mman.h
> index 12219eb..0cf18e3 100644
> --- a/arch/parisc/include/asm/mman.h
> +++ b/arch/parisc/include/asm/mman.h
> @@ -6,6 +6,7 @@
>  #define PROT_EXEC	0x4		/* page can be executed */
>  #define PROT_SEM	0x8		/* page may be used for atomic ops */
>  #define PROT_NONE	0x0		/* page can not be accessed */
> +#define PROT_FINAL	0x80	/* unset page prot bits cannot be set later */
>  #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
>  #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
>  
> diff --git a/arch/powerpc/include/asm/mman.h b/arch/powerpc/include/asm/mman.h
> index d4a7f64..c0014eb 100644
> --- a/arch/powerpc/include/asm/mman.h
> +++ b/arch/powerpc/include/asm/mman.h
> @@ -52,7 +52,8 @@ static inline pgprot_t arch_vm_get_page_prot(unsigned long vm_flags)
>  
>  static inline int arch_validate_prot(unsigned long prot)
>  {
> -	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM | PROT_SAO))
> +	if (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM
> +		     | PROT_SAO | PROT_FINAL))
>  		return 0;
>  	if ((prot & PROT_SAO) && !cpu_has_feature(CPU_FTR_SAO))
>  		return 0;
> diff --git a/arch/xtensa/include/asm/mman.h b/arch/xtensa/include/asm/mman.h
> index 25bc6c1..680b3c4 100644
> --- a/arch/xtensa/include/asm/mman.h
> +++ b/arch/xtensa/include/asm/mman.h
> @@ -27,6 +27,7 @@
>  #define PROT_EXEC	0x4		/* page can be executed */
>  
>  #define PROT_SEM	0x10		/* page may be used for atomic ops */
> +#define PROT_FINAL	0x80		/* unset page prot bits cannot be set later */
>  #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
>  #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end fo growsup vma */
>  
> diff --git a/include/asm-generic/mman-common.h b/include/asm-generic/mman-common.h
> index d030d2c..5687993 100644
> --- a/include/asm-generic/mman-common.h
> +++ b/include/asm-generic/mman-common.h
> @@ -10,6 +10,7 @@
>  #define PROT_WRITE	0x2		/* page can be written */
>  #define PROT_EXEC	0x4		/* page can be executed */
>  #define PROT_SEM	0x8		/* page may be used for atomic ops */
> +#define PROT_FINAL	0x80		/* unset page prot bits cannot be set later */
>  #define PROT_NONE	0x0		/* page can not be accessed */
>  #define PROT_GROWSDOWN	0x01000000	/* mprotect flag: extend change to start of growsdown vma */
>  #define PROT_GROWSUP	0x02000000	/* mprotect flag: extend change to end of growsup vma */
> diff --git a/include/linux/mman.h b/include/linux/mman.h
> index 8b74e9b..c11b1ab 100644
> --- a/include/linux/mman.h
> +++ b/include/linux/mman.h
> @@ -51,7 +51,8 @@ static inline void vm_unacct_memory(long pages)
>   */
>  static inline int arch_validate_prot(unsigned long prot)
>  {
> -	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM)) == 0;
> +	return (prot & ~(PROT_READ | PROT_WRITE | PROT_EXEC
> +			 | PROT_SEM | PROT_FINAL)) == 0;
>  }
>  #define arch_validate_prot arch_validate_prot
>  #endif
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 872441e..292f988 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1101,6 +1101,14 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
>  		}
>  	}
>  
> +	/*
> +	 * PROT_FINAL indicates that prot bits not requested by this
> +	 * mmap() call cannot be added later
> +	 */
> +	if (prot & PROT_FINAL)
> +		vm_flags &= ~(VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
> +			    | (vm_flags << 4);
> +
>  	return mmap_region(file, addr, len, flags, vm_flags, pgoff);
>  }
>  
> diff --git a/mm/mprotect.c b/mm/mprotect.c
> index a409926..7a39f73 100644
> --- a/mm/mprotect.c
> +++ b/mm/mprotect.c
> @@ -301,6 +301,14 @@ SYSCALL_DEFINE3(mprotect, unsigned long, start, size_t, len,
>  			goto out;
>  		}
>  
> +		/*
> +		 * PROT_FINAL indicates that prot bits removed by this
> +		 * mprotect() call cannot be added later
> +		 */
> +		if (prot & PROT_FINAL)
> +			newflags &= ~(VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC)
> +				    | (newflags << 4);
> +
>  		error = security_file_mprotect(vma, reqprot, prot);
>  		if (error)
>  			goto out;
> -- 
> 1.7.9.5
> 
> 
> 

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Andrew Morton]

On Wed, 03 Oct 2012 16:43:53 +0200
Ard Biesheuvel <ard.biesheuvel@gmail.com> wrote:

> This patch adds support for the PROT_FINAL flag to
> the mmap() and mprotect() syscalls.
> 
> The PROT_FINAL flag indicates that the requested set
> of protection bits should be final, i.e., it shall
> not be allowed for a subsequent mprotect call to
> set protection bits that were not set already.
> 
> This is mainly intended for the dynamic linker,
> which sets up the address space on behalf of
> dynamic binaries. By using this flag, it can
> prevent exploited code from remapping read-only
> executable code or data sections read-write.

Again: has this proposal been reviewed by the glibc maintainers?  If
so, what was their position on it?

Also, you earlier stated that "It's a more direct version of PaX's
"MPROTECT" feature[1]".  This is useful information.  Please update the
changelog to describe that PaX feature and to describe the difference
between the two, and the reasons for that difference.

It sounds as though the PaX developers could provide useful review
input on this proposal.  Do they know about it?  If so, what is their
position?

Thanks.

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Kees Cook]

On Wed, Oct 3, 2012 at 2:18 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> On Wed, 03 Oct 2012 16:43:53 +0200
> Ard Biesheuvel <ard.biesheuvel@gmail.com> wrote:
>
>> This patch adds support for the PROT_FINAL flag to
>> the mmap() and mprotect() syscalls.
>>
>> The PROT_FINAL flag indicates that the requested set
>> of protection bits should be final, i.e., it shall
>> not be allowed for a subsequent mprotect call to
>> set protection bits that were not set already.
>>
>> This is mainly intended for the dynamic linker,
>> which sets up the address space on behalf of
>> dynamic binaries. By using this flag, it can
>> prevent exploited code from remapping read-only
>> executable code or data sections read-write.
>
> Again: has this proposal been reviewed by the glibc maintainers?  If
> so, what was their position on it?

Ard have you talked with them? I would expect it would be welcomed.
Roland, do you know who would be the right person to ask about this
for glibc? (patch: https://lkml.org/lkml/2012/10/3/262)

> Also, you earlier stated that "It's a more direct version of PaX's
> "MPROTECT" feature[1]".  This is useful information.  Please update the
> changelog to describe that PaX feature and to describe the difference
> between the two, and the reasons for that difference.

AIUI, it's much more aggressive. It tries to protect all processes
automatically (rather than this which is at the request of the
process) and gets in the way of things like Java that expect to be
able to do w+x mappings.

> It sounds as though the PaX developers could provide useful review
> input on this proposal.  Do they know about it?  If so, what is their
> position?

I'd rather not speak for them, but I understood it to be along the
lines of "that's nice, we'll keep ours." :) (Now added to CC.)

-Kees

-- 
Kees Cook
Chrome OS Security

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Roland McGrath]

> > Again: has this proposal been reviewed by the glibc maintainers?  If
> > so, what was their position on it?
> 
> Ard have you talked with them? I would expect it would be welcomed.
> Roland, do you know who would be the right person to ask about this
> for glibc? (patch: https://lkml.org/lkml/2012/10/3/262)

It's me and others.  Someone should post (avoiding lots of cross-posting,
please) to libc-alpha@sourceware.org with a straightforward description of
the userland semantics being introduced (we don't need to see kernel
patches) and (ideally) a proposed patch for the dynamic linker code to make
use of it (a strawman example is OK to begin with).


Thanks,
Roland

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

2012/10/4 Kees Cook <keescook@chromium.org>:
> On Wed, Oct 3, 2012 at 2:18 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
>> Again: has this proposal been reviewed by the glibc maintainers?  If
>> so, what was their position on it?
>
> Ard have you talked with them? I would expect it would be welcomed.
> Roland, do you know who would be the right person to ask about this
> for glibc? (patch: https://lkml.org/lkml/2012/10/3/262)
>

I haven't spoken to the glibc people. However, I have been in contact
with the Android/Bionic people (on CC) to merge back some of the
security/hardening enhancements I have been making to Android on
behalf of my employer. This change is part of that.

I will post to the libc mailing list and once I get some feedback I
will post the summary here.

>> Also, you earlier stated that "It's a more direct version of PaX's
>> "MPROTECT" feature[1]".  This is useful information.  Please update the
>> changelog to describe that PaX feature and to describe the difference
>> between the two, and the reasons for that difference.
>
> AIUI, it's much more aggressive. It tries to protect all processes
> automatically (rather than this which is at the request of the
> process) and gets in the way of things like Java that expect to be
> able to do w+x mappings.
>

The main difference is that PROT_FINAL is just a feature. It is
completely up to the userland to decide in which cases (if at all) to
use it, which -as Kees points out- makes it a lot easier to allow
certain cases (ashmem filebacked rwx mappings for the JIT heap) while
ruling out all others. By implementing it in the loader, the vast
majority of code, rodata and relro mappings are hardened while
explicit uses of mmap/mprotect in userland code are completely
unaffected.

OTOH PaX's MPROTECT is a full blown policy implemented inside the
kernel. It imposes rules on which of the various combinations of VM_*
and VM_MAY* are permissible with little or no control from userland.

-- 
Ard.

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[PaX Team]

On 3 Oct 2012 at 15:19, Kees Cook wrote:
> On Wed, Oct 3, 2012 at 2:18 PM, Andrew Morton <akpm@linux-foundation.org> wrote:
> > It sounds as though the PaX developers could provide useful review
> > input on this proposal.  Do they know about it?  If so, what is their
> > position?
> 
> I'd rather not speak for them, but I understood it to be along the
> lines of "that's nice, we'll keep ours." :) (Now added to CC.)

thanks for the heads up Kees ;). i've read through the thread since
the first post in August and i have one big lingering question before
i can say anything about the implementation: what is the whole point
of this exercise? in particular, what kind of threat model underlies
the design? what do we expect from an attacker? what restrictions does
he operate under?

i'm asking these basic questions because i have the feeling that there's
some misundertanding here. the following is my speculation based on Ard's
first post that was quoted below already (https://lkml.org/lkml/2012/8/14/448):

> >> This patch adds support for the PROT_FINAL flag to
> >> the mmap() and mprotect() syscalls.
> >>
> >> The PROT_FINAL flag indicates that the requested set
> >> of protection bits should be final, i.e., it shall
> >> not be allowed for a subsequent mprotect call to
> >> set protection bits that were not set already.
> >>
> >> This is mainly intended for the dynamic linker,
> >> which sets up the address space on behalf of
> >> dynamic binaries. By using this flag, it can
> >> prevent exploited code from remapping read-only
> >> executable code or data sections read-write.

now this last paragraph/sentence seems to be the closest thing that
describes a scenario where we have some vulnerability (i'm guessing
of the memory corruption kind) that allows an attacker (exploit writer)
to force the attacked vulnerable application to perform a gratuitous
mprotect(PROT_WRITE|PROT_EXEC) on some library/etc mapping and prepare
it for shellcode injection/execution (a.k.a. 'game over' or the 'holy
grail' ;). so far so good. the question that arises here is that what
prevents the same exploit technique (say, ret2libc in this case) from
executing an open/mmap(PROT_WRITE|PROT_EXEC) of the very same library
that PROT_FINAL would protect? or just a plain anon mmap that is just as
good for shellcode execution? in other words, if the attacker can make
the attacked application execute mprotect with arbitrary parameters,
then why can't the same attacker execute open/mmap/etc as well, completely
circumventing the proposed solution? i hope you see now why this issue
has to be settled before anyone looks into the implementation details.

now later in the thread Ard said this:

> The 'interface' we use is a LSM .ko which registers handlers for
> mmap() and mprotect() that fail the respective invocations if the
> passed arguments do not adhere to the policy.

i'm guessing again that their LSM tries to tackle the above described
scenarios except we don't know if this LSM will ever become public,
whether/how other LSMs will acquire the same capabilities and why it's
not mentioned in the PROT_FINAL submission that by itself this feature
doesn't increase security. i'm also wondering what kind of policy can
allow ld.so to load a library but forbid it a second time, it doesn't
seem compatible with real-life cases (think dynamically loaded and
unloaded plugins), not to mention the control of anonymous mappings.

last but not least, just saw this from Ard while not on CC:

> ptrace() doesn't care whether or not the process itself can write to
> its .text segment.

ptrace cares about VM_MAYWRITE which PROT_FINAL can take away (under
PaX MPROTECT'ed processes cannot be debugged with sw breakpoints).

> Could we at least agree on the fundamental notion that the special
> powers the loader has to modify .text and .rodata sections are hardly
> ever needed by the programs themselves? In that sense, this is similar
> to dropping root privileges when not required anymore, and that is
> typically recognized as a sensible idea.

the difference is that the uid is a process-wide attribute, whereas
PROT_FINAL isn't, unlike PaX's MPROTECT. in other words, dropping root
is irreversible but PROT_FINAL isn't, one just has to create an entirely
new mapping to acquire the access rights that PROT_FINAL was supposed to
prevent (again, all this modulo an LSM and policies).

> > Again: has this proposal been reviewed by the glibc maintainers?  If
> > Also, you earlier stated that "It's a more direct version of PaX's
> > "MPROTECT" feature[1]".  This is useful information.  Please update the
> > changelog to describe that PaX feature and to describe the difference
> > between the two, and the reasons for that difference.
> 
> AIUI, it's much more aggressive. It tries to protect all processes
> automatically (rather than this which is at the request of the
> process) and gets in the way of things like Java that expect to be
> able to do w+x mappings.

for the gory details: http://pax.grsecurity.net/docs/mprotect.txt.
the basic idea is that you either grant a process the ability to
generate code at runtime (in whatever form) or you take it away for
good, as nobody has come up with a secure middle-ground so far (it's
a very non-trivial exercise to create such a mechanism assuming the
generic 'attacker can read/write arbitrary memory' model).

as for java/etc, PaX has a per-process control mechanism to disable
this enforcement, but it's static (decided at process creation time,
ideally from a MAC system), not dynamic (something an attack could
abuse).

cheers,
 PaX Team

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

2012/10/4 PaX Team <pageexec@freemail.hu>:

Thanks for taking a look at this matter.

>> >> This is mainly intended for the dynamic linker,
>> >> which sets up the address space on behalf of
>> >> dynamic binaries. By using this flag, it can
>> >> prevent exploited code from remapping read-only
>> >> executable code or data sections read-write.
>
> now this last paragraph/sentence seems to be the closest thing that
> describes a scenario where we have some vulnerability (i'm guessing
> of the memory corruption kind) that allows an attacker (exploit writer)
> to force the attacked vulnerable application to perform a gratuitous
> mprotect(PROT_WRITE|PROT_EXEC) on some library/etc mapping and prepare
> it for shellcode injection/execution (a.k.a. 'game over' or the 'holy
> grail' ;). so far so good. the question that arises here is that what
> prevents the same exploit technique (say, ret2libc in this case) from
> executing an open/mmap(PROT_WRITE|PROT_EXEC) of the very same library
> that PROT_FINAL would protect? or just a plain anon mmap that is just as
> good for shellcode execution? in other words, if the attacker can make
> the attacked application execute mprotect with arbitrary parameters,
> then why can't the same attacker execute open/mmap/etc as well, completely
> circumventing the proposed solution? i hope you see now why this issue
> has to be settled before anyone looks into the implementation details.
>

The main difference here is that it is much easier to doctor a set of
stack frames that issues a mprotect(+PROT_EXEC) on the whole address
space than it is to re-issue the exact same mmap() call (with
MAP_FIXED this time, and the exact offset the kernel decided to load
it this time around) that will put the same code/data in exactly the
same place in memory (relocated and all) without blowing up your
process. In our specific implementation, it is mainly about rodata
(public keys) rather than executable code, but the same applies; for
us, it is more about rigging binaries: the attacker may not be
interested in hijacking the whole process, but just nop'ing out some
code that makes the process behave more to his liking.

>> The 'interface' we use is a LSM .ko which registers handlers for
>> mmap() and mprotect() that fail the respective invocations if the
>> passed arguments do not adhere to the policy.
>
> i'm guessing again that their LSM tries to tackle the above described
> scenarios except we don't know if this LSM will ever become public,
> whether/how other LSMs will acquire the same capabilities and why it's
> not mentioned in the PROT_FINAL submission that by itself this feature
> doesn't increase security. i'm also wondering what kind of policy can
> allow ld.so to load a library but forbid it a second time, it doesn't
> seem compatible with real-life cases (think dynamically loaded and
> unloaded plugins), not to mention the control of anonymous mappings.
>

The LSM encapsulates the policy, and relies on the PROT_FINAL feature.
The fact that the policy can impose the use of PROT_FINAL in
particular cases makes the implementation of the policy potentially
much simpler than that of PaX MPROTECT (but not necessarily as
secure).

However, let's decide first whether there is a point to having some
control over the VM_MAY* flags directly. If yes, then the patch makes
sense, otherwise it doesn't. How policies may be built on top of that
is part of another debate.

> last but not least, just saw this from Ard while not on CC:
>
>> ptrace() doesn't care whether or not the process itself can write to
>> its .text segment.
>
> ptrace cares about VM_MAYWRITE which PROT_FINAL can take away (under
> PaX MPROTECT'ed processes cannot be debugged with sw breakpoints).
>

My bad: I spent some time looking at access_process_vm() but could not
find any references to the vma permissions.

>> Could we at least agree on the fundamental notion that the special
>> powers the loader has to modify .text and .rodata sections are hardly
>> ever needed by the programs themselves? In that sense, this is similar
>> to dropping root privileges when not required anymore, and that is
>> typically recognized as a sensible idea.
>
> the difference is that the uid is a process-wide attribute, whereas
> PROT_FINAL isn't, unlike PaX's MPROTECT. in other words, dropping root
> is irreversible but PROT_FINAL isn't, one just has to create an entirely
> new mapping to acquire the access rights that PROT_FINAL was supposed to
> prevent (again, all this modulo an LSM and policies).
>

There remains a fundamental difference between the ability to
manipulate existing mappings and creating entirely new ones.
Especially when trying to manipulate GOT/PLT or vtable entries or
tweak the behavior of a program in other subtle ways, mmap()'ing the
same file again is just not going to cut it.

Regards,
Ard.

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[PaX Team]

On 4 Oct 2012 at 15:56, Ard Biesheuvel wrote:

> 2012/10/4 PaX Team <pageexec@freemail.hu>:
> The main difference here is that it is much easier to doctor a set of
> stack frames that issues a mprotect(+PROT_EXEC) on the whole address
> space than it is to re-issue the exact same mmap() call (with
> MAP_FIXED this time, and the exact offset the kernel decided to load
> it this time around) that will put the same code/data in exactly the
> same place in memory (relocated and all) without blowing up your
> process.

sadly, this is not true at all, for multiple reasons:

1. not all archs pass (all) parameters on the stack (arm, amd64 come to
   mind), so the content of the stack at the time the bug is triggered
   is not too relevant, gadgets can be used to construct anything later.

2. the exploit payload doesn't need to be on the process/thread stack
   at all. in fact, the more reliable exploits have for long used some
   form of heap spraying and stack pivoting to increase the chances of
   success. see also

   https://www.corelan.be/index.php/2010/06/16/exploit-writing-tutorial-part-10-chaining-dep-with-rop-the-rubikstm-cube/

3. exploit techniques moved away from simple linear stack overwrites
   and shellcode execution over a decade ago, the relevant methods all
   rely on ret2libc/ROP style exploitation. this has been automated to
   the point that tools can analyze binaries to extract a gadget dictionary
   (think 'insn set') and feed it into a gadget compiler that will produce
   a working and reliable exploit automatically. suggested reading:

   http://users.ece.cmu.edu/~ejschwar/bib/schwartz_2011_rop-abstract.html

4. for your particular suggestions about mprotect vs. mmap: mprotect cannot
   be called with arbitrary parameters either, the start address must fall
   inside a mapping and the end address must fall within the process address
   space limit and the protection bits can't have arbitrary bits set either.

   regarding reissuing the exact same mmap call: i just mentioned this to
   show that the PROT_FINAL proposal is circumventible (easily so, if we
   assume the attacker you mention in the changelog), obviously in a real
   life exploit scenario we wouldn't care about this, the attacker would
   simply create an rwx mapping and copy the shellcode there and execute
   it from there. and if for some reason rwx mappings are prevented otherwise,
   an rw mapping will do perfectly fine for a gadget based exploit (to the
   point that iOS jailbreaks implement *kernel* exploits with gadgets).

> In our specific implementation, it is mainly about rodata
> (public keys) rather than executable code, but the same applies; for
> us, it is more about rigging binaries: the attacker may not be
> interested in hijacking the whole process, but just nop'ing out some
> code that makes the process behave more to his liking.

and how do you prevent an attack from overmapping that rodata map? is there
something in your LSM that 'cements' specific mappings into the address
space (and some mechanism that does the same to all writable pointers
that refer to said mappings)?

also what about mprotect(PROT_NONE), i.e., instead of trying to acquire
more rights, degrade them? is that allowed? does your app handle this
properly? (these are side questions, just trying to poke holes ;)

> > i'm guessing again that their LSM tries to tackle the above described
> > scenarios except we don't know if this LSM will ever become public,
> > whether/how other LSMs will acquire the same capabilities and why it's
> > not mentioned in the PROT_FINAL submission that by itself this feature
> > doesn't increase security. i'm also wondering what kind of policy can
> > allow ld.so to load a library but forbid it a second time, it doesn't
> > seem compatible with real-life cases (think dynamically loaded and
> > unloaded plugins), not to mention the control of anonymous mappings.
> >
> 
> The LSM encapsulates the policy, and relies on the PROT_FINAL feature.
> The fact that the policy can impose the use of PROT_FINAL in
> particular cases[...]

i don't understand something here: if your kernel can detect when to use
PROT_FINAL, then why do you need it at all? why doesn't the kernel at
that point simply enforce the behaviour of PROT_FINAL itself (not unlike
PaX/MPROTECT does with runtime codegen)? in other words, why do you need
userland to tell the kernel about PROT_FINAL when your kernel can already
detect its need (dictated by the policy you mentioned above).

> [...]makes the implementation of the policy potentially
> much simpler than that of PaX MPROTECT (but not necessarily as
> secure).

the 'policy' to manage PaX/MPROTECT is very simple. you enable it in the
kernel, and you get MPROTECT enabled on all apps by default. if you want
to disable it on an app, you set it in the binary or in grsec's RBAC policy
(a single line per subject), i don't think you can get it done any simpler
than that. but it'd certainly help to see more details about how you intend
to use this and equally importantly, how the other LSMs could make use of
it. and by 'use' i mean 'increase security of the system'.

> However, let's decide first whether there is a point to having some
> control over the VM_MAY* flags directly. If yes, then the patch makes
> sense, otherwise it doesn't. How policies may be built on top of that
> is part of another debate.

there's certainly a point (i've been doing it for 12 years now), but to
make an mprotect flag into an actual security feature, it had better pass
simple tests, such as non-circumventability. any method relying on userland
playing nice is already suspect of being the wrong way and right now i don't
see how PROT_FINAL could be used for actual security.

> > ptrace cares about VM_MAYWRITE which PROT_FINAL can take away (under
> > PaX MPROTECT'ed processes cannot be debugged with sw breakpoints).
> >
> 
> My bad: I spent some time looking at access_process_vm() but could not
> find any references to the vma permissions.

look at get_user_pages and how the 'force' parameter is passed down/handled
later.

> >> Could we at least agree on the fundamental notion that the special
> >> powers the loader has to modify .text and .rodata sections are hardly
> >> ever needed by the programs themselves? In that sense, this is similar
> >> to dropping root privileges when not required anymore, and that is
> >> typically recognized as a sensible idea.
> >
> > the difference is that the uid is a process-wide attribute, whereas
> > PROT_FINAL isn't, unlike PaX's MPROTECT. in other words, dropping root
> > is irreversible but PROT_FINAL isn't, one just has to create an entirely
> > new mapping to acquire the access rights that PROT_FINAL was supposed to
> > prevent (again, all this modulo an LSM and policies).
> >
> 
> There remains a fundamental difference between the ability to
> manipulate existing mappings and creating entirely new ones.

what would be that fundamental difference? for an exploit writer it's the
same (even automated) effort.

> Especially when trying to manipulate GOT/PLT or vtable entries or
> tweak the behavior of a program in other subtle ways, mmap()'ing the
> same file again is just not going to cut it.

don't get too hooked up on mapping the same file, i just mentioned it
to prove that userland can circumvent the PROT_FINAL feature as designed.
a real exploit would of course do something different after the initial
control flow hijacking. as a sidenote, the GOT is already read-only these
days under RELRO and BIND_NOW.

cheers,
  PaX Team

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

2012/10/6 PaX Team <pageexec@freemail.hu>:
> sadly, this is not true at all, for multiple reasons:
>
.. snip ...
>
> cheers,
>   PaX Team
>

So can I summarize your position as that there is no merit at all in
the ability to inhibit future permissions of existing mappings?

-- 
Ard.

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[PaX Team]

On 7 Oct 2012 at 9:43, Ard Biesheuvel wrote:

> 2012/10/6 PaX Team <pageexec@freemail.hu>:
> > sadly, this is not true at all, for multiple reasons:
> >
> .. snip ...
> >
> > cheers,
> >   PaX Team
> >
> 
> So can I summarize your position as that there is no merit at all in
> the ability to inhibit future permissions of existing mappings?

i believe i answered this in the previous mail already:

> there's certainly a point (i've been doing it for 12 years now), but to
> make an mprotect flag into an actual security feature, it had better pass
> simple tests, such as non-circumventability. any method relying on
> userland playing nice is already suspect of being the wrong way and right
> now i don't see how PROT_FINAL could be used for actual security.

so if PROT_FINAL wants to be useful, you'd have to present a case of
how it does something useful *while* an exploited userland cannot get
around it. in fact i think i already told you that presenting your own
use case in more detail (read: source code, policy, etc) would be a
great step in 'selling the idea'. the fact that you seem to be reluctant
to follow up on this leaves me somewhat uneasy as it may be the sign
of a proprietary vendor's trying to push some code into mainline that
nobody else has a clear idea how it'd benefit the rest of us. you see,
you're asking for a change in a system call, which is a very important
boundary for kernel developers as they'll have to maintain it indefinitely.
so the burden is on you to prove that either this is the only way to
implement a useful feature or at least it is the optimal way as opposed
to other approaches. i suggested you ways to both attack the initially
presented concept and also how it may be improved, but i got no answers
to them yet.

cheers,
  PaX Team

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[halfdog]

PaX Team wrote:
> On 7 Oct 2012 at 9:43, Ard Biesheuvel wrote:
> 
>> 2012/10/6 PaX Team <pageexec@freemail.hu>:
>>> sadly, this is not true at all, for multiple reasons:
>>>
>> .. snip ...
>>>
>>> cheers,
>>>   PaX Team
>>>
>>
>> So can I summarize your position as that there is no merit at all in
>> the ability to inhibit future permissions of existing mappings?
> 
> i believe i answered this in the previous mail already:
> 
>> there's certainly a point (i've been doing it for 12 years now), but to
>> make an mprotect flag into an actual security feature, it had better pass
>> simple tests, such as non-circumventability. any method relying on
>> userland playing nice is already suspect of being the wrong way and right
>> now i don't see how PROT_FINAL could be used for actual security.
> 
> so if PROT_FINAL wants to be useful, you'd have to present a case of
> how it does something useful *while* an exploited userland cannot get
> around it. in fact i think i already told you that presenting your own
> use case in more detail (read: source code, policy, etc) would be a
> great step in 'selling the idea'.

I like the idea of final memory protection, but I guess it is quite
tricky to make it non-circumventable for reading or non-modification. To
block code execution, this feature makes it harder but does not prevent
anyway: if you can execute already (e.g. ROP), one still has ways to
exec more of anything, e.g. load more stack data and stay ROPed, map new
segments, write to file and map it r-x or exec the new file, .... but
per-application policies to prevent that could be simpler than without
PROT_FINAL.

>From my point of view, when protecting against reading/modifiction, it
would make only sense when current vm and all clones stay protected,
e.g. against proc/$$/mem-reading, ptrace attaching of process to self or
clones, not core-dumpable. Otherwise, except for the latest issue, it
should be possible, that the process forks, parent modify child via
ptrace or proc/mem, then parent just waits or commits suicide. If the
content in memory or modification of running process is that important
for success of attack, efforts might be taken to do that.

But if PROT_FINAL could be made that solid, it might be quite
interesting, especially with some proc-fs settings like
final-modification-action: ignore (do not check final, e.g. for
debugging), log (log and fail), kill (get rid of process immediately).
With kernel-wide default and e.g. uid-0 modification of policy per
process, that would still allow all debugging also.

-- 
http://www.halfdog.net/
PGP: 156A AE98 B91F 0114 FE88  2BD8 C459 9386 feed a bee

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Mikael Pettersson]

Ard Biesheuvel writes:
 > This patch adds support for the PROT_FINAL flag to
 > the mmap() and mprotect() syscalls.
 > 
 > The PROT_FINAL flag indicates that the requested set
 > of protection bits should be final, i.e., it shall
 > not be allowed for a subsequent mprotect call to
 > set protection bits that were not set already.
 > 
 > This is mainly intended for the dynamic linker,
 > which sets up the address space on behalf of
 > dynamic binaries. By using this flag, it can
 > prevent exploited code from remapping read-only
 > executable code or data sections read-write.

I can see why you might think this is a good idea, but I don't
like it for several reasons:

- If .text is mapped non-writable and final, how would a debugger
  (or any ptrace-using monitor-like application) plant a large
  number of breakpoints in a target process? Breakpoint registers
  aren't enough because (a) they're few in number, and (b) not
  all CPUs have them.

- You're proposing to give one component (the dynamic linker/
  loader) absolute power to impose new policies on all
  applications. How would an application that _deliberately_
  does something the new policies don't allow tell the dynamic
  linker or kernel to get out of its way?

This clearly changes the de-facto ABIs, and as such I think
it needs much more detailed analysis than what you've done
here.

At the very least I think this change should be opt-in, but
that would require a kernel option or sysctl, or some config
file for the user-space dynamic linker/loader.

Re: Updated: [PATCH] hardening: add PROT_FINAL prot flag to mmap/mprotect

[Ard Biesheuvel]

2012/10/4 Mikael Pettersson <mikpe@it.uu.se>:
> - If .text is mapped non-writable and final, how would a debugger
>   (or any ptrace-using monitor-like application) plant a large
>   number of breakpoints in a target process? Breakpoint registers
>   aren't enough because (a) they're few in number, and (b) not
>   all CPUs have them.
>

ptrace() doesn't care whether or not the process itself can write to
its .text segment.

> - You're proposing to give one component (the dynamic linker/
>   loader) absolute power to impose new policies on all
>   applications. How would an application that _deliberately_
>   does something the new policies don't allow tell the dynamic
>   linker or kernel to get out of its way?
>

You are debating cases in which the userland would choose not to use
the feature. That is exactly the point of this patch: the kernel
supplies the feature and it is up to the userland to use it when
desired. If not in the loader, perhaps processes running setuid
binaries or browser sandboxes could choose to call mprotect() to
finalize some of their existing mappings (their stack?) before handing
over to less trusted code or opening up to the network.

> This clearly changes the de-facto ABIs, and as such I think
> it needs much more detailed analysis than what you've done
> here.
>

Could we at least agree on the fundamental notion that the special
powers the loader has to modify .text and .rodata sections are hardly
ever needed by the programs themselves? In that sense, this is similar
to dropping root privileges when not required anymore, and that is
typically recognized as a sensible idea.

> At the very least I think this change should be opt-in, but
> that would require a kernel option or sysctl, or some config
> file for the user-space dynamic linker/loader.

As long as the kernel does not impose its use, I don't see a reason
for an interface into the kernel to deactivate it.
I would be interested in learning about example cases that have a
valid need to modify their own code and constant data, as
understanding those would greatly help in designing the ways userland
should be able to have control over this.

Regards,
Ard.