[PATCH] x86/module: Fixed bug allowing invalid relocation addresses.

[PATCH] x86/module: Fixed bug allowing invalid relocation addresses.

[Gilad Wharton Kleinman]

If a kernel module with a bad relocation offset is loaded to a x86 kernel,
the kernel will apply the relocation to a address not inside the module
(resulting in memory in the kernel being overridden).

Signed-off-by: Gilad Wharton Kleinman <dkgs1998@gmail.com>
---
 arch/x86/kernel/module.c | 57 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 53 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/module.c b/arch/x86/kernel/module.c
index d5c72cb877b3..0929b614b62a 100644
--- a/arch/x86/kernel/module.c
+++ b/arch/x86/kernel/module.c
@@ -96,10 +96,19 @@ int apply_relocate(Elf32_Shdr *sechdrs,
 	Elf32_Rel *rel = (void *)sechdrs[relsec].sh_addr;
 	Elf32_Sym *sym;
 	uint32_t *location;
+	Elf32_Word section_size;
 
 	DEBUGP("Applying relocate section %u to %u\n",
 	       relsec, sechdrs[relsec].sh_info);
 	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
+
+		section_size = sechdrs[sechdrs[relsec].sh_info].sh_size;
+		if (section_size < rel[i].r_offset + sizeof(u32)) {
+			pr_err("%s: Relocation offset %u is not in section (section len %u)\n",
+				me->name, rel[i].r_offset, section_size);
+			return -ENOEXEC;
+		}
+
 		/* This is where to make the change */
 		location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
 			+ rel[i].r_offset;
@@ -126,6 +135,47 @@ int apply_relocate(Elf32_Shdr *sechdrs,
 	return 0;
 }
 #else /*X86_64*/
+
+bool is_reloc_addr_valid(Elf64_Shdr *sechdrs,
+			unsigned int relsec,
+			Elf64_Rela rel,
+			struct module *me)
+{
+	unsigned int rel_size;
+	Elf64_Xword section_size = sechdrs[sechdrs[relsec].sh_info].sh_size;
+
+	switch (ELF64_R_TYPE(rel.r_info)) {
+	case R_X86_64_NONE:
+		return true;
+
+	case R_X86_64_64:
+	case R_X86_64_PC64:
+		rel_size = sizeof(u64);
+		break;
+
+	case R_X86_64_32:
+	case R_X86_64_32S:
+	case R_X86_64_PC32:
+	case R_X86_64_PLT32:
+		rel_size = sizeof(u32);
+		break;
+
+	default:
+		pr_err("%s: Unknown rela relocation: %llu\n",
+				me->name, ELF64_R_TYPE(rel.r_info));
+		return false;
+	}
+
+	if (section_size < rel.r_offset + rel_size) {
+		pr_err("%s: Relocation offset %llu and of length %u, is not in section (section len %llu)\n",
+			me->name, rel.r_offset,
+			rel_size, section_size);
+		return false;
+	}
+
+	return true;
+}
+
 int apply_relocate_add(Elf64_Shdr *sechdrs,
 		   const char *strtab,
 		   unsigned int symindex,
@@ -141,6 +191,9 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 	DEBUGP("Applying relocate section %u to %u\n",
 	       relsec, sechdrs[relsec].sh_info);
 	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
+		if (!is_reloc_addr_valid(sechdrs, relsec, rel[i], me))
+			return -ENOEXEC;
+
 		/* This is where to make the change */
 		loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
 			+ rel[i].r_offset;
@@ -195,10 +248,6 @@ int apply_relocate_add(Elf64_Shdr *sechdrs,
 			val -= (u64)loc;
 			*(u64 *)loc = val;
 			break;
-		default:
-			pr_err("%s: Unknown rela relocation: %llu\n",
-			       me->name, ELF64_R_TYPE(rel[i].r_info));
-			return -ENOEXEC;
 		}
 	}
 	return 0;
-- 
2.17.1

Re: [PATCH] x86/module: Fixed bug allowing invalid relocation addresses.

[Peter Zijlstra]

On Sat, Mar 14, 2020 at 11:36:26PM +0200, Gilad Wharton Kleinman wrote:
> If a kernel module with a bad relocation offset is loaded to a x86 kernel,
> the kernel will apply the relocation to a address not inside the module
> (resulting in memory in the kernel being overridden).

Why !?

If you load a bad module it's game over anyway. At best this protects us
from broken toolchains.

Re: [PATCH] x86/module: Fixed bug allowing invalid relocation addresses.

[gilad kleinman]

When you load a bad module and the kernel crashes -- the panic message
will usually be related to the module and easily traced back. If you
load a module with a bad relocation table (because of a bit-flip \ bad
toolchain), it will override other kernel code, resulting in an
impossible to trace back kernel panic.

On Mon, Mar 16, 2020 at 2:58 PM Peter Zijlstra <peterz@infradead.org> wrote:
>
> On Sat, Mar 14, 2020 at 11:36:26PM +0200, Gilad Wharton Kleinman wrote:
> > If a kernel module with a bad relocation offset is loaded to a x86 kernel,
> > the kernel will apply the relocation to a address not inside the module
> > (resulting in memory in the kernel being overridden).
>
> Why !?
>
> If you load a bad module it's game over anyway. At best this protects us
> from broken toolchains.

Re: [PATCH] x86/module: Fixed bug allowing invalid relocation addresses.

[Borislav Petkov]

On Wed, Mar 18, 2020 at 09:40:47PM +0200, gilad kleinman wrote:
> When you load a bad module and the kernel crashes -- the panic message will
> usually be related to the module and easily traced back. If you load a
> module with a bad relocation table (because of a bit-flip \ bad toolchain),

First of all, please do not top-post.

Then, if you're worried about corrupted module text, we have
CONFIG_MODULE_SIG which verifies previously signed modules. AFAICT, that
should take care of the issue you're talking about here.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette