Re: [PATCH v4] powerpc: Do not make the entire heap executable
From: Oleg Nesterov <oleg@redhat.com>
Date: 2016-08-10 20:24:11
Also in:
linux-mm, lkml
On 08/10, Denys Vlasenko wrote:
Currently, to support 32-bit binaries with PLT in BSS kernel maps *entire brk area* with executable rights for all binaries, even --secure-plt ones. Stop doing that.
Can't really review this patch, but at least the change in mm/mmap.c looks technically correct to me... One nit below, feel free to ignore.
quoted hunk ↗ jump to hunk
@@ -2668,7 +2668,7 @@ static int do_brk(unsigned long addr, unsigned long request) if (!len) return 0; - flags = VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags; + flags |= VM_DATA_DEFAULT_FLAGS | VM_ACCOUNT | mm->def_flags;
OK. But note that we have mlock_future_check(mm->def_flags); a few lines below and after this change this _looks_ wrong because VM_LOCKED can come from the new "flags" argument passed to do_brk(). Nobody does this right now, still this looks wrong/confusing. I'd suggest to add another change - mlock_future_check(mm->def_flags); + mlock_future_check(flags); or add a sanity check at the start to deny VM_LOCKED and perhaps something else... The same for vm_brk_flags() which after your change does do_brk_flags(flags); populate = (mm->def_flags & VM_LOCKED); again, this is just a nit, I do not think it will be ever called with VM_LOCKED in "flags". Oleg.