Re: [RFC] security hardening: block write to read_only pages of a target process.

[Jann Horn <jannh@google.com>]

On Fri, Aug 23, 2019 at 2:38 PM Lev Olshvang <levonshe@yandex.com> wrote:
> Target process is not a current process.
> It is a foreign process in the terminogy of page fault handler.
>
> Typically debuggers, such as gdb, write to read-only code [text]
> sections of target process.
> This patch introduce kernel hardening configuration option.
> When enabled, it will stop attacks modifying code or jump tables.

This patch is missing context. What, at a high level, is your goal
with this patch? My guess is that you're trying to close gaps in the
protection that SELinux provides with "execmod" rules, and that you
want to ensure that an attacker who is exploiting a memory corruption
bug still can't modify machine code in the process being exploited.

You should CC appropriate kernel developers and lists for the code
you're modifying; see Documentation/process/submitting-patches.rst,
section "5) Select the recipients for your patch".

> Onky Code of arch_vma_access_permitted() function was extended to
> check foreign vma vm_flags.
>
> New logic denies to accept page fault caused by page protection violation.
>
> Separatly applied for x86,powerpc and unicore32
> arch_vma_access_permitted() function is not referenced in unicore32 and um
> architectures and seems to be obsolete,IMHO.

I think you're putting your checks in the wrong place.
If you put security checks into architecture-specific parts of the
codebase, then someone who wants to modify those checks might only
change some of the copies; and then the security checks become
inconsistent across architectures.
But also, there is already a function parameter at a higher level that
controls this behavior: In fs/proc/base.c, the function mem_rw() calls
access_remote_vm() with the flag FOLL_FORCE, which controls whether
overwriting non-writable memory is allowed. If you want to prevent the
use of /proc/*/mem to overwrite non-writable memory, then you can just
make it configurable whether that flag is used in that function.


> diff --git a/security/Kconfig b/security/Kconfig
> index 0d65594..03ff948 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> +config PROTECT_READONLY_USER_MEMORY
> +       bool "protect read only process memory"
> +       depends on !(CONFIG_CROSS_MEMORY_ATTACH)

Why this "depends on" rule? CONFIG_CROSS_MEMORY_ATTACH only controls
the process_vm_readv() and process_vm_writev() syscalls, and those
can't modify read-only memory because process_vm_rw_single_vec()
doesn't set FOLL_FORCE.

Also, I think the CONFIG_ prefix shouldn't be used inside Kconfig.

> +       help
> +         Protects read only memory of process code and PLT table from possible attack
> +         through /proc/PID/mem.
> +         Forbid writes to READ ONLY user pages of foreign process
> +         Mostly advised for embedded and production system.
> +         Disables process_vm_writev() syscall used in MP computing.

You are introducing a kernel config flag, but no corresponding sysctl,
which means that changing the flag requires recompiling the kernel.
If your intent really is to strengthen SELinux execmod rules as I
guessed, then you could also perform a check against the SELinux
context of the tracer in mem_open(), or something like that - then you
might not need to make this configurable via kconfig anymore.