From d3cac35cd0a2a987f7559e1829fb0253cea33872 Mon Sep 17 00:00:00 2001 From: Ralf Baechle Date: Wed, 8 Aug 2012 14:57:03 +0200 Subject: MIPS: Fix memory leak in error path of HI16/LO16 relocation handling. Commit 6f5d2e970452b5c86906adcb8e7ad246f535ba39 (lmo) / 477c4b07406357ad93d0e32788dbf3ee814eadaa (kernel.org) [[MIPS: VPE: Free relocation chain on error.] fixed the same issue in the vpe loader in 2009 but back then the same bug in module.c went unfixed. Signed-off-by: Ralf Baechle Reported-by: Akhilesh Kumar --- arch/mips/kernel/module.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) (limited to 'arch/mips/kernel/module.c') diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c index a5066b1c3de..e5f2f56524e 100644 --- a/arch/mips/kernel/module.c +++ b/arch/mips/kernel/module.c @@ -146,16 +146,15 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) { unsigned long insnlo = *location; Elf_Addr val, vallo; + struct mips_hi16 *l, *next; /* Sign extend the addend we extract from the lo insn. */ vallo = ((insnlo & 0xffff) ^ 0x8000) - 0x8000; if (mips_hi16_list != NULL) { - struct mips_hi16 *l; l = mips_hi16_list; while (l != NULL) { - struct mips_hi16 *next; unsigned long insn; /* @@ -201,6 +200,12 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) return 0; out_danger: + while (l) { + next = l->next; + kfree(l); + l = next; + } + pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); return -ENOEXEC; -- cgit v1.2.3-70-g09d2 From 861667dc82f561e65336ea67f73021b782b4ff74 Mon Sep 17 00:00:00 2001 From: Ralf Baechle Date: Wed, 8 Aug 2012 16:59:43 +0200 Subject: MIPS: Fix race condition in module relocation code. The relocation code was essentially taken from the 2.4 modutils which perform relocation in userspace. In 2.6 relocation of multiple modules may be performed in parallel by the in-kernel loader so the global variable mips_hi16_list won't fly anymore. Fix race by moving it into mod_arch_specific. [ralf@linux-mips.org: folded in Tony's followup fix. Thanks Tony!] Signed-off-by: Ralf Baechle Signed-off-by: Tony Wu Cc: linux-mips@linux-mips.org Patchwork: http://patchwork.linux-mips.org/patch/4189/ --- arch/mips/include/asm/module.h | 1 + arch/mips/kernel/module.c | 13 ++++++------- 2 files changed, 7 insertions(+), 7 deletions(-) (limited to 'arch/mips/kernel/module.c') diff --git a/arch/mips/include/asm/module.h b/arch/mips/include/asm/module.h index 7531ecd654d..dca8bce8c7a 100644 --- a/arch/mips/include/asm/module.h +++ b/arch/mips/include/asm/module.h @@ -10,6 +10,7 @@ struct mod_arch_specific { struct list_head dbe_list; const struct exception_table_entry *dbe_start; const struct exception_table_entry *dbe_end; + struct mips_hi16 *r_mips_hi16_list; }; typedef uint8_t Elf64_Byte; /* Type for a 8-bit quantity. */ diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c index e5f2f56524e..8e1fb802c3e 100644 --- a/arch/mips/kernel/module.c +++ b/arch/mips/kernel/module.c @@ -39,8 +39,6 @@ struct mips_hi16 { Elf_Addr value; }; -static struct mips_hi16 *mips_hi16_list; - static LIST_HEAD(dbe_list); static DEFINE_SPINLOCK(dbe_lock); @@ -128,8 +126,8 @@ static int apply_r_mips_hi16_rel(struct module *me, u32 *location, Elf_Addr v) n->addr = (Elf_Addr *)location; n->value = v; - n->next = mips_hi16_list; - mips_hi16_list = n; + n->next = me->arch.r_mips_hi16_list; + me->arch.r_mips_hi16_list = n; return 0; } @@ -151,9 +149,9 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) /* Sign extend the addend we extract from the lo insn. */ vallo = ((insnlo & 0xffff) ^ 0x8000) - 0x8000; - if (mips_hi16_list != NULL) { + if (me->arch.r_mips_hi16_list != NULL) { - l = mips_hi16_list; + l = me->arch.r_mips_hi16_list; while (l != NULL) { unsigned long insn; @@ -187,7 +185,7 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) l = next; } - mips_hi16_list = NULL; + me->arch.r_mips_hi16_list = NULL; } /* @@ -278,6 +276,7 @@ int apply_relocate(Elf_Shdr *sechdrs, const char *strtab, pr_debug("Applying relocate section %u to %u\n", relsec, sechdrs[relsec].sh_info); + me->arch.r_mips_hi16_list = NULL; for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) { /* This is where to make the change */ location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr -- cgit v1.2.3-70-g09d2 From c54de490a2e4e74164f747925ff05c00dfa153cd Mon Sep 17 00:00:00 2001 From: Ralf Baechle Date: Tue, 14 Aug 2012 00:34:18 +0200 Subject: MIPS: Module: Deal with malformed HI16/LO16 relocation sequences. In case a series of R_MIPS_HI16 relocations was not followed by an R_MIPS_LO16 relocation we were leaking the hi16 relocation chain. Handle that error and return an error. Signed-off-by: Ralf Baechle --- arch/mips/kernel/module.c | 35 ++++++++++++++++++++++++++++------- 1 file changed, 28 insertions(+), 7 deletions(-) (limited to 'arch/mips/kernel/module.c') diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c index 8e1fb802c3e..4f8c3cba8c0 100644 --- a/arch/mips/kernel/module.c +++ b/arch/mips/kernel/module.c @@ -140,19 +140,30 @@ static int apply_r_mips_hi16_rela(struct module *me, u32 *location, Elf_Addr v) return 0; } +static void free_relocation_chain(struct mips_hi16 *l) +{ + struct mips_hi16 *next; + + while (l) { + next = l->next; + kfree(l); + l = next; + } +} + static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) { unsigned long insnlo = *location; + struct mips_hi16 *l; Elf_Addr val, vallo; - struct mips_hi16 *l, *next; /* Sign extend the addend we extract from the lo insn. */ vallo = ((insnlo & 0xffff) ^ 0x8000) - 0x8000; if (me->arch.r_mips_hi16_list != NULL) { - l = me->arch.r_mips_hi16_list; while (l != NULL) { + struct mips_hi16 *next; unsigned long insn; /* @@ -198,11 +209,8 @@ static int apply_r_mips_lo16_rel(struct module *me, u32 *location, Elf_Addr v) return 0; out_danger: - while (l) { - next = l->next; - kfree(l); - l = next; - } + free_relocation_chain(l); + me->arch.r_mips_hi16_list = NULL; pr_err("module %s: dangerous R_MIPS_LO16 REL relocation\n", me->name); @@ -300,6 +308,19 @@ int apply_relocate(Elf_Shdr *sechdrs, const char *strtab, return res; } + /* + * Normally the hi16 list should be deallocated at this point. A + * malformed binary however could contain a series of R_MIPS_HI16 + * relocations not followed by a R_MIPS_LO16 relocation. In that + * case, free up the list and return an error. + */ + if (me->arch.r_mips_hi16_list) { + free_relocation_chain(me->arch.r_mips_hi16_list); + me->arch.r_mips_hi16_list = NULL; + + return -ENOEXEC; + } + return 0; } -- cgit v1.2.3-70-g09d2