| Message ID | 1566563895-2081-1-git-send-email-levonshe@yandex.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [RFC] security hardening: block write to read_only pages of a target process. | expand |
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.
diff --git a/arch/powerpc/include/asm/mmu_context.h b/arch/powerpc/include/asm/mmu_context.h index 58efca9..db37c61 100644 --- a/arch/powerpc/include/asm/mmu_context.h +++ b/arch/powerpc/include/asm/mmu_context.h @@ -251,6 +251,13 @@ 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 + if (write && foreign && (!(vma->vm_flags & VM_WRITE))) { + /* Forbid write to PROT_READ pages of foreign process */ + return false; + } +#endif + /* by default, allow everything */ return true; } diff --git a/arch/powerpc/mm/book3s64/pkeys.c b/arch/powerpc/mm/book3s64/pkeys.c index ae7fca4..b70fdfd 100644 --- a/arch/powerpc/mm/book3s64/pkeys.c +++ b/arch/powerpc/mm/book3s64/pkeys.c @@ -406,6 +406,12 @@ static inline bool vma_is_foreign(struct vm_area_struct *vma) bool arch_vma_access_permitted(struct vm_area_struct *vma, bool write, bool execute, bool foreign) { +#ifdef CONFIG_PROTECT_READONLY_USER_MEMORY + if (write && foreign && (!(vma->vm_flags & VM_WRITE))) { + /* Forbid write to PROT_READ pages of foreign process */ + return false; + } +#endif if (static_branch_likely(&pkey_disabled)) return true; /* diff --git a/arch/um/include/asm/mmu_context.h b/arch/um/include/asm/mmu_context.h index 00cefd3..2c56ce9 100644 --- a/arch/um/include/asm/mmu_context.h +++ b/arch/um/include/asm/mmu_context.h @@ -33,6 +33,12 @@ static inline void arch_bprm_mm_init(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 + if (write && foreign && (!(vma->vm_flags & VM_WRITE))) { + /* Forbid write to PROT_READ pages of foreign process */ + return false; + } +#endif /* by default, allow everything */ return true; } diff --git a/arch/unicore32/include/asm/mmu_context.h b/arch/unicore32/include/asm/mmu_context.h index 247a07a..730997c 100644 --- a/arch/unicore32/include/asm/mmu_context.h +++ b/arch/unicore32/include/asm/mmu_context.h @@ -97,7 +97,13 @@ static inline void arch_bprm_mm_init(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 + if (write && foreign && (!(vma->vm_flags & VM_WRITE))) { + /* Forbid write to PROT_READ pages of foreign process */ + return false; + } +#endif /* by default, allow everything */ return true; } -#endif +#endif /*__UNICORE_MMU_CONTEXT_H__*/ diff --git a/arch/x86/include/asm/mmu_context.h b/arch/x86/include/asm/mmu_context.h index 9024236..77b2801 100644 --- a/arch/x86/include/asm/mmu_context.h +++ b/arch/x86/include/asm/mmu_context.h @@ -329,12 +329,20 @@ static inline bool vma_is_foreign(struct vm_area_struct *vma) 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 + if (write && foreign && (!(vma->vm_flags & VM_WRITE))) { + /* Forbid write to PROT_READ pages of foreign process */ + 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); } diff --git a/include/asm-generic/mm_hooks.h b/include/asm-generic/mm_hooks.h index 6736ed2..31dae5a 100644 --- a/include/asm-generic/mm_hooks.h +++ b/include/asm-generic/mm_hooks.h @@ -30,6 +30,12 @@ static inline void arch_bprm_mm_init(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 + if (write && foreign && (!(vma->vm_flags & VM_WRITE))) { + /* Forbid write to PROT_READ pages of foreign process */ + return false; + } +#endif /* by default, allow everything */ return true; } diff --git a/security/Kconfig b/security/Kconfig index 0d65594..03ff948 100644 --- a/security/Kconfig +++ b/security/Kconfig @@ -143,6 +143,17 @@ 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" + depends on !(CONFIG_CROSS_MEMORY_ATTACH) + 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. + + config HAVE_HARDENED_USERCOPY_ALLOCATOR bool help
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. 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. 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@yandex.com> --- arch/powerpc/include/asm/mmu_context.h | 7 +++++++ arch/powerpc/mm/book3s64/pkeys.c | 6 ++++++ arch/um/include/asm/mmu_context.h | 6 ++++++ arch/unicore32/include/asm/mmu_context.h | 8 +++++++- arch/x86/include/asm/mmu_context.h | 10 +++++++++- include/asm-generic/mm_hooks.h | 6 ++++++ security/Kconfig | 11 +++++++++++ 7 files changed, 52 insertions(+), 2 deletions(-)