[RFC v1 14/19] ptwrite uprobes: Use atomic patching for multinop sites

[Andi Kleen]

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.

The motivation for multinop is mainly to support the gcc
function entry patch sites and these are always aligned.

So enforce 8 bytes alignment of the multinop and use a safe RMW 8 byte store
ot 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 sashiko
Signed-off-by: Andi Kleen <ak@xxxxxxxxxx>
---
 arch/x86/kernel/uprobes.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/arch/x86/kernel/uprobes.c b/arch/x86/kernel/uprobes.c
index 806e40f7b0ab..8d9dadc2b1fc 100644
--- a/arch/x86/kernel/uprobes.c
+++ b/arch/x86/kernel/uprobes.c
@@ -1812,8 +1812,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 pun_site_is_nop(const u8 *orig, bool allow_nop_run)
 {
@@ -1831,6 +1832,13 @@ static bool pun_site_is_nop(const u8 *orig, bool allow_nop_run)
 		orig[2] == 0x90 && orig[3] == 0x90 && orig[4] == 0x90;
 }
 
+/* Identify the explicitly opted-in run of five one-byte NOPs. */
+static bool ptwrite_site_is_multinop(const u8 *orig, bool allow_nop_run)
+{
+	return allow_nop_run && orig[0] == 0x90 && orig[1] == 0x90 &&
+		orig[2] == 0x90 && orig[3] == 0x90 && orig[4] == 0x90;
+}
+
 /*
  * Classify the site's single instruction for out-of-line execution.
  * Returns the length, or 0 when it cannot run safely out of line.
@@ -2224,6 +2232,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) &&
+	    (vaddr & 7))
+		return pun_install(auprobe, vma, vaddr, orig);
 	if (ptwrite_is_installed(mm, vaddr, orig))
 		return 0;
 
@@ -2249,9 +2260,13 @@ int arch_uprobe_install_ptwrite(struct arch_uprobe *auprobe,
 			continue;
 		if (!__in_uprobe_ptwrite(mm, ptw->vaddr))
 			continue;
-		ret = ptwrite_text_poke(auprobe, vma, vaddr,
-					ptw->vaddr + ptw->index[b].off);
-		goto out;
+		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)
@@ -2287,8 +2302,11 @@ 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)
+	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;
-- 
2.54.0