create-diff-object: avoid reloc-type collisions on elf.h constants

The elf.h reloc-type constants are not unique across archs, for example: #define R_PPC64_REL24 10 /* PC relative 26 bit */ #define R_X86_64_32 10 /* Direct 32 bit zero extended */ so to avoid any unexpected aliasing, guard all R_arch_type refs with a check on kelf->arch, or a global default arch set from the first elf encountered. Closes: dynup#1356 ("Do we need more robust archeticture protection") Signed-off-by: Pete Swain <[email protected]> Signed-off-by: Mihails Strasuns <[email protected]> Signed-off-by: Joe Lawrence <[email protected]>
joe-lawrence · Oct 10, 2024 · e03ef59 · e03ef59
1 parent a527949
commit e03ef59
Show file tree

Hide file tree

Showing 4 changed files with 77 additions and 30 deletions.
diff --git a/kpatch-build/create-diff-object.c b/kpatch-build/create-diff-object.c
@@ -215,6 +215,8 @@ static bool is_gcc6_localentry_bundled_sym(struct kpatch_elf *kelf,
  */
 static struct rela *toc_rela(const struct rela *rela)
 {
+	if (!is_arch(PPC64))
+		return (struct rela *)rela;
 	if (rela->type != R_PPC64_TOC16_HA &&
 	    rela->type != R_PPC64_TOC16_LO_DS)
 		return (struct rela *)rela;
@@ -1623,7 +1625,7 @@ static void kpatch_replace_sections_syms(struct kpatch_elf *kelf)
 
 				if (is_text_section(relasec->base) &&
 				    !is_text_section(sym->sec) &&
-				    rela->type == R_X86_64_32S &&
+				    is_arch(X86_64) && rela->type == R_X86_64_32S &&
 				    rela->addend == (long)sym->sec->sh.sh_size &&
 				    end == (long)sym->sec->sh.sh_size) {
 
@@ -3241,12 +3243,23 @@ static int function_ptr_rela(const struct rela *rela)
 {
 	const struct rela *rela_toc = toc_rela(rela);
 
+	switch (def_arch()) {
+	case PPC64:
+		if (rela->type != R_PPC64_TOC16_HA &&
+		    rela->type != R_PPC64_TOC16_LO_DS)
+			return false;
+		break;
+	case X86_64:
+		if (rela->type != R_X86_64_32S)
+			return false;
+		break;
+	default:
+		break;
+	}
+
 	return (rela_toc && rela_toc->sym->type == STT_FUNC &&
 		!rela_toc->sym->parent &&
-		rela_toc->addend == (int)rela_toc->sym->sym.st_value &&
-		(rela->type == R_X86_64_32S ||
-		rela->type == R_PPC64_TOC16_HA ||
-		rela->type == R_PPC64_TOC16_LO_DS));
+		rela_toc->addend == (int)rela_toc->sym->sym.st_value);
 }
 
 static bool need_klp_reloc(struct kpatch_elf *kelf, struct lookup_table *table,
@@ -3261,32 +3274,38 @@ static bool need_klp_reloc(struct kpatch_elf *kelf, struct lookup_table *table,
 	 * These references are treated specially by the module loader and
 	 * should never be converted to klp relocations.
 	 */
-	if (rela->type == R_PPC64_REL16_HA || rela->type == R_PPC64_REL16_LO ||
-	    rela->type == R_PPC64_ENTRY)
-		return false;
+	switch (kelf->arch) {
+	case PPC64:
+		if (rela->type == R_PPC64_REL16_HA || rela->type == R_PPC64_REL16_LO ||
+		    rela->type == R_PPC64_ENTRY)
+			return false;
 
-	/* v5.13+ kernels use relative jump labels */
-	if (rela->type == R_PPC64_REL64 && strcmp(relasec->name, ".rela__jump_table"))
-		return false;
+		/* v5.13+ kernels use relative jump labels */
+		if (rela->type == R_PPC64_REL64 && strcmp(relasec->name, ".rela__jump_table"))
+			return false;
 
-	/*
-	 * On powerpc, the function prologue generated by GCC 6 has the
-	 * sequence:
-	 *
-	 *	.globl my_func
-	 *	.type my_func, @function
-	 *	.quad .TOC.-my_func
-	 * my_func:
-	 *	.reloc ., R_PPC64_ENTRY ; optional
-	 *	ld r2,-8(r12)
-	 *	add r2,r2,r12
-	 *	.localentry my_func, .-my_func
-	 *
-	 * The R_PPC64_ENTRY is optional and its symbol might have an empty
-	 * name.  Leave it as a normal rela.
-	 */
-	if (rela->type == R_PPC64_ENTRY)
-		return false;
+		/*
+		 * On powerpc, the function prologue generated by GCC 6 has the
+		 * sequence:
+		 *
+		 *	.globl my_func
+		 *	.type my_func, @function
+		 *	.quad .TOC.-my_func
+		 * my_func:
+		 *	.reloc ., R_PPC64_ENTRY ; optional
+		 *	ld r2,-8(r12)
+		 *	add r2,r2,r12
+		 *	.localentry my_func, .-my_func
+		 *
+		 * The R_PPC64_ENTRY is optional and its symbol might have an empty
+		 * name.  Leave it as a normal rela.
+		 */
+		if (rela->type == R_PPC64_ENTRY)
+			return false;
+		break;
+	default:
+		break;
+	}
 
 	/*
 	 * Allow references to core module symbols to remain as normal

diff --git a/kpatch-build/create-kpatch-module.c b/kpatch-build/create-kpatch-module.c
@@ -58,6 +58,9 @@ static void create_dynamic_rela_sections(struct kpatch_elf *kelf, struct section
 	dynsec = create_section_pair(kelf, ".kpatch.dynrelas", sizeof(*dynrelas), nr);
 	dynrelas = dynsec->data->d_buf;
 
+	if (kelf->arch != X86_64)
+		return;
+
 	for (index = 0; index < nr; index++) {
 		offset = index * (unsigned int)sizeof(*krelas);
 

diff --git a/kpatch-build/kpatch-elf.c b/kpatch-build/kpatch-elf.c
@@ -38,6 +38,26 @@
  * Helper functions
  ******************/
 
+static enum architecture current_arch;
+
+enum architecture def_arch(void)
+{
+	return current_arch;
+}
+
+bool is_arch(enum architecture arch)
+{
+	return current_arch == arch;
+}
+
+void set_arch(enum architecture arch)
+{
+	if (!arch || (current_arch && arch != current_arch))
+		ERROR("inconsistent ELF arch: setting %d but already %d",
+		      arch, current_arch);
+	current_arch = arch;
+}
+
 char *status_str(enum status status)
 {
 	switch(status) {
@@ -606,8 +626,10 @@ struct kpatch_elf *kpatch_elf_open(const char *name)
 		kelf->arch = S390;
 		break;
 	default:
-		ERROR("Unsupported target architecture");
+		ERROR("Unsupported target architecture: e_machine %x",
+		      ehdr.e_machine);
 	}
+	set_arch(kelf->arch);
 
 	kpatch_create_section_list(kelf);
 	kpatch_create_symbol_list(kelf);

diff --git a/kpatch-build/kpatch-elf.h b/kpatch-build/kpatch-elf.h
@@ -161,6 +161,9 @@ int offset_of_string(struct list_head *list, char *name);
 long rela_target_offset(struct kpatch_elf *kelf, struct section *relasec,
 			struct rela *rela);
 unsigned int insn_length(struct kpatch_elf *kelf, void *addr);
+enum architecture def_arch(void);
+void set_arch(enum architecture);
+bool is_arch(enum architecture);
 
 #ifndef R_PPC64_ENTRY
 #define R_PPC64_ENTRY   118