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

Kees Cook
2020-04-06T19:16:20Z

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