Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unchanged and unpatchable function moves from .text to .text.unlikely #1386

Closed
joe-lawrence opened this issue Apr 15, 2024 · 5 comments
Closed
Labels

Comments

@joe-lawrence
Copy link
Contributor

Xinhua Li reports another potential bug similar to #1385 (building the following kpatch with RHEL9 kernel-5.14.0-362.24.1.el9_3.x86_64) except this time it's an UNchanged function that moves (so create-diff-object marks as CHANGED), but then it fails the mcount/fentry check as it's annotated as notrace:

--- src.orig/arch/x86/kernel/cpu/proc.c	2024-04-15 14:46:29.203617033 -0400
+++ src/arch/x86/kernel/cpu/proc.c	2024-04-15 14:46:43.803518044 -0400
@@ -65,7 +65,8 @@ static int show_cpuinfo(struct seq_file
 	int i;
 
 	cpu = c->cpu_index;
-	seq_printf(m, "processor\t: %u\n"
+	printk(KERN_ALERT "show_cpuinfo current \n");
+	seq_printf(m, "TAM demo processor\t: %u\n"
 		   "vendor_id\t: %s\n"
 		   "cpu family\t: %d\n"
 		   "model\t\t: %u\n"

results in kpatch-build error:

proc.ORIG.o: function cpumask_weight has no fentry/mcount call, unable to patch
ERROR: proc.ORIG.o: 1 function(s) can not be patched
create-diff-object: unreconcilable difference
@joe-lawrence
Copy link
Contributor Author

repro-1386.tar.gz

Note that cpumask_weight() moves from .text.cpumask_weight to .text.unlikely.cpumask_weight, but otherwise carries no object code differences:

$ diff -upr \
      <(objdump -Dr -j.text.cpumask_weight          proc.ORIG.o) \
      <(objdump -Dr -j.text.unlikely.cpumask_weight proc.PATCHED.o)
--- /dev/fd/63  2024-04-15 14:53:36.281721258 -0400
+++ /dev/fd/62  2024-04-15 14:53:36.282721251 -0400
@@ -1,8 +1,8 @@
 
-proc.ORIG.o:     file format elf64-x86-64
+proc.PATCHED.o:     file format elf64-x86-64
 
 
-Disassembly of section .text.cpumask_weight:
+Disassembly of section .text.unlikely.cpumask_weight:
 
 0000000000000000 <__pfx_cpumask_weight>:
    0:  90                      nop

cpumask_weight() is included because create-diff-object considers the section change as allowed and marks the symbol as CHANGED:

static void kpatch_compare_correlated_symbol(struct symbol *sym)
{

	[ ... snip ... ]

	/*
	 * If two symbols are correlated but their sections are not, then the
	 * symbol has changed sections.  This is only allowed if the symbol is
	 * moving out of an ignored section, or moving between normal/hot/unlikely
	 * subsections.
	 */
	if (sym1->sec && sym2->sec && sym1->sec->twin != sym2->sec) {
		if ((sym2->sec->twin && sym2->sec->twin->ignore) ||
		    kpatch_subsection_changed(sym1->sec, sym2->sec))
			sym->status = CHANGED;
		else
			DIFF_FATAL("symbol changed sections: %s", sym1->name);
	}

	[ ... snip ... ]

The CHANGED status means that we will be trying to kpatch the original function with the new one.

Copy link

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

@github-actions github-actions bot added the stale label May 16, 2024
Copy link

This issue was closed because it was inactive for 7 days after being marked stale.

Copy link

This issue has been open for 30 days with no activity and no assignee. It will be closed in 7 days unless a comment is added.

@github-actions github-actions bot added the stale label Jun 23, 2024
Copy link

github-actions bot commented Jul 1, 2024

This issue was closed because it was inactive for 7 days after being marked stale.

@github-actions github-actions bot closed this as completed Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant