Commit 0c05e7bd authored by Song Liu's avatar Song Liu Committed by Petr Mladek
Browse files

livepatch,x86: Clear relocation targets on a module removal 



Josh reported a bug:

  When the object to be patched is a module, and that module is
  rmmod'ed and reloaded, it fails to load with:

  module: x86/modules: Skipping invalid relocation target, existing value is nonzero for type 2, loc 00000000ba0302e9, val ffffffffa03e293c
  livepatch: failed to initialize patch 'livepatch_nfsd' for module 'nfsd' (-8)
  livepatch: patch 'livepatch_nfsd' failed for module 'nfsd', refusing to load module 'nfsd'

  The livepatch module has a relocation which references a symbol
  in the _previous_ loading of nfsd. When apply_relocate_add()
  tries to replace the old relocation with a new one, it sees that
  the previous one is nonzero and it errors out.

He also proposed three different solutions. We could remove the error
check in apply_relocate_add() introduced by commit eda9cec4
("x86/module: Detect and skip invalid relocations"). However the check
is useful for detecting corrupted modules.

We could also deny the patched modules to be removed. If it proved to be
a major drawback for users, we could still implement a different
approach. The solution would also complicate the existing code a lot.

We thus decided to reverse the relocation patching (clear all relocation
targets on x86_64). The solution is not
universal and is too much arch-specific, but it may prove to be simpler
in the end.

Reported-by: default avatarJosh Poimboeuf <jpoimboe@redhat.com>
Originally-by: default avatarMiroslav Benes <mbenes@suse.cz>
Signed-off-by: default avatarSong Liu <song@kernel.org>
Acked-by: default avatarMiroslav Benes <mbenes@suse.cz>
Reviewed-by: default avatarPetr Mladek <pmladek@suse.com>
Acked-by: default avatarJosh Poimboeuf <jpoimboe@kernel.org>
Reviewed-by: default avatarJoe Lawrence <joe.lawrence@redhat.com>
Tested-by: default avatarJoe Lawrence <joe.lawrence@redhat.com>
Signed-off-by: default avatarPetr Mladek <pmladek@suse.com>
Link: https://lore.kernel.org/r/20230125185401.279042-2-song@kernel.org
parent bbb93362

arch/x86/kernel/module.c

+60 −33
Original line number Diff line number Diff line
@@ -128,22 +128,27 @@ int apply_relocate(Elf32_Shdr *sechdrs, 
	return 0;

}

#else /*X86_64*/

static int __apply_relocate_add(Elf64_Shdr *sechdrs,

static int __write_relocate_add(Elf64_Shdr *sechdrs,

		   const char *strtab,

		   unsigned int symindex,

		   unsigned int relsec,

		   struct module *me,

		   void *(*write)(void *dest, const void *src, size_t len))

		   void *(*write)(void *dest, const void *src, size_t len),

		   bool apply)

{

	unsigned int i;

	Elf64_Rela *rel = (void *)sechdrs[relsec].sh_addr;

	Elf64_Sym *sym;

	void *loc;

	u64 val;

	u64 zero = 0ULL;



	DEBUGP("Applying relocate section %u to %u\n",

	DEBUGP("%s relocate section %u to %u\n",

	       apply ? "Applying" : "Clearing",

	       relsec, sechdrs[relsec].sh_info);

	for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {

		size_t size;



		/* This is where to make the change */

		loc = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr

			+ rel[i].r_offset;
@@ -161,51 +166,52 @@ static int __apply_relocate_add(Elf64_Shdr *sechdrs, 


		switch (ELF64_R_TYPE(rel[i].r_info)) {

		case R_X86_64_NONE:

			break;

			continue;  /* nothing to write */

		case R_X86_64_64:

			if (*(u64 *)loc != 0)

				goto invalid_relocation;

			write(loc, &val, 8);

			size = 8;

			break;

		case R_X86_64_32:

			if (*(u32 *)loc != 0)

				goto invalid_relocation;

			write(loc, &val, 4);

			if (val != *(u32 *)loc)

			if (val != *(u32 *)&val)

				goto overflow;

			size = 4;

			break;

		case R_X86_64_32S:

			if (*(s32 *)loc != 0)

				goto invalid_relocation;

			write(loc, &val, 4);

			if ((s64)val != *(s32 *)loc)

			if ((s64)val != *(s32 *)&val)

				goto overflow;

			size = 4;

			break;

		case R_X86_64_PC32:

		case R_X86_64_PLT32:

			if (*(u32 *)loc != 0)

				goto invalid_relocation;

			val -= (u64)loc;

			write(loc, &val, 4);

			size = 4;

			break;

		case R_X86_64_PC64:

			if (*(u64 *)loc != 0)

				goto invalid_relocation;

			val -= (u64)loc;

			write(loc, &val, 8);

			size = 8;

			break;

		default:

			pr_err("%s: Unknown rela relocation: %llu\n",

			       me->name, ELF64_R_TYPE(rel[i].r_info));

			return -ENOEXEC;

		}

	}

	return 0;



invalid_relocation:

	pr_err("x86/modules: Skipping invalid relocation target, existing value is nonzero for type %d, loc %p, val %Lx\n",

		if (apply) {

			if (memcmp(loc, &zero, size)) {

				pr_err("x86/modules: Invalid relocation target, existing value is nonzero for type %d, loc %p, val %Lx\n",

				       (int)ELF64_R_TYPE(rel[i].r_info), loc, val);

				return -ENOEXEC;

			}

			write(loc, &val, size);

		} else {

			if (memcmp(loc, &val, size)) {

				pr_warn("x86/modules: Invalid relocation target, existing value does not match expected value for type %d, loc %p, val %Lx\n",

					(int)ELF64_R_TYPE(rel[i].r_info), loc, val);

				return -ENOEXEC;

			}

			write(loc, &zero, size);

		}

	}

	return 0;



overflow:

	pr_err("overflow in relocation type %d val %Lx\n",
@@ -215,11 +221,12 @@ static int __apply_relocate_add(Elf64_Shdr *sechdrs, 
	return -ENOEXEC;

}



int apply_relocate_add(Elf64_Shdr *sechdrs,

static int write_relocate_add(Elf64_Shdr *sechdrs,

			      const char *strtab,

			      unsigned int symindex,

			      unsigned int relsec,

		   struct module *me)

			      struct module *me,

			      bool apply)

{

	int ret;

	bool early = me->state == MODULE_STATE_UNFORMED;
@@ -230,8 +237,8 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, 
		mutex_lock(&text_mutex);

	}



	ret = __apply_relocate_add(sechdrs, strtab, symindex, relsec, me,

				   write);

	ret = __write_relocate_add(sechdrs, strtab, symindex, relsec, me,

				   write, apply);



	if (!early) {

		text_poke_sync();
@@ -241,6 +248,26 @@ int apply_relocate_add(Elf64_Shdr *sechdrs, 
	return ret;

}



int apply_relocate_add(Elf64_Shdr *sechdrs,

		   const char *strtab,

		   unsigned int symindex,

		   unsigned int relsec,

		   struct module *me)

{

	return write_relocate_add(sechdrs, strtab, symindex, relsec, me, true);

}



#ifdef CONFIG_LIVEPATCH

void clear_relocate_add(Elf64_Shdr *sechdrs,

			const char *strtab,

			unsigned int symindex,

			unsigned int relsec,

			struct module *me)

{

	write_relocate_add(sechdrs, strtab, symindex, relsec, me, false);

}

#endif



#endif



int module_finalize(const Elf_Ehdr *hdr,

include/linux/moduleloader.h

+17 −0
Original line number Diff line number Diff line
@@ -72,6 +72,23 @@ int apply_relocate_add(Elf_Shdr *sechdrs, 
		       unsigned int symindex,

		       unsigned int relsec,

		       struct module *mod);

#ifdef CONFIG_LIVEPATCH

/*

 * Some architectures (namely x86_64 and ppc64) perform sanity checks when

 * applying relocations.  If a patched module gets unloaded and then later

 * reloaded (and re-patched), klp re-applies relocations to the replacement

 * function(s).  Any leftover relocations from the previous loading of the

 * patched module might trigger the sanity checks.

 *

 * To prevent that, when unloading a patched module, clear out any relocations

 * that might trigger arch-specific sanity checks on a future module reload.

 */

void clear_relocate_add(Elf_Shdr *sechdrs,

		   const char *strtab,

		   unsigned int symindex,

		   unsigned int relsec,

		   struct module *me);

#endif

#else

static inline int apply_relocate_add(Elf_Shdr *sechdrs,

				     const char *strtab,

kernel/livepatch/core.c

+49 −13
Original line number Diff line number Diff line
@@ -261,6 +261,14 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab, 
	return 0;

}



void __weak clear_relocate_add(Elf_Shdr *sechdrs,

		   const char *strtab,

		   unsigned int symindex,

		   unsigned int relsec,

		   struct module *me)

{

}



/*

 * At a high-level, there are two types of klp relocation sections: those which

 * reference symbols which live in vmlinux; and those which reference symbols
@@ -284,10 +292,10 @@ static int klp_resolve_symbols(Elf_Shdr *sechdrs, const char *strtab, 
 *    the to-be-patched module to be loaded and patched sometime *after* the

 *    klp module is loaded.

 */

int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,

static int klp_write_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,

				    const char *shstrtab, const char *strtab,

				    unsigned int symndx, unsigned int secndx,

			     const char *objname)

				    const char *objname, bool apply)

{

	int cnt, ret;

	char sec_objname[MODULE_NAME_LEN];
@@ -309,13 +317,28 @@ int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs, 
	if (strcmp(objname ? objname : "vmlinux", sec_objname))

		return 0;



	ret = klp_resolve_symbols(sechdrs, strtab, symndx, sec, sec_objname);

	if (apply) {

		ret = klp_resolve_symbols(sechdrs, strtab, symndx,

					  sec, sec_objname);

		if (ret)

			return ret;



		return apply_relocate_add(sechdrs, strtab, symndx, secndx, pmod);

	}



	clear_relocate_add(sechdrs, strtab, symndx, secndx, pmod);

	return 0;

}



int klp_apply_section_relocs(struct module *pmod, Elf_Shdr *sechdrs,

			     const char *shstrtab, const char *strtab,

			     unsigned int symndx, unsigned int secndx,

			     const char *objname)

{

	return klp_write_section_relocs(pmod, sechdrs, shstrtab, strtab, symndx,

					secndx, objname, true);

}



/*

 * Sysfs Interface

 *
@@ -762,8 +785,9 @@ static int klp_init_func(struct klp_object *obj, struct klp_func *func) 
			   func->old_sympos ? func->old_sympos : 1);

}



static int klp_apply_object_relocs(struct klp_patch *patch,

				   struct klp_object *obj)

static int klp_write_object_relocs(struct klp_patch *patch,

				   struct klp_object *obj,

				   bool apply)

{

	int i, ret;

	struct klp_modinfo *info = patch->mod->klp_info;
@@ -774,10 +798,10 @@ static int klp_apply_object_relocs(struct klp_patch *patch, 
		if (!(sec->sh_flags & SHF_RELA_LIVEPATCH))

			continue;



		ret = klp_apply_section_relocs(patch->mod, info->sechdrs,

		ret = klp_write_section_relocs(patch->mod, info->sechdrs,

					       info->secstrings,

					       patch->mod->core_kallsyms.strtab,

					       info->symndx, i, obj->name);

					       info->symndx, i, obj->name, apply);

		if (ret)

			return ret;

	}
@@ -785,6 +809,18 @@ static int klp_apply_object_relocs(struct klp_patch *patch, 
	return 0;

}



static int klp_apply_object_relocs(struct klp_patch *patch,

				   struct klp_object *obj)

{

	return klp_write_object_relocs(patch, obj, true);

}



static void klp_clear_object_relocs(struct klp_patch *patch,

				    struct klp_object *obj)

{

	klp_write_object_relocs(patch, obj, false);

}



/* parts of the initialization that is done only when the object is loaded */

static int klp_init_object_loaded(struct klp_patch *patch,

				  struct klp_object *obj)
@@ -1172,7 +1208,7 @@ static void klp_cleanup_module_patches_limited(struct module *mod, 
			klp_unpatch_object(obj);



			klp_post_unpatch_callback(obj);



			klp_clear_object_relocs(patch, obj);

			klp_free_object_loaded(obj);

			break;

		}