kernel-hardening.lists.openwall.com archive mirror
 help / color / mirror / Atom feed
* [RFC PATCH 0/5] Prevent write to read-only pages (text, PLT/GOT
@ 2020-04-06 14:20 Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process Lev Olshvang
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Lev Olshvang @ 2020-04-06 14:20 UTC (permalink / raw)
  To: arnd; +Cc: kernel-hardening, Lev Olshvang

The purpose of this patch is produce hardened kernel for Embedded
or Production systems.

Typically debuggers, such as gdb, write to read-only code [text]
sections of target process.(ptrace)
This kind of page protectiion violation raises minor page fault, but
kernel's fault handler allows it by default.
This is clearly attack surface for adversary.

The proposed kernel hardening configuration option checks the type of
protection of the foreign vma and blocks writes to read only vma.

When enabled, it will stop attacks modifying code or jump tables, etc.

Lev Olshvang (5):
  security : hardening : prevent write to proces's read-only pages from
    another process
  Prevent write to read-only pages (text, PLT/GOT tables from another
    process
  Prevent write to read-only pages (text, PLT/GOT tables from another
    process
  X86:Prevent write to read-only pages (text, PLT/GOT tables from
    another process
  UM:Prevent write to read-only pages (text, PLT/GOT tables from another
    process

 arch/powerpc/include/asm/mmu_context.h   |  7 ++++++-
 arch/powerpc/mm/book3s64/pkeys.c         |  5 +++++
 arch/um/include/asm/mmu_context.h        | 11 ++++++++---
 arch/unicore32/include/asm/mmu_context.h |  7 ++++++-
 arch/x86/include/asm/mmu_context.h       |  9 ++++++++-
 include/asm-generic/mm_hooks.h           |  5 +++++
 security/Kconfig                         | 10 ++++++++++
 7 files changed, 48 insertions(+), 6 deletions(-)

--
2.17.1


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

* [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process
  2020-04-06 14:20 [RFC PATCH 0/5] Prevent write to read-only pages (text, PLT/GOT Lev Olshvang
@ 2020-04-06 14:20 ` Lev Olshvang
  2020-04-06 19:15   ` Kees Cook
  2020-04-06 14:20 ` [RFC PATCH 2/5] Prevent write to " Lev Olshvang
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Lev Olshvang @ 2020-04-06 14:20 UTC (permalink / raw)
  To: arnd; +Cc: kernel-hardening, Lev Olshvang

The purpose of this patch is produce hardened kernel for Embedded
or Production systems.

Typically debuggers, such as gdb, write to read-only code [text]
sections of target process.(ptrace)
This kind of page protectiion violation raises minor page fault, but
kernel's fault handler allows it by default.
This is clearly attack surface for adversary.

The proposed kernel hardening configuration option checks the type of
protection of the foreign vma and blocks writes to read only vma.

When enabled, it will stop attacks modifying code or jump tables, etc.

Code of arch_vma_access_permitted() function was extended to
check foreign vma flags.

Tested on x86_64 and ARM(QEMU) with dd command which writes to
/proc/PID/mem in r--p or r--xp of vma area addresses range

dd reports IO failure when tries to write to adress taken from
from /proc/PID/maps (PLT or code section)

Signed-off-by: Lev Olshvang <levonshe@gmail.com>
---
 include/asm-generic/mm_hooks.h |  5 +++++
 security/Kconfig               | 10 ++++++++++
 2 files changed, 15 insertions(+)

diff --git a/include/asm-generic/mm_hooks.h b/include/asm-generic/mm_hooks.h
index 4dbb177d1150..6e1fcce44cc2 100644
--- a/include/asm-generic/mm_hooks.h
+++ b/include/asm-generic/mm_hooks.h
@@ -25,6 +25,11 @@ static inline void arch_unmap(struct mm_struct *mm,
 static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 		bool write, bool execute, bool foreign)
 {
+#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
+	/* Forbid write to PROT_READ pages of foreign process */
+	if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
+		return false;
+#endif
 	/* by default, allow everything */
 	return true;
 }
diff --git a/security/Kconfig b/security/Kconfig
index cd3cc7da3a55..d92e79c90d67 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -143,6 +143,16 @@ config LSM_MMAP_MIN_ADDR
 	  this low address space will need the permission specific to the
 	  systems running LSM.
 
+config PROTECT_READONLY_USER_MEMORY
+	bool "Protect read only process memory"
+	help
+	  Protects read only memory of process code and PLT table
+	  from possible attack through /proc/PID/mem or through /dev/mem.
+	  Refuses to insert and stop at debuggers breakpoints (prtace,gdb)
+	  Mostly advised for embedded and production system.
+	  Stops attempts of the malicious process to modify read only memory of another process
+
+
 config HAVE_HARDENED_USERCOPY_ALLOCATOR
 	bool
 	help
-- 
2.17.1


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

* [RFC PATCH 2/5] Prevent write to read-only pages from another process
  2020-04-06 14:20 [RFC PATCH 0/5] Prevent write to read-only pages (text, PLT/GOT Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process Lev Olshvang
@ 2020-04-06 14:20 ` Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 3/5] Prevent write to read-only pages text, PLT/GOT tables " Lev Olshvang
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Lev Olshvang @ 2020-04-06 14:20 UTC (permalink / raw)
  To: arnd; +Cc: kernel-hardening, Lev Olshvang

Signed-off-by: Lev Olshvang <levonshe@gmail.com>
---
 arch/unicore32/include/asm/mmu_context.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/unicore32/include/asm/mmu_context.h b/arch/unicore32/include/asm/mmu_context.h
index 388c0c811c68..caf240b8a748 100644
--- a/arch/unicore32/include/asm/mmu_context.h
+++ b/arch/unicore32/include/asm/mmu_context.h
@@ -92,7 +92,12 @@ static inline void arch_unmap(struct mm_struct *mm,
 static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 		bool write, bool execute, bool foreign)
 {
+#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
+	/* Forbid write to PROT_READ pages of foreign process */
+	if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
+		return false;
+#endif
 	/* by default, allow everything */
 	return true;
 }
-#endif
+#endif /*__UNICORE_MMU_CONTEXT_H__*/
--
2.17.1


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

* [RFC PATCH 3/5] Prevent write to read-only pages text, PLT/GOT tables from another process
  2020-04-06 14:20 [RFC PATCH 0/5] Prevent write to read-only pages (text, PLT/GOT Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 2/5] Prevent write to " Lev Olshvang
@ 2020-04-06 14:20 ` Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 4/5] X86:Prevent write to read-only pages :text, " Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 5/5] UM:Prevent " Lev Olshvang
  4 siblings, 0 replies; 9+ messages in thread
From: Lev Olshvang @ 2020-04-06 14:20 UTC (permalink / raw)
  To: arnd; +Cc: kernel-hardening, Lev Olshvang

Signed-off-by: Lev Olshvang <levonshe@gmail.com>
---
 arch/powerpc/include/asm/mmu_context.h | 7 ++++++-
 arch/powerpc/mm/book3s64/pkeys.c       | 5 +++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h
index 360367c579de..b25e5726fa99 100644
--- a/arch/powerpc/include/asm/mmu_context.h
+++ b/arch/powerpc/include/asm/mmu_context.h
@@ -246,10 +246,15 @@ void arch_dup_pkeys(struct mm_struct *oldmm, struct mm_struct *mm);
 static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 		bool write, bool execute, bool foreign)
 {
+#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
+	/* Forbid write to PROT_READ pages of foreign process */
+	if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
+		return false;
+#endif
 	/* by default, allow everything */
 	return true;
 }
-
+#endif
 #define pkey_mm_init(mm)
 #define thread_pkey_regs_save(thread)
 #define thread_pkey_regs_restore(new_thread, old_thread)
diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c
index 07527f1ed108..230058a52009 100644
--- a/arch/powerpc/mm/book3s64/pkeys.c
+++ b/arch/powerpc/mm/book3s64/pkeys.c
@@ -384,6 +384,11 @@ bool arch_pte_access_permitted(u64 pte, bool write, bool execute)
 bool arch_vma_access_permitted(struct vm_area_struct *vma, bool write,
 			       bool execute, bool foreign)
 {
+#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
+	/* Forbid write to PROT_READ pages of foreign process */
+	if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
+		return false;
+#endif
 	if (static_branch_likely(&pkey_disabled))
 		return true;
 	/*
--
2.17.1


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

* [RFC PATCH 4/5] X86:Prevent write to read-only pages :text, PLT/GOT tables from another process
  2020-04-06 14:20 [RFC PATCH 0/5] Prevent write to read-only pages (text, PLT/GOT Lev Olshvang
                   ` (2 preceding siblings ...)
  2020-04-06 14:20 ` [RFC PATCH 3/5] Prevent write to read-only pages text, PLT/GOT tables " Lev Olshvang
@ 2020-04-06 14:20 ` Lev Olshvang
  2020-04-06 14:20 ` [RFC PATCH 5/5] UM:Prevent " Lev Olshvang
  4 siblings, 0 replies; 9+ messages in thread
From: Lev Olshvang @ 2020-04-06 14:20 UTC (permalink / raw)
  To: arnd; +Cc: kernel-hardening, Lev Olshvang

Signed-off-by: Lev Olshvang <levonshe@gmail.com>
---
 arch/x86/include/asm/mmu_context.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h
index 4e55370e48e8..708135112d95 100644
--- a/arch/x86/include/asm/mmu_context.h
+++ b/arch/x86/include/asm/mmu_context.h
@@ -216,12 +216,19 @@ static inline void arch_unmap(struct mm_struct *mm, unsigned long start,
 static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 		bool write, bool execute, bool foreign)
 {
-	/* pkeys never affect instruction fetches */
+#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
+	/* Forbid write to PROT_READ pages of foreign process */
+	if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
+		return false;
+#endif
+	/* Don't check PKRU since pkeys never affect instruction fetches */
 	if (execute)
 		return true;
+
 	/* allow access if the VMA is not one from this process */
 	if (foreign || vma_is_foreign(vma))
 		return true;
+
 	return __pkru_allows_pkey(vma_pkey(vma), write);
 }

--
2.17.1


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

* [RFC PATCH 5/5] UM:Prevent write to read-only pages :text, PLT/GOT tables from another process
  2020-04-06 14:20 [RFC PATCH 0/5] Prevent write to read-only pages (text, PLT/GOT Lev Olshvang
                   ` (3 preceding siblings ...)
  2020-04-06 14:20 ` [RFC PATCH 4/5] X86:Prevent write to read-only pages :text, " Lev Olshvang
@ 2020-04-06 14:20 ` Lev Olshvang
  4 siblings, 0 replies; 9+ messages in thread
From: Lev Olshvang @ 2020-04-06 14:20 UTC (permalink / raw)
  To: arnd; +Cc: kernel-hardening, Lev Olshvang

Signed-off-by: Lev Olshvang <levonshe@gmail.com>
---
 arch/um/include/asm/mmu_context.h | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/arch/um/include/asm/mmu_context.h b/arch/um/include/asm/mmu_context.h
index b4deb1bfbb68..2de21d52bd60 100644
--- a/arch/um/include/asm/mmu_context.h
+++ b/arch/um/include/asm/mmu_context.h
@@ -1,5 +1,5 @@
 /* SPDX-License-Identifier: GPL-2.0 */
-/*
+/*
  * Copyright (C) 2002 - 2007 Jeff Dike (jdike@{addtoit,linux.intel}.com)
  */

@@ -28,6 +28,11 @@ static inline void arch_unmap(struct mm_struct *mm,
 static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
 		bool write, bool execute, bool foreign)
 {
+	#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
+	/* Forbid write to PROT_READ pages of foreign process */
+	if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
+	return false;
+	#endif
 	/* by default, allow everything */
 	return true;
 }
@@ -52,7 +57,7 @@ static inline void activate_mm(struct mm_struct *old, struct mm_struct *new)
 	up_write(&new->mmap_sem);
 }

-static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
+static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 			     struct task_struct *tsk)
 {
 	unsigned cpu = smp_processor_id();
@@ -65,7 +70,7 @@ static inline void switch_mm(struct mm_struct *prev, struct mm_struct *next,
 	}
 }

-static inline void enter_lazy_tlb(struct mm_struct *mm,
+static inline void enter_lazy_tlb(struct mm_struct *mm,
 				  struct task_struct *tsk)
 {
 }
--
2.17.1


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

* Re: [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process
  2020-04-06 14:20 ` [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process Lev Olshvang
@ 2020-04-06 19:15   ` Kees Cook
  2020-04-07 10:16     ` Lev R. Oshvang .
  0 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2020-04-06 19:15 UTC (permalink / raw)
  To: Lev Olshvang; +Cc: arnd, kernel-hardening, Jann Horn

On Mon, Apr 06, 2020 at 05:20:41PM +0300, Lev Olshvang wrote:
> The purpose of this patch is produce hardened kernel for Embedded
> or Production systems.
> 
> Typically debuggers, such as gdb, write to read-only code [text]
> sections of target process.(ptrace)
> This kind of page protectiion violation raises minor page fault, but
> kernel's fault handler allows it by default.
> This is clearly attack surface for adversary.
> 
> The proposed kernel hardening configuration option checks the type of
> protection of the foreign vma and blocks writes to read only vma.
> 
> When enabled, it will stop attacks modifying code or jump tables, etc.
> 
> Code of arch_vma_access_permitted() function was extended to
> check foreign vma flags.
> 
> Tested on x86_64 and ARM(QEMU) with dd command which writes to
> /proc/PID/mem in r--p or r--xp of vma area addresses range
> 
> dd reports IO failure when tries to write to adress taken from
> from /proc/PID/maps (PLT or code section)

So, just to give some background here: the reason for this behavior is
so debuggers can insert software breakpoints in the .text section (0xcc)
etc. This is implemented with the "FOLL_FORCE" flag, and an attempt to
remove it was made here:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8ee74a91ac30
but it was later reverted (see below).

There have been many prior discussions about this behavior, and a
good thread (which I link from https://github.com/KSPP/linux/issues/37
"Block process from writing to its own /proc/$pid/mem") is this one:
https://lore.kernel.org/lkml/CAGXu5j+PHzDwnJxJwMJ=WuhacDn_vJWe9xZx+Kbsh28vxOGRiA@mail.gmail.com/

For details on the revert see:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f511c0b17b08

All this said, I think this feature would still be nice to have,
available with some kind of knob to control it. Do you get the
results you were expecting from just re-applying 8ee74a91ac30? If
so, that's a much smaller change, and a single place to apply
a knob. It would likely be best implemented with a sysctl and a
static_branch(). A possible example for this can be seen here:
https://lore.kernel.org/lkml/20200324203231.64324-4-keescook@chromium.org/
Though it doesn't use a sysctl. (And perhaps this feature needs to be a
per-process setting like "dumpable", but let's start simple with a
system-wide control.)

Can you test the FOLL_FORCE removal and refactor things to use a
static_branch() instead?

-Kees

> Signed-off-by: Lev Olshvang <levonshe@gmail.com>
> ---
>  include/asm-generic/mm_hooks.h |  5 +++++
>  security/Kconfig               | 10 ++++++++++
>  2 files changed, 15 insertions(+)
> 
> diff --git a/include/asm-generic/mm_hooks.h b/include/asm-generic/mm_hooks.h
> index 4dbb177d1150..6e1fcce44cc2 100644
> --- a/include/asm-generic/mm_hooks.h
> +++ b/include/asm-generic/mm_hooks.h
> @@ -25,6 +25,11 @@ static inline void arch_unmap(struct mm_struct *mm,
>  static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
>  		bool write, bool execute, bool foreign)
>  {
> +#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
> +	/* Forbid write to PROT_READ pages of foreign process */
> +	if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
> +		return false;
> +#endif
>  	/* by default, allow everything */
>  	return true;
>  }
> diff --git a/security/Kconfig b/security/Kconfig
> index cd3cc7da3a55..d92e79c90d67 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -143,6 +143,16 @@ config LSM_MMAP_MIN_ADDR
>  	  this low address space will need the permission specific to the
>  	  systems running LSM.
>  
> +config PROTECT_READONLY_USER_MEMORY
> +	bool "Protect read only process memory"
> +	help
> +	  Protects read only memory of process code and PLT table
> +	  from possible attack through /proc/PID/mem or through /dev/mem.
> +	  Refuses to insert and stop at debuggers breakpoints (prtace,gdb)
> +	  Mostly advised for embedded and production system.
> +	  Stops attempts of the malicious process to modify read only memory of another process
> +
> +
>  config HAVE_HARDENED_USERCOPY_ALLOCATOR
>  	bool
>  	help
> -- 
> 2.17.1
> 

-- 
Kees Cook

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

* Re: [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process
  2020-04-06 19:15   ` Kees Cook
@ 2020-04-07 10:16     ` Lev R. Oshvang .
  2020-04-07 16:25       ` Kees Cook
  0 siblings, 1 reply; 9+ messages in thread
From: Lev R. Oshvang . @ 2020-04-07 10:16 UTC (permalink / raw)
  To: Kees Cook; +Cc: arnd, kernel-hardening, Jann Horn

Hi Kees,

The patch you referred to is scoped in /proc fs.

There is a chance that hackers may use other attack methods except procfs.
There is process_vm_writev syscall , /dev/mem.
An attacker can also hijack one of the process threads and write to
read-only pages.
I admit that I am the newbie and lack knowledge, but I think my
solution is more generic and protects from more possible attacks.
Second, you suggested to control it in run-time with a knob.
I think that configuration option I propose better fit embedded system needs.
There is no need in an embedded system to turn it on/off since there is no gdb.
(the same argument for a production system),  These systems are locked
down, and perhaps the proper place to put this option is in lockdown
LSM.
Thank you for your response.
Lev


On Mon, Apr 6, 2020 at 10:16 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Apr 06, 2020 at 05:20:41PM +0300, Lev Olshvang wrote:
> > The purpose of this patch is produce hardened kernel for Embedded
> > or Production systems.
> >
> > Typically debuggers, such as gdb, write to read-only code [text]
> > sections of target process.(ptrace)
> > This kind of page protectiion violation raises minor page fault, but
> > kernel's fault handler allows it by default.
> > This is clearly attack surface for adversary.
> >
> > The proposed kernel hardening configuration option checks the type of
> > protection of the foreign vma and blocks writes to read only vma.
> >
> > When enabled, it will stop attacks modifying code or jump tables, etc.
> >
> > Code of arch_vma_access_permitted() function was extended to
> > check foreign vma flags.
> >
> > Tested on x86_64 and ARM(QEMU) with dd command which writes to
> > /proc/PID/mem in r--p or r--xp of vma area addresses range
> >
> > dd reports IO failure when tries to write to adress taken from
> > from /proc/PID/maps (PLT or code section)
>
> So, just to give some background here: the reason for this behavior is
> so debuggers can insert software breakpoints in the .text section (0xcc)
> etc. This is implemented with the "FOLL_FORCE" flag, and an attempt to
> remove it was made here:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8ee74a91ac30
> but it was later reverted (see below).
>
> There have been many prior discussions about this behavior, and a
> good thread (which I link from https://github.com/KSPP/linux/issues/37
> "Block process from writing to its own /proc/$pid/mem") is this one:
> https://lore.kernel.org/lkml/CAGXu5j+PHzDwnJxJwMJ=WuhacDn_vJWe9xZx+Kbsh28vxOGRiA@mail.gmail.com/
>
> For details on the revert see:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f511c0b17b08
>
> All this said, I think this feature would still be nice to have,
> available with some kind of knob to control it. Do you get the
> results you were expecting from just re-applying 8ee74a91ac30? If
> so, that's a much smaller change, and a single place to apply
> a knob. It would likely be best implemented with a sysctl and a
> static_branch(). A possible example for this can be seen here:
> https://lore.kernel.org/lkml/20200324203231.64324-4-keescook@chromium.org/
> Though it doesn't use a sysctl. (And perhaps this feature needs to be a
> per-process setting like "dumpable", but let's start simple with a
> system-wide control.)
>
> Can you test the FOLL_FORCE removal and refactor things to use a
> static_branch() instead?
>
> -Kees
>
> > Signed-off-by: Lev Olshvang <levonshe@gmail.com>
> > ---
> >  include/asm-generic/mm_hooks.h |  5 +++++
> >  security/Kconfig               | 10 ++++++++++
> >  2 files changed, 15 insertions(+)
> >
> > diff --git a/include/asm-generic/mm_hooks.h b/include/asm-generic/mm_hooks.h
> > index 4dbb177d1150..6e1fcce44cc2 100644
> > --- a/include/asm-generic/mm_hooks.h
> > +++ b/include/asm-generic/mm_hooks.h
> > @@ -25,6 +25,11 @@ static inline void arch_unmap(struct mm_struct *mm,
> >  static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> >               bool write, bool execute, bool foreign)
> >  {
> > +#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
> > +     /* Forbid write to PROT_READ pages of foreign process */
> > +     if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
> > +             return false;
> > +#endif
> >       /* by default, allow everything */
> >       return true;
> >  }
> > diff --git a/security/Kconfig b/security/Kconfig
> > index cd3cc7da3a55..d92e79c90d67 100644
> > --- a/security/Kconfig
> > +++ b/security/Kconfig
> > @@ -143,6 +143,16 @@ config LSM_MMAP_MIN_ADDR
> >         this low address space will need the permission specific to the
> >         systems running LSM.
> >
> > +config PROTECT_READONLY_USER_MEMORY
> > +     bool "Protect read only process memory"
> > +     help
> > +       Protects read only memory of process code and PLT table
> > +       from possible attack through /proc/PID/mem or through /dev/mem.
> > +       Refuses to insert and stop at debuggers breakpoints (prtace,gdb)
> > +       Mostly advised for embedded and production system.
> > +       Stops attempts of the malicious process to modify read only memory of another process
> > +
> > +
> >  config HAVE_HARDENED_USERCOPY_ALLOCATOR
> >       bool
> >       help
> > --
> > 2.17.1
> >
>
> --
> Kees Cook

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

* Re: [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process
  2020-04-07 10:16     ` Lev R. Oshvang .
@ 2020-04-07 16:25       ` Kees Cook
  0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2020-04-07 16:25 UTC (permalink / raw)
  To: Lev R. Oshvang .; +Cc: arnd, kernel-hardening, Jann Horn

(Please avoid top-posting
https://www.kernel.org/doc/html/latest/process/2.Process.html#mailing-lists)

On Tue, Apr 07, 2020 at 01:16:00PM +0300, Lev R. Oshvang . wrote:
> 
> 
> On Mon, Apr 6, 2020 at 10:16 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > On Mon, Apr 06, 2020 at 05:20:41PM +0300, Lev Olshvang wrote:
> > > The purpose of this patch is produce hardened kernel for Embedded
> > > or Production systems.
> > >
> > > Typically debuggers, such as gdb, write to read-only code [text]
> > > sections of target process.(ptrace)
> > > This kind of page protectiion violation raises minor page fault, but
> > > kernel's fault handler allows it by default.
> > > This is clearly attack surface for adversary.
> > >
> > > The proposed kernel hardening configuration option checks the type of
> > > protection of the foreign vma and blocks writes to read only vma.
> > >
> > > When enabled, it will stop attacks modifying code or jump tables, etc.
> > >
> > > Code of arch_vma_access_permitted() function was extended to
> > > check foreign vma flags.
> > >
> > > Tested on x86_64 and ARM(QEMU) with dd command which writes to
> > > /proc/PID/mem in r--p or r--xp of vma area addresses range
> > >
> > > dd reports IO failure when tries to write to adress taken from
> > > from /proc/PID/maps (PLT or code section)
> >
> > So, just to give some background here: the reason for this behavior is
> > so debuggers can insert software breakpoints in the .text section (0xcc)
> > etc. This is implemented with the "FOLL_FORCE" flag, and an attempt to
> > remove it was made here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=8ee74a91ac30
> > but it was later reverted (see below).
> >
> > There have been many prior discussions about this behavior, and a
> > good thread (which I link from https://github.com/KSPP/linux/issues/37
> > "Block process from writing to its own /proc/$pid/mem") is this one:
> > https://lore.kernel.org/lkml/CAGXu5j+PHzDwnJxJwMJ=WuhacDn_vJWe9xZx+Kbsh28vxOGRiA@mail.gmail.com/
> >
> > For details on the revert see:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f511c0b17b08
> >
> > All this said, I think this feature would still be nice to have,
> > available with some kind of knob to control it. Do you get the
> > results you were expecting from just re-applying 8ee74a91ac30? If
> > so, that's a much smaller change, and a single place to apply
> > a knob. It would likely be best implemented with a sysctl and a
> > static_branch(). A possible example for this can be seen here:
> > https://lore.kernel.org/lkml/20200324203231.64324-4-keescook@chromium.org/
> > Though it doesn't use a sysctl. (And perhaps this feature needs to be a
> > per-process setting like "dumpable", but let's start simple with a
> > system-wide control.)
> >
> > Can you test the FOLL_FORCE removal and refactor things to use a
> > static_branch() instead?
>
> Hi Kees,
> 
> The patch you referred to is scoped in /proc fs.

Ah yes, good point. Is FOLL_FORCE used in the other places as well? I'd
really like to find way to make this change architecture-agnostic.

> There is a chance that hackers may use other attack methods except procfs.
> There is process_vm_writev syscall , /dev/mem.
> An attacker can also hijack one of the process threads and write to
> read-only pages.
> I admit that I am the newbie and lack knowledge, but I think my
> solution is more generic and protects from more possible attacks.
> Second, you suggested to control it in run-time with a knob.
> I think that configuration option I propose better fit embedded system needs.
> There is no need in an embedded system to turn it on/off since there is no gdb.
> (the same argument for a production system),  These systems are locked
> down, and perhaps the proper place to put this option is in lockdown
> LSM.

This is mainly about upstream acceptability: using CONFIGs for these
kinds of things is frowned upon because it means that distro kernel
users can't use the feature (since distros won't enable the CONFIG since
it absolutely breaks known userspace workflows). So a static branch
(with its default controlled by CONFIG) means that everyone can use it,
embedded or not.

> Thank you for your response.

Thanks for chasing this problem down! If you comment on the bug, I can
mark it as assigned to you, if want?
https://github.com/KSPP/linux/issues/37

-Kees

> Lev
> >
> > -Kees
> >
> > > Signed-off-by: Lev Olshvang <levonshe@gmail.com>
> > > ---
> > >  include/asm-generic/mm_hooks.h |  5 +++++
> > >  security/Kconfig               | 10 ++++++++++
> > >  2 files changed, 15 insertions(+)
> > >
> > > diff --git a/include/asm-generic/mm_hooks.h b/include/asm-generic/mm_hooks.h
> > > index 4dbb177d1150..6e1fcce44cc2 100644
> > > --- a/include/asm-generic/mm_hooks.h
> > > +++ b/include/asm-generic/mm_hooks.h
> > > @@ -25,6 +25,11 @@ static inline void arch_unmap(struct mm_struct *mm,
> > >  static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
> > >               bool write, bool execute, bool foreign)
> > >  {
> > > +#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY
> > > +     /* Forbid write to PROT_READ pages of foreign process */
> > > +     if (write && foreign && (!(vma->vm_flags & VM_WRITE)))
> > > +             return false;
> > > +#endif
> > >       /* by default, allow everything */
> > >       return true;
> > >  }
> > > diff --git a/security/Kconfig b/security/Kconfig
> > > index cd3cc7da3a55..d92e79c90d67 100644
> > > --- a/security/Kconfig
> > > +++ b/security/Kconfig
> > > @@ -143,6 +143,16 @@ config LSM_MMAP_MIN_ADDR
> > >         this low address space will need the permission specific to the
> > >         systems running LSM.
> > >
> > > +config PROTECT_READONLY_USER_MEMORY
> > > +     bool "Protect read only process memory"
> > > +     help
> > > +       Protects read only memory of process code and PLT table
> > > +       from possible attack through /proc/PID/mem or through /dev/mem.
> > > +       Refuses to insert and stop at debuggers breakpoints (prtace,gdb)
> > > +       Mostly advised for embedded and production system.
> > > +       Stops attempts of the malicious process to modify read only memory of another process
> > > +
> > > +
> > >  config HAVE_HARDENED_USERCOPY_ALLOCATOR
> > >       bool
> > >       help
> > > --
> > > 2.17.1
> > >
> >
> > --
> > Kees Cook

-- 
Kees Cook

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

end of thread, other threads:[~2020-04-07 16:25 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-04-06 14:20 [RFC PATCH 0/5] Prevent write to read-only pages (text, PLT/GOT Lev Olshvang
2020-04-06 14:20 ` [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process Lev Olshvang
2020-04-06 19:15   ` Kees Cook
2020-04-07 10:16     ` Lev R. Oshvang .
2020-04-07 16:25       ` Kees Cook
2020-04-06 14:20 ` [RFC PATCH 2/5] Prevent write to " Lev Olshvang
2020-04-06 14:20 ` [RFC PATCH 3/5] Prevent write to read-only pages text, PLT/GOT tables " Lev Olshvang
2020-04-06 14:20 ` [RFC PATCH 4/5] X86:Prevent write to read-only pages :text, " Lev Olshvang
2020-04-06 14:20 ` [RFC PATCH 5/5] UM:Prevent " Lev Olshvang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).