Re: [RFC PATCH 1/5] security : hardening : prevent write to proces's read-only pages from another process

[Kees Cook <keescook@chromium.org>]

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