[RFC PATCH v2 09/11] ptwrite uprobes: Use atomic patching for multinop sites

[Andi Kleen <ak@kernel.org>]

The earlier multinop patching is not quite safe because the cross
modified CPU could be already executing on a later nop when the
cross patching occurs. The Intel SDM allows cross modification
by larger stores as long as they are aligned. AMD has a similar
guarantee.

Support GCC function-entry patch sites is the main motivation for
multinop, and these sites are always aligned.

So enforce 8 bytes alignment of the multinop and use a safe RMW 8 byte store
to overwrite the 5 byte sequence. This assumes that the code is not
changing in parallel, but if that happens cross modification safety
is probably the smallest of the issues.

Assisted-by: omp:gpt-5.6-luna
Signed-off-by: Andi Kleen <ak@kernel.org>
---
 arch/x86/kernel/uprobes.c | 183 +++++++++++++++++++++++---------------
 kernel/events/uprobes.c   |  15 +++-
 2 files changed, 123 insertions(+), 75 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index af9b36a219d3..114905bb69a8 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -17,6 +17,7 @@
 #include <linux/kdebug.h>
 #include <linux/highmem.h>
 #include <linux/mm.h>
+#include <linux/security.h>
 #include <asm/processor.h>
 #include <asm/insn.h>
 #include <asm/insn-eval.h>
@@ -828,6 +829,8 @@ int uprobe_ptwrite_dup_mmap(struct mm_struct *oldmm, struct mm_struct *newmm)
 		kunmap_local(src);
 		new->vaddr = ptw->vaddr;
 		new->cursor = ptw->cursor;
+		new->nblocks = ptw->nblocks;
+		memcpy(new->index, ptw->index, sizeof(new->index));
 
 		vma = install_uprobe_ptwrite_vma(newmm, new->vaddr);
 		if (IS_ERR(vma)) {
@@ -1448,7 +1451,7 @@ static_assert((((9 + UPROBE_PTWRITE_SERIALIZE_LFENCES *
 			  UPROBE_PTWRITE_MAX_ARGS *
 				  (10 + UPROBE_PTWRITE_SERIALIZE_LFENCES *
 				   UPROBE_PTWRITE_LFENCE_SIZE) +
-			  5 + 7) & ~7) +
+			  UPROBE_PTWRITE_COPY_SIZE + 5 + 7) & ~7) +
 		       8 * (1 + UPROBE_PTWRITE_MAX_ARGS)) <=
 		      UPROBE_PTWRITE_STUB_SIZE,
 		      "worst-case ptwrite stub block exceeds UPROBE_PTWRITE_STUB_SIZE");
@@ -1460,6 +1463,7 @@ static bool ptwrite_has_room(const u8 *base, const u8 *p, size_t len)
 }
 
 static bool pun_site_is_nop(const u8 *orig, bool allow_nop_run);
+static bool ptwrite_site_is_multinop(const u8 *orig, bool allow_nop_run);
 static int pun_classify_insn(struct insn *insn, u8 *disp_off, s32 *disp);
 static int pun_decode_site(struct inode *inode, struct file *file,
 			       loff_t offset, u8 *copy,
@@ -1494,7 +1498,18 @@ int arch_uprobe_ptwrite_prepare(struct arch_uprobe *auprobe,
 		return -EINVAL;
 
 	/* The generic registration path copied these bytes before this hook. */
+	ptw->allow_nop_run =
+		desc->flags & UPROBE_PTWRITE_FL_ALLOW_NOP_RUN;
 	memcpy(ptw->orig, auprobe->insn, sizeof(ptw->orig));
+	/*
+	 * File mappings preserve page offsets, so an unaligned file offset
+	 * cannot become an aligned runtime address. Reject it before the probe
+	 * is exposed; install-time failures for a future mapping are otherwise
+	 * not observable through the tracefs enable operation.
+	 */
+	if (ptwrite_site_is_multinop(ptw->orig, ptw->allow_nop_run) &&
+	    !IS_ALIGNED(offset, sizeof(u64)))
+		return -EINVAL;
 
 	for (i = 0; i < desc->nargs; i++) {
 		switch (desc->args[i].src) {
@@ -1618,7 +1633,7 @@ int arch_uprobe_ptwrite_prepare(struct arch_uprobe *auprobe,
 
 	ptw->stub_len = data_off + 8 * (1 + n_imm);
 	ptw->ndata = 1 + n_imm;
-	ptw->allow_nop_run = desc->flags & UPROBE_PTWRITE_FL_ALLOW_NOP_RUN;
+
 
 	ret = pun_decode_site(inode, file, offset, code + ptw->copy_off,
 				  &ptw->disp_off, &ptw->disp,
@@ -1753,8 +1768,9 @@ get_uprobe_ptwrite_page(struct mm_struct *mm, unsigned long vaddr,
 }
 
 /*
- * A run of short NOPs is accepted only when requested. This validation does
- * not make the three-phase poke safe for threads that already passed byte 0.
+ * A run of short NOPs is accepted only when requested. It is patched with
+ * an aligned eight-byte read-modify-write, preserving the following bytes;
+ * code is not expected to change concurrently.
  */
 static bool ptwrite_is_nop_run(const u8 *orig)
 {
@@ -1940,6 +1956,42 @@ static int ptwrite_text_poke(struct arch_uprobe *auprobe,
 	return err;
 }
 
+/*
+ * Replace an aligned five-byte NOP run with a JMP in one eight-byte store.
+ * The trailing three bytes are read from the existing text. We assume
+ * nobody else is changing it. This is covered by the Intel/AMD "aligned store"
+ * cross modifying guarantee.
+ */
+static int ptwrite_multinop_text_poke(struct arch_uprobe *auprobe,
+				      struct vm_area_struct *vma,
+				      unsigned long vaddr,
+				      unsigned long stub_addr)
+{
+	struct mm_struct *mm = vma->vm_mm;
+	struct write_opcode_ctx ctx = {
+		.base = vaddr,
+		.expect = EXPECT_BYTE,
+		.expect_byte = 0x90,
+	};
+	u8 patch[sizeof(u64)];
+	s32 rel;
+	int err;
+
+	if (!IS_ALIGNED(vaddr, sizeof(u64)))
+		return -EINVAL;
+	if (!ptwrite_rel32(vaddr + 5, stub_addr, &rel))
+		return -ERANGE;
+	err = copy_from_vaddr(mm, vaddr, patch, sizeof(patch));
+	if (err)
+		return err;
+	patch[0] = 0xe9;
+	memcpy(&patch[1], &rel, sizeof(rel));
+	err = uprobe_write(auprobe, vma, vaddr, patch, sizeof(patch),
+			   verify_insn, true, false, &ctx);
+	if (!err)
+		smp_text_poke_sync_each_cpu();
+	return err;
+}
 static int pun_text_poke(struct arch_uprobe *auprobe,
 				 struct vm_area_struct *vma,
 				 unsigned long vaddr, u8 e9,
@@ -1971,65 +2023,40 @@ static int pun_install(struct arch_uprobe *auprobe,
 	unsigned long t, page_base, block_off, stub_addr;
 	s64 site_delta, target;
 	s32 jump_rel, disp32, orig_rel;
-	u8 site_len;
 	bool found = false;
-	bool nop_fallback = ptwrite_site_is_multinop(orig,
-						     ptw_a->allow_nop_run) &&
-			    (vaddr & 7);
 	u8 *kaddr;
 	int b, ret;
 
 	mmap_assert_write_locked(mm);
-	if (nop_fallback) {
-		hlist_for_each_entry(ptw, &state->head_ptwrite, node) {
-			site_delta = (s64)vaddr - (s64)ptw->vaddr;
-			if (site_delta < INT_MIN || site_delta > INT_MAX)
-				continue;
-			for (b = 0; b < smp_load_acquire(&ptw->nblocks); b++)
-				if (!ptw->index[b].pun &&
-				    ptw->index[b].site_off == (s32)site_delta &&
-				    ptw->index[b].site_len == 5 &&
-				    !memcmp(ptw->index[b].site_insn, orig, 5))
-					break;
-			if (b >= smp_load_acquire(&ptw->nblocks))
-				continue;
-			if (!__in_uprobe_ptwrite(mm, ptw->vaddr))
-				continue;
-			return ptwrite_text_poke(auprobe, vma, vaddr,
-						 ptw->vaddr + ptw->index[b].off);
+	memcpy(&orig_rel, orig + 1, sizeof(orig_rel));
+	target = (s64)vaddr + 5 + (s64)orig_rel;
+	if (target < PAGE_SIZE || target >= TASK_SIZE_MAX)
+		return -EADDRNOTAVAIL;
+	t = (unsigned long)target;
+	page_base = t & PAGE_MASK;
+	ret = security_mmap_addr(page_base);
+	if (ret)
+		return ret;
+	block_off = t & (PAGE_SIZE - 1);
+	if (block_off + ptw_a->stub_len > PAGE_SIZE)
+		return -ENOSPC;
+
+	/* Reuse an existing ptwrite page at the target, else map a new one. */
+	hlist_for_each_entry(ptw, &state->head_ptwrite, node) {
+		if (ptw->vaddr == page_base) {
+			found = true;
+			break;
 		}
-		ptw = get_uprobe_ptwrite_page(mm, vaddr, ptw_a->stub_len);
+	}
+	if (!found) {
+		if (vma_lookup(mm, page_base))
+			return -EADDRNOTAVAIL;
+		ptw = create_uprobe_ptwrite_page_at(mm, page_base);
 		if (!ptw)
 			return -ENOMEM;
-		block_off = ptw->cursor;
-	} else {
-		memcpy(&orig_rel, orig + 1, sizeof(orig_rel));
-		target = (s64)vaddr + 5 + (s64)orig_rel;
-		if (target < PAGE_SIZE || target >= TASK_SIZE_MAX)
-			return -EADDRNOTAVAIL;
-		t = (unsigned long)target;
-		page_base = t & PAGE_MASK;
-		block_off = t & (PAGE_SIZE - 1);
-		if (block_off + ptw_a->stub_len > PAGE_SIZE)
-			return -ENOSPC;
-
-		/* reuse an existing ptwrite page at the target, else map a new one */
-		hlist_for_each_entry(ptw, &state->head_ptwrite, node) {
-			if (ptw->vaddr == page_base) {
-				found = true;
-				break;
-			}
-		}
-		if (!found) {
-			if (vma_lookup(mm, page_base))
-				return -EADDRNOTAVAIL;	/* target page occupied */
-			ptw = create_uprobe_ptwrite_page_at(mm, page_base);
-			if (!ptw)
-				return -ENOMEM;
-			/* Order page initialization before publishing it to fault readers. */
-			smp_wmb();
-			hlist_add_head_rcu(&ptw->node, &state->head_ptwrite);
-		}
+		/* Publish initialized page fields before fault readers find it. */
+		smp_wmb();
+		hlist_add_head_rcu(&ptw->node, &state->head_ptwrite);
 	}
 
 	site_delta = (s64)vaddr - (s64)ptw->vaddr;
@@ -2068,19 +2095,13 @@ static int pun_install(struct arch_uprobe *auprobe,
 		disp32 = (s32)d;
 	}
 
-	/*
-	 * A NOP fallback needs a synthetic rel32 at the site, so it uses
-	 * the full five-byte poke and restore path rather than punning.
-	 */
-	site_len = nop_fallback ? 5 : ptw_a->len;
 	ptw->index[ptw->nblocks].off = block_off;
 	ptw->index[ptw->nblocks].len = ptw_a->stub_len;
-	ptw->index[ptw->nblocks].pun = !nop_fallback;
+	ptw->index[ptw->nblocks].pun = 1;
 	ptw->index[ptw->nblocks].orig0 = orig[0];
-	ptw->index[ptw->nblocks].site_len = site_len;
+	ptw->index[ptw->nblocks].site_len = ptw_a->len;
 	ptw->index[ptw->nblocks].site_off = (s32)site_delta;
-	memcpy(ptw->index[ptw->nblocks].site_insn, ptw_a->orig, site_len);
-	smp_store_release(&ptw->nblocks, ptw->nblocks + 1);
+	memcpy(ptw->index[ptw->nblocks].site_insn, ptw_a->orig, ptw_a->len);
 
 	kaddr = kmap_local_page(ptw->page);
 	memcpy(kaddr + block_off, ptw_a->stub, ptw_a->stub_len);
@@ -2089,11 +2110,10 @@ static int pun_install(struct arch_uprobe *auprobe,
 		memcpy(kaddr + block_off + ptw_a->copy_off + ptw_a->disp_off,
 		       &disp32, sizeof(disp32));
 	kunmap_local(kaddr);
+	/* Publish initialized metadata before exposing the probe jump. */
+	smp_store_release(&ptw->nblocks, ptw->nblocks + 1);
 
-	if (nop_fallback)
-		ret = ptwrite_text_poke(auprobe, vma, vaddr, stub_addr);
-	else
-		ret = pun_text_poke(auprobe, vma, vaddr, 0xe9, &ctx);
+	ret = pun_text_poke(auprobe, vma, vaddr, 0xe9, &ctx);
 	if (ret) {
 		/* Publish rollback before readers observe the reduced block count. */
 		smp_store_release(&ptw->nblocks, ptw->nblocks - 1);
@@ -2129,6 +2149,9 @@ int arch_uprobe_install_ptwrite(struct arch_uprobe *auprobe,
 	ret = copy_from_vaddr(mm, vaddr, orig, sizeof(orig));
 	if (ret)
 		return ret;
+	if (ptwrite_site_is_multinop(orig, ptw_a->allow_nop_run) &&
+	    !IS_ALIGNED(vaddr, sizeof(u64)))
+		return -EINVAL;
 	if (ptwrite_is_installed(mm, vaddr, orig))
 		return 0;
 
@@ -2151,8 +2174,13 @@ int arch_uprobe_install_ptwrite(struct arch_uprobe *auprobe,
 			continue;
 		if (!__in_uprobe_ptwrite(mm, ptw->vaddr))
 			continue;
-		return ptwrite_text_poke(auprobe, vma, vaddr,
-					ptw->vaddr + ptw->index[b].off);
+		if (ptwrite_site_is_multinop(orig, ptw_a->allow_nop_run))
+			ret = ptwrite_multinop_text_poke(auprobe, vma, vaddr,
+							ptw->vaddr + ptw->index[b].off);
+		else
+			ret = ptwrite_text_poke(auprobe, vma, vaddr,
+						ptw->vaddr + ptw->index[b].off);
+		return ret;
 	}
 	ptw = get_uprobe_ptwrite_page(mm, vaddr, ptw_a->stub_len);
 	if (!ptw)
@@ -2186,10 +2214,17 @@ int arch_uprobe_install_ptwrite(struct arch_uprobe *auprobe,
 	memcpy(kaddr + block_off + ptw_a->jmp_off, &rel, sizeof(rel));
 	kunmap_local(kaddr);
 
-	ret = ptwrite_text_poke(auprobe, vma, vaddr, stub_addr);
-	if (!ret)
-		ptw->cursor = block_off + ptw_a->stub_len;
-	return ret;
+	if (ptwrite_site_is_multinop(orig, ptw_a->allow_nop_run))
+		ret = ptwrite_multinop_text_poke(auprobe, vma, vaddr, stub_addr);
+	else
+		ret = ptwrite_text_poke(auprobe, vma, vaddr, stub_addr);
+	if (ret) {
+		/* Publish rollback before readers use the reduced block count. */
+		smp_store_release(&ptw->nblocks, ptw->nblocks - 1);
+		return ret;
+	}
+	ptw->cursor = block_off + ptw_a->stub_len;
+	return 0;
 }
 
 int arch_uprobe_uninstall_ptwrite(struct arch_uprobe *auprobe,
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 78bc847d73cc..18c5df46a509 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -192,7 +192,20 @@ void uprobe_copy_from_page(struct page *page, unsigned long vaddr, void *dst, in
 static void copy_to_page(struct page *page, unsigned long vaddr, const void *src, int len)
 {
 	void *kaddr = kmap_local_page(page);
-	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
+	void *dst = kaddr + (vaddr & ~PAGE_MASK);
+
+	/*
+	 * Atomic eight-byte stores are required for safe cross-modification of
+	 * live user text; other writes use the ordinary byte-copy path.
+	 */
+	if (len == sizeof(u64) && IS_ALIGNED(vaddr, sizeof(u64))) {
+		u64 value;
+
+		memcpy(&value, src, sizeof(value));
+		WRITE_ONCE(*(u64 *)dst, value);
+	} else {
+		memcpy(dst, src, len);
+	}
 	kunmap_local(kaddr);
 }
 
-- 
2.54.0