linux-kernel - Re: [PATCH v3 3/3] kernel/events/uprobes: uprobe_write

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: <69256f08-1d1c-45ac-945a-495ef4ffc558@redhat.com>
Date: Fri, 21 Mar 2025 14:17:12 +0100
From: David Hildenbrand <david@...hat.com>
To: Jiri Olsa <olsajiri@...il.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org,
 linux-arm-kernel@...ts.infradead.org, linux-trace-kernel@...r.kernel.org,
 linux-perf-users@...r.kernel.org, Andrew Morton <akpm@...ux-foundation.org>,
 Andrii Nakryiko <andrii.nakryiko@...il.com>,
 Matthew Wilcox <willy@...radead.org>, Russell King <linux@...linux.org.uk>,
 Masami Hiramatsu <mhiramat@...nel.org>, Oleg Nesterov <oleg@...hat.com>,
 Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
 Arnaldo Carvalho de Melo <acme@...nel.org>,
 Namhyung Kim <namhyung@...nel.org>, Mark Rutland <mark.rutland@....com>,
 Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
 Ian Rogers <irogers@...gle.com>, Adrian Hunter <adrian.hunter@...el.com>,
 "Liang, Kan" <kan.liang@...ux.intel.com>,
 Tong Tiangen <tongtiangen@...wei.com>
Subject: Re: [PATCH v3 3/3] kernel/events/uprobes: uprobe_write_opcode()
 rewrite

On 21.03.25 14:05, Jiri Olsa wrote:
> On Fri, Mar 21, 2025 at 12:37:13PM +0100, David Hildenbrand wrote:
> 
> SNIP
> 
>> +static int __uprobe_write_opcode(struct vm_area_struct *vma,
>> +		struct folio_walk *fw, struct folio *folio,
>> +		unsigned long opcode_vaddr, uprobe_opcode_t opcode)
>> +{
>> +	const unsigned long vaddr = opcode_vaddr & PAGE_MASK;
>> +	const bool is_register = !!is_swbp_insn(&opcode);
>> +	bool pmd_mappable;
>> +
>> +	/* For now, we'll only handle PTE-mapped folios. */
>> +	if (fw->level != FW_LEVEL_PTE)
>> +		return -EFAULT;
>> +
>> +	/*
>> +	 * See can_follow_write_pte(): we'd actually prefer a writable PTE here,
>> +	 * but the VMA might not be writable.
>> +	 */
>> +	if (!pte_write(fw->pte)) {
>> +		if (!PageAnonExclusive(fw->page))
>> +			return -EFAULT;
>> +		if (unlikely(userfaultfd_pte_wp(vma, fw->pte)))
>> +			return -EFAULT;
>> +		/* SOFTDIRTY is handled via pte_mkdirty() below. */
>> +	}
>> +
>> +	/*
>> +	 * We'll temporarily unmap the page and flush the TLB, such that we can
>> +	 * modify the page atomically.
>> +	 */
>> +	flush_cache_page(vma, vaddr, pte_pfn(fw->pte));
>> +	fw->pte = ptep_clear_flush(vma, vaddr, fw->ptep);
>> +	copy_to_page(fw->page, opcode_vaddr, &opcode, UPROBE_SWBP_INSN_SIZE);
>> +
>> +	/*
>> +	 * When unregistering, we may only zap a PTE if uffd is disabled and
>> +	 * there are no unexpected folio references ...
>> +	 */
>> +	if (is_register || userfaultfd_missing(vma) ||
>> +	    (folio_ref_count(folio) != folio_mapcount(folio) + 1 +
>> +	     folio_test_swapcache(folio) * folio_nr_pages(folio)))
>> +		goto remap;
>> +
>> +	/*
>> +	 * ... and the mapped page is identical to the original page that
>> +	 * would get faulted in on next access.
>> +	 */
>> +	if (!orig_page_is_identical(vma, vaddr, fw->page, &pmd_mappable))
>> +		goto remap;
>> +
>> +	dec_mm_counter(vma->vm_mm, MM_ANONPAGES);
>> +	folio_remove_rmap_pte(folio, fw->page, vma);
>> +	if (!folio_mapped(folio) && folio_test_swapcache(folio) &&
>> +	     folio_trylock(folio)) {
>> +		folio_free_swap(folio);
>> +		folio_unlock(folio);
>> +	}
>> +	folio_put(folio);
> 
> hi,
> it's probably ok and I'm missing something, but why do we call folio_put
> in here, I'd think it's done by folio_put call in uprobe_write_opcode

Hi!

We're zapping a page table mapping mapping (folio_remove_rmap_pte()), so 
we have to drop that reference.

That's the folio_put(old_folio) in the old __replace_page().

Thanks!

-- 
Cheers,

David / dhildenb