linux-kernel - Re: [PATCH] arm64: kprobe: make page to RO mode when allocate it

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAK8P3a0oSn8Z6wJT1X2si8+A2wttkh7EjqD_MZYt4+L5a_XKLg@mail.gmail.com>
Date:   Mon, 29 Oct 2018 13:11:24 +0100
From:   Arnd Bergmann <arnd@...db.de>
To:     Will Deacon <will.deacon@....com>
Cc:     Anders Roxell <anders.roxell@...aro.org>, catalin.marinas@....com,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        Ard Biesheuvel <ard.biesheuvel@...aro.org>,
        Laura Abbott <labbott@...hat.com>
Subject: Re: [PATCH] arm64: kprobe: make page to RO mode when allocate it

On 10/29/18, Will Deacon <will.deacon@....com> wrote:
> On Mon, Oct 15, 2018 at 01:16:00PM +0200, Anders Roxell wrote:

>> -static int __kprobes patch_text(kprobe_opcode_t *addr, u32 opcode)
>> +void *alloc_insn_page(void)
>>  {
>> -	void *addrs[1];
>> -	u32 insns[1];
>> +	void *page;
>>
>> -	addrs[0] = (void *)addr;
>> -	insns[0] = (u32)opcode;
>> +	page = vmalloc_exec(PAGE_SIZE);
>> +	if (page)
>> +		set_memory_ro((unsigned long)page & PAGE_MASK, 1);
>
> This looks a bit strange to me -- you're allocating PAGE_SIZE bytes so
> that we can adjust the permissions, yet we can't guarantee that page is
> actually page-aligned and therefore end up explicitly masking down.
>
> In which case allocating an entire page isn't actually helping us, and
> we could end up racing with somebody else changing permission on the
> same page afaict.
>
> I think we need to ensure we really have an entire page, perhaps using
> vmap() instead? Or have I missed some subtle detail here?

I'm fairly sure that vmalloc() and vmalloc_exec() is guaranteed to be page
aligned everywhere. The documentation is a bit vague here, but I'm
still confident enough that we can make that assumption based on

/**
 *      vmalloc_exec  -  allocate virtually contiguous, executable memory
 *      @size:          allocation size
 *
 *      Kernel-internal function to allocate enough pages to cover @size
 *      the page level allocator and map them into contiguous and
 *      executable kernel virtual space.
 *
 *      For tight control over page level allocator and protection flags
 *      use __vmalloc() instead.
 */
void *vmalloc_exec(unsigned long size)

       Arnd