create-diff-object: detect function padding without __pfx symbol

When IBT is enabled, objtool runs on the final linked vmlinux.o object instead of the individual translation units. So the __pfx symbols may be missing. Detect function padding regardless of whether the __pfx symbols exists. Signed-off-by: Josh Poimboeuf <[email protected]>
jpoimboe · Mar 18, 2024 · 3b18160 · 3b18160
1 parent 4077d87
commit 3b18160
Show file tree

Hide file tree

Showing 3 changed files with 66 additions and 3 deletions.
diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c
@@ -228,6 +228,62 @@ static struct rela *toc_rela(const struct rela *rela)
 				   (unsigned int)rela->addend);
 }
 
+static unsigned int nop_bytes_before_function(struct kpatch_elf *kelf,
+					      struct symbol *sym)
+{
+	switch (kelf->arch) {
+	case X86_64:
+	{
+		unsigned char *insn = sym->sec->data->d_buf;
+		unsigned int size = 0, i;
+
+		for (i = 0; i < sym->sym.st_value; i++, insn++) {
+			if (*insn != 0x90)
+				break;
+			size++;
+		}
+		return size;
+	}
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+static unsigned int function_padding_size(struct kpatch_elf *kelf,
+					  struct symbol *sym)
+{
+	static unsigned int size = (unsigned int)-1;
+	char *str;
+
+	if (size == (unsigned int)-1) {
+		str = getenv("CONFIG_FUNCTION_PADDING_BYTES");
+		if (str)
+			size = atoi(str);
+		else if (!getenv("KPATCH_UNIT_TEST"))
+			size = 0;
+		else {
+			/*
+			 * This is a unit test.  We could in theory require
+			 * unit tests to set CONFIG_FUNCTION_PADDING_BYTES in
+			 * CDO_ENV and/or in the test-specific .env file.
+			 * However, that might be more trouble than it's worth.
+			 *
+			 * For now, just find the padding dynamically.  Note
+			 * this is only calculated once ('size' is a static
+			 * local).
+			 */
+
+			size = nop_bytes_before_function(kelf, sym);
+			if (size != 0 && size != 16)
+				ERROR("unexpected bundled function offset in unit test: %u", size);
+		}
+	}
+
+	return size;
+}
+
 /*
  * When compiling with -ffunction-sections and -fdata-sections, almost every
  * symbol gets its own dedicated section.  We call such symbols "bundled"
@@ -244,6 +300,8 @@ static void kpatch_bundle_symbols(struct kpatch_elf *kelf)
 				expected_offset = 16;
 			else if (is_gcc6_localentry_bundled_sym(kelf, sym))
 				expected_offset = 8;
+			else if (sym->type == STT_FUNC)
+				expected_offset = function_padding_size(kelf, sym);
 			else
 				expected_offset = 0;
 

diff --git a/kpatch-build/kpatch-build b/kpatch-build/kpatch-build
@@ -1114,6 +1114,9 @@ trace_off "reading .config"
 source "$CONFIGFILE"
 trace_on
 
+# for create-diff-object
+export CONFIG_FUNCTION_PADDING_BYTES
+
 [[ "$DISTRO" = openEuler ]] && [[ -z "$CONFIG_LIVEPATCH_PER_TASK_CONSISTENCY" ]] && \
 	die "openEuler kernel doesn't have 'CONFIG_LIVEPATCH_PER_TASK_CONSISTENCY' enabled"
 
@@ -1453,7 +1456,7 @@ KPATCH_LDFLAGS="$KPATCH_LDFLAGS" \
 CROSS_COMPILE="$CROSS_COMPILE"
 save_env
 
-make "${MAKEVARS[@]}" 2>&1 | logger || die
+make V=1 "${MAKEVARS[@]}" 2>&1 | logger || die
 
 if [[ "$USE_KLP" -eq 1 ]]; then
 	if [[ "$USE_KLP_ARCH" -eq 0 ]]; then

diff --git a/test/unit/Makefile.include b/test/unit/Makefile.include
@@ -57,15 +57,17 @@ clean:
 	@echo "BUILD $(TNAME)"
 	$(call check_all,$(TNAME).$(EXT_ORIG))
 	$(call check_all,$(TNAME).$(EXT_PATCHED))
-	$(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_PATCHED) \
+	KPATCH_UNIT_TEST=1 $(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) \
+		$(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_PATCHED) \
 		vmlinux $(TNAME).$(EXT_SYMTAB) $(SYMVERS_FILE) \
 		test_$(TNAME) $@ $(MUTE_PASS)
 
 %.$(EXT_OUTPUT): %.$(EXT_ORIG) %.$(EXT_FAIL) %.$(EXT_SYMTAB) $(CDO)
 	@echo "BUILD $(TNAME)-FAIL"
 	$(call check_all,$(TNAME).$(EXT_ORIG))
 	$(call check_all,$(TNAME).$(EXT_FAIL))
-	! $(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) $(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_FAIL) \
+	! KPATCH_UNIT_TEST=1 $(CDO_ENV) $(shell cat $(TNAME).$(EXT_ENV) 2>/dev/null) \
+		$(CDO) $(TNAME).$(EXT_ORIG) $(TNAME).$(EXT_FAIL) \
 		vmlinux $(TNAME).$(EXT_SYMTAB) $(SYMVERS_FILE) \
 		test_$(TNAME) $@ $(MUTE_FAIL)
 # Expecting to fail, thus create output file manually so we won't rerun the