LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel@arm.com>
To: Anshuman Khandual <anshuman.khandual@arm.com>,
	linux-arm-kernel@lists.infradead.org
Cc: mark.rutland@arm.com, marc.zyngier@arm.com,
	Will Deacon <will.deacon@arm.com>,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Nadav Amit <namit@vmware.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	James Morse <james.morse@arm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Rick Edgecombe <rick.p.edgecombe@intel.com>
Subject: Re: [PATCH 3/4] arm64/kprobes: set VM_FLUSH_RESET_PERMS on kprobe instruction pages
Date: Tue, 28 May 2019 10:23:14 +0200	[thread overview]
Message-ID: <7c5e1fea-c93e-fc5b-8c77-c9bd9ec41fb3@arm.com> (raw)
In-Reply-To: <e10f0e6c-2669-8e1e-1b28-ed7816e0b248@arm.com>

On 5/28/19 10:20 AM, Anshuman Khandual wrote:
> 
> 
> On 05/23/2019 03:52 PM, Ard Biesheuvel wrote:
>> In order to avoid transient inconsistencies where freed code pages
>> are remapped writable while stale TLB entries still exist on other
>> cores, mark the kprobes text pages with the VM_FLUSH_RESET_PERMS
>> attribute. This instructs the core vmalloc code not to defer the
>> TLB flush when this region is unmapped and returned to the page
>> allocator.
> 
> Makes sense.
> 
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@arm.com>
>> ---
>>   arch/arm64/kernel/probes/kprobes.c | 4 +++-
>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
>> index 2509fcb6d404..036cfbf9682a 100644
>> --- a/arch/arm64/kernel/probes/kprobes.c
>> +++ b/arch/arm64/kernel/probes/kprobes.c
>> @@ -131,8 +131,10 @@ void *alloc_insn_page(void)
>>   	void *page;
>>   
>>   	page = vmalloc_exec(PAGE_SIZE);
>> -	if (page)
>> +	if (page) {
>>   		set_memory_ro((unsigned long)page, 1);
>> +		set_vm_flush_reset_perms(page);
>> +	}
> 
> Looks good. It seems there might be more users who would like to set
> VM_FLUSH_RESET_PERMS right after their allocation for the same reason.
> Hence would not it help to have a variant like vmalloc_exec_reset() or
> such which will tag vm_struct->flags with VM_FLUSH_RESET_PERMS right
> after it's allocation without requiring the caller to do the same.
> 

If there is a sufficient number of similar users, then of course, it 
makes sense to factor this out. However, the kprobes code is slightly 
unusual in the sense that it allocates memory and immediately remaps it 
read-only, and relies on code patching to populate this allocation. I am 
not expecting to see this pattern in other places.




  reply	other threads:[~2019-05-28  8:23 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-23 10:22 [PATCH 0/4] arm64: wire up VM_FLUSH_RESET_PERMS Ard Biesheuvel
2019-05-23 10:22 ` [PATCH 1/4] arm64: module: create module allocations without exec permissions Ard Biesheuvel
2019-05-28  5:35   ` Anshuman Khandual
2019-05-28  6:24     ` Ard Biesheuvel
2019-05-23 10:22 ` [PATCH 2/4] arm64/mm: wire up CONFIG_ARCH_HAS_SET_DIRECT_MAP Ard Biesheuvel
2019-05-28  8:10   ` Anshuman Khandual
2019-05-28  8:20     ` Ard Biesheuvel
2019-05-28  8:41       ` Anshuman Khandual
2019-05-28  8:58         ` Ard Biesheuvel
2019-05-23 10:22 ` [PATCH 3/4] arm64/kprobes: set VM_FLUSH_RESET_PERMS on kprobe instruction pages Ard Biesheuvel
2019-05-28  8:20   ` Anshuman Khandual
2019-05-28  8:23     ` Ard Biesheuvel [this message]
2019-05-23 10:22 ` [PATCH 4/4] arm64: bpf: do not allocate executable memory Ard Biesheuvel
2019-05-28 10:04 ` [PATCH 0/4] arm64: wire up VM_FLUSH_RESET_PERMS Will Deacon
2019-05-28 10:29   ` Ard Biesheuvel
2019-06-24 11:16   ` Will Deacon
2019-06-24 11:22     ` Ard Biesheuvel
2019-06-24 14:29       ` Ard Biesheuvel
2019-06-24 17:14         ` Catalin Marinas
2019-06-24 17:15           ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=7c5e1fea-c93e-fc5b-8c77-c9bd9ec41fb3@arm.com \
    --to=ard.biesheuvel@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=anshuman.khandual@arm.com \
    --cc=james.morse@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.zyngier@arm.com \
    --cc=mark.rutland@arm.com \
    --cc=mhiramat@kernel.org \
    --cc=namit@vmware.com \
    --cc=peterz@infradead.org \
    --cc=rick.p.edgecombe@intel.com \
    --cc=will.deacon@arm.com \
    --subject='Re: [PATCH 3/4] arm64/kprobes: set VM_FLUSH_RESET_PERMS on kprobe instruction pages' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).