[PATCH 0/5] Support new MIPSr6 relocations

[PATCH 0/5] Support new MIPSr6 relocations

[Paul Burton]

MIPSr6 introduced a few new relocations that may be present in loadable
kernel modules. This series introduces support for them in both their
rel & rela forms for MIPS32 & MIPS64 kernels respectively, and ensures
that any future missing relocs cause module loading to fail gracefully.

Paul Burton (3):
  MIPS: Bail on unsupported module relocs
  MIPS: Support R_MIPS_PC16 rel-style reloc
  MIPS: Implement MIPSr6 R_MIPS_PC2x rel-style relocs

Steven J. Hill (2):
  MIPS: module-rela: Make consistent use of pr_*()
  MIPS: Add support for 64-bit R6 ELF relocations

 arch/mips/include/asm/elf.h    |  5 +++
 arch/mips/kernel/module-rela.c | 96 ++++++++++++++++++++++++++++++++++++++----
 arch/mips/kernel/module.c      | 85 +++++++++++++++++++++++++++++++++++--
 3 files changed, 173 insertions(+), 13 deletions(-)

-- 
2.7.0

[PATCH 1/5] MIPS: Bail on unsupported module relocs

[Paul Burton]

When an unsupported reloc is encountered in a module, we currently
blindly branch to whatever would be at its entry in the reloc handler
function pointer arrays. This may be NULL, or if the unsupported reloc
has a type greater than that of the supported reloc with the highest
type then we'll dereference some value after the function pointer array
& branch to that. The result is at best a kernel oops.

Fix this by checking that the reloc type has an entry in the function
pointer array (ie. is less than the number of items in the array) and
that the handler is non-NULL, returning an error code to fail the module
load if no handler is found.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
Cc: James Hogan <james.hogan@imgtec.com>
---

 arch/mips/kernel/module-rela.c | 19 ++++++++++++++++---
 arch/mips/kernel/module.c      | 19 ++++++++++++++++---
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/arch/mips/kernel/module-rela.c b/arch/mips/kernel/module-rela.c
index 2b70723..769e316 100644
--- a/arch/mips/kernel/module-rela.c
+++ b/arch/mips/kernel/module-rela.c
@@ -109,9 +109,10 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 		       struct module *me)
 {
 	Elf_Mips_Rela *rel = (void *) sechdrs[relsec].sh_addr;
+	int (*handler)(struct module *me, u32 *location, Elf_Addr v);
 	Elf_Sym *sym;
 	u32 *location;
-	unsigned int i;
+	unsigned int i, type;
 	Elf_Addr v;
 	int res;
 
@@ -134,9 +135,21 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 			return -ENOENT;
 		}
 
-		v = sym->st_value + rel[i].r_addend;
+		type = ELF_MIPS_R_TYPE(rel[i]);
+
+		if (type < ARRAY_SIZE(reloc_handlers_rela))
+			handler = reloc_handlers_rela[type];
+		else
+			handler = NULL;
 
-		res = reloc_handlers_rela[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
+		if (!handler) {
+			pr_warn("%s: Unknown relocation type %u\n",
+				me->name, type);
+			return -EINVAL;
+		}
+
+		v = sym->st_value + rel[i].r_addend;
+		res = handler(me, location, v);
 		if (res)
 			return res;
 	}
diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 1833f51..2adf572 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -197,9 +197,10 @@ int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
 		   struct module *me)
 {
 	Elf_Mips_Rel *rel = (void *) sechdrs[relsec].sh_addr;
+	int (*handler)(struct module *me, u32 *location, Elf_Addr v);
 	Elf_Sym *sym;
 	u32 *location;
-	unsigned int i;
+	unsigned int i, type;
 	Elf_Addr v;
 	int res;
 
@@ -223,9 +224,21 @@ int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
 			return -ENOENT;
 		}
 
-		v = sym->st_value;
+		type = ELF_MIPS_R_TYPE(rel[i]);
+
+		if (type < ARRAY_SIZE(reloc_handlers_rel))
+			handler = reloc_handlers_rel[type];
+		else
+			handler = NULL;
 
-		res = reloc_handlers_rel[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
+		if (!handler) {
+			pr_warn("%s: Unknown relocation type %u\n",
+				me->name, type);
+			return -EINVAL;
+		}
+
+		v = sym->st_value;
+		res = handler(me, location, v);
 		if (res)
 			return res;
 	}
-- 
2.7.0

[PATCH 2/5] MIPS: module-rela: Make consistent use of pr_*()

[Paul Burton]

From: "Steven J. Hill" <Steven.Hill@imgtec.com>

The module relocation handling code has inconsistent use of printk() and
pr_*() functions. Convert printk() calls to use pr_err() and pr_warn().

Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/kernel/module-rela.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/arch/mips/kernel/module-rela.c b/arch/mips/kernel/module-rela.c
index 769e316..f1ff64b 100644
--- a/arch/mips/kernel/module-rela.c
+++ b/arch/mips/kernel/module-rela.c
@@ -35,15 +35,13 @@ static int apply_r_mips_32_rela(struct module *me, u32 *location, Elf_Addr v)
 static int apply_r_mips_26_rela(struct module *me, u32 *location, Elf_Addr v)
 {
 	if (v % 4) {
-		pr_err("module %s: dangerous R_MIPS_26 RELArelocation\n",
+		pr_err("module %s: dangerous R_MIPS_26 RELA relocation\n",
 		       me->name);
 		return -ENOEXEC;
 	}
 
 	if ((v & 0xf0000000) != (((unsigned long)location + 4) & 0xf0000000)) {
-		printk(KERN_ERR
-		       "module %s: relocation overflow\n",
-		       me->name);
+		pr_err("module %s: relocation overflow\n", me->name);
 		return -ENOEXEC;
 	}
 
@@ -130,7 +128,7 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 			/* Ignore unresolved weak symbol */
 			if (ELF_ST_BIND(sym->st_info) == STB_WEAK)
 				continue;
-			printk(KERN_WARNING "%s: Unknown symbol %s\n",
+			pr_warn("%s: Unknown symbol %s\n",
 			       me->name, strtab + sym->st_name);
 			return -ENOENT;
 		}
-- 
2.7.0

[PATCH 3/5] MIPS: Add support for 64-bit R6 ELF relocations

[Paul Burton]

From: "Steven J. Hill" <Steven.Hill@imgtec.com>

This patch fixes MIPS64r6 kernel modules by adding new ELF
relocations. Toolchains that compile 64-bit R6 binaries emit
two new ELF relocations R_MIPS_PC21_S2 and R_MIPS_PC26_S2.
The pre-existing R_MIPS_PC16 ELF relocation is also emitted.

Signed-off-by: Steven J. Hill <Steven.Hill@imgtec.com>
Signed-off-by: James Hogan <james.hogan@imgtec.com>
Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/include/asm/elf.h    |  5 +++
 arch/mips/kernel/module-rela.c | 69 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 73 insertions(+), 1 deletion(-)

diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index cefb7a5..7cec234 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -111,6 +111,11 @@
 #define R_MIPS_CALLHI16		30
 #define R_MIPS_CALLLO16		31
 /*
+ * Introduced for MIPSr6.
+ */
+#define R_MIPS_PC21_S2		60
+#define R_MIPS_PC26_S2		61
+/*
  * This range is reserved for vendor specific relocations.
  */
 #define R_MIPS_LOVENDOR		100
diff --git a/arch/mips/kernel/module-rela.c b/arch/mips/kernel/module-rela.c
index f1ff64b..2dbe1b0 100644
--- a/arch/mips/kernel/module-rela.c
+++ b/arch/mips/kernel/module-rela.c
@@ -16,6 +16,7 @@
  *  Copyright (C) 2001 Rusty Russell.
  *  Copyright (C) 2003, 2004 Ralf Baechle (ralf@linux-mips.org)
  *  Copyright (C) 2005 Thiemo Seufer
+ *  Copyright (C) 2015 Imagination Technologies Ltd.
  */
 
 #include <linux/elf.h>
@@ -65,6 +66,27 @@ static int apply_r_mips_lo16_rela(struct module *me, u32 *location, Elf_Addr v)
 	return 0;
 }
 
+static int apply_r_mips_pc16_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+	long offset;
+
+	if (v % 4) {
+		pr_err("module %s: dangerous R_MIPS_PC16 RELA relocation\n",
+		       me->name);
+		return -ENOEXEC;
+	}
+
+	offset = ((long)v - (long)location) >> 2;
+	if (offset != (int16_t)offset) {
+		pr_err("module %s: relocation overflow\n", me->name);
+		return -ENOEXEC;
+	}
+
+	*location = (*location & ~0xffff) | (offset & 0xffff);
+
+	return 0;
+}
+
 static int apply_r_mips_64_rela(struct module *me, u32 *location, Elf_Addr v)
 {
 	*(Elf_Addr *)location = v;
@@ -90,6 +112,48 @@ static int apply_r_mips_highest_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_mips_pc21_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+	long offset;
+
+	if (v % 4) {
+		pr_err("module %s: dangerous R_MIPS_PC21 RELA relocation\n",
+		       me->name);
+		return -ENOEXEC;
+	}
+
+	offset = ((long)v - (long)location) >> 2;
+	if ((offset >> 20) > 0 || (offset >> 20) < -1) {
+		pr_err("module %s: relocation overflow\n", me->name);
+		return -ENOEXEC;
+	}
+
+	*location = (*location & ~0x001fffff) | (offset & 0x001fffff);
+
+	return 0;
+}
+
+static int apply_r_mips_pc26_rela(struct module *me, u32 *location, Elf_Addr v)
+{
+	long offset;
+
+	if (v % 4) {
+		pr_err("module %s: dangerous R_MIPS_PC26 RELA relocation\n",
+		       me->name);
+		return -ENOEXEC;
+	}
+
+	offset = ((long)v - (long)location) >> 2;
+	if ((offset >> 25) > 0 || (offset >> 25) < -1) {
+		pr_err("module %s: relocation overflow\n", me->name);
+		return -ENOEXEC;
+	}
+
+	*location = (*location & ~0x03ffffff) | (offset & 0x03ffffff);
+
+	return 0;
+}
+
 static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 				Elf_Addr v) = {
 	[R_MIPS_NONE]		= apply_r_mips_none,
@@ -97,9 +161,12 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 	[R_MIPS_26]		= apply_r_mips_26_rela,
 	[R_MIPS_HI16]		= apply_r_mips_hi16_rela,
 	[R_MIPS_LO16]		= apply_r_mips_lo16_rela,
+	[R_MIPS_PC16]		= apply_r_mips_pc16_rela,
 	[R_MIPS_64]		= apply_r_mips_64_rela,
 	[R_MIPS_HIGHER]		= apply_r_mips_higher_rela,
-	[R_MIPS_HIGHEST]	= apply_r_mips_highest_rela
+	[R_MIPS_HIGHEST]	= apply_r_mips_highest_rela,
+	[R_MIPS_PC21_S2]	= apply_r_mips_pc21_rela,
+	[R_MIPS_PC26_S2]	= apply_r_mips_pc26_rela,
 };
 
 int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
-- 
2.7.0

[PATCH 4/5] MIPS: Support R_MIPS_PC16 rel-style reloc

[Paul Burton]

MIPS32 code uses rel-style relocs, and MIPS32r6 modules may include the
R_MIPS_PC16 relocation. We thus need to support R_MIPS_PC16 rel-style
relocations in order to load MIPS32r6 kernel modules. This patch adds
such support, which is similar to the rela-style R_MIPS_PC16 support but
making use of the implicit addend from the instruction encoding.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 arch/mips/kernel/module.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index 2adf572..f2de9b8 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -183,13 +183,25 @@ out_danger:
 	return -ENOEXEC;
 }
 
+static int apply_r_mips_pc16_rel(struct module *me, u32 *location, Elf_Addr v)
+{
+	u16 val;
+
+	val = *location;
+	val += (v - (Elf_Addr)location) >> 2;
+	*location = (*location & 0xffff0000) | val;
+
+	return 0;
+}
+
 static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
 				Elf_Addr v) = {
 	[R_MIPS_NONE]		= apply_r_mips_none,
 	[R_MIPS_32]		= apply_r_mips_32_rel,
 	[R_MIPS_26]		= apply_r_mips_26_rel,
 	[R_MIPS_HI16]		= apply_r_mips_hi16_rel,
-	[R_MIPS_LO16]		= apply_r_mips_lo16_rel
+	[R_MIPS_LO16]		= apply_r_mips_lo16_rel,
+	[R_MIPS_PC16]		= apply_r_mips_pc16_rel,
 };
 
 int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
-- 
2.7.0

[PATCH 5/5] MIPS: Implement MIPSr6 R_MIPS_PC2x rel-style relocs

[Paul Burton]

MIPS32r6 code makes use of rel-stye relocations & may contain the new
relocations R_MIPS_PC21_S2 or R_MIPS_PC26_S2 which were introduced with
MIPSr6. Implement support for those relocations such that we can load
MIPS32r6 kernel modules.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>

---

 arch/mips/kernel/module.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
index f2de9b8..d62fd56 100644
--- a/arch/mips/kernel/module.c
+++ b/arch/mips/kernel/module.c
@@ -194,6 +194,56 @@ static int apply_r_mips_pc16_rel(struct module *me, u32 *location, Elf_Addr v)
 	return 0;
 }
 
+static int apply_r_mips_pc21_rel(struct module *me, u32 *location, Elf_Addr v)
+{
+	long offset;
+
+	if (v % 4) {
+		pr_err("module %s: dangerous R_MIPS_PC21 REL relocation\n",
+		       me->name);
+		return -ENOEXEC;
+	}
+
+	/* retrieve & sign extend implicit addend */
+	offset = *location & 0x1fffff;
+	offset |= (offset & BIT(20)) ? (~0l & ~0x1fffffl) : 0;
+
+	offset += ((long)v - (long)location) >> 2;
+	if ((offset >> 20) > 0 || (offset >> 20) < -1) {
+		pr_err("module %s: relocation overflow\n", me->name);
+		return -ENOEXEC;
+	}
+
+	*location = (*location & ~0x001fffff) | (offset & 0x001fffff);
+
+	return 0;
+}
+
+static int apply_r_mips_pc26_rel(struct module *me, u32 *location, Elf_Addr v)
+{
+	long offset;
+
+	if (v % 4) {
+		pr_err("module %s: dangerous R_MIPS_PC26 REL relocation\n",
+		       me->name);
+		return -ENOEXEC;
+	}
+
+	/* retrieve & sign extend implicit addend */
+	offset = *location & 0x3ffffff;
+	offset |= (offset & BIT(25)) ? (~0l & ~0x3ffffffl) : 0;
+
+	offset += ((long)v - (long)location) >> 2;
+	if ((offset >> 25) > 0 || (offset >> 25) < -1) {
+		pr_err("module %s: relocation overflow\n", me->name);
+		return -ENOEXEC;
+	}
+
+	*location = (*location & ~0x03ffffff) | (offset & 0x03ffffff);
+
+	return 0;
+}
+
 static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
 				Elf_Addr v) = {
 	[R_MIPS_NONE]		= apply_r_mips_none,
@@ -202,6 +252,8 @@ static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
 	[R_MIPS_HI16]		= apply_r_mips_hi16_rel,
 	[R_MIPS_LO16]		= apply_r_mips_lo16_rel,
 	[R_MIPS_PC16]		= apply_r_mips_pc16_rel,
+	[R_MIPS_PC21_S2]	= apply_r_mips_pc21_rel,
+	[R_MIPS_PC26_S2]	= apply_r_mips_pc26_rel,
 };
 
 int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
-- 
2.7.0

Re: [PATCH 4/5] MIPS: Support R_MIPS_PC16 rel-style reloc

[Sergei Shtylyov]

Hello.

On 2/3/2016 6:44 AM, Paul Burton wrote:

> MIPS32 code uses rel-style relocs, and MIPS32r6 modules may include the
> R_MIPS_PC16 relocation. We thus need to support R_MIPS_PC16 rel-style
> relocations in order to load MIPS32r6 kernel modules. This patch adds
> such support, which is similar to the rela-style R_MIPS_PC16 support but

     R_MIPS_LO16, you mean?

> making use of the implicit addend from the instruction encoding.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
>
>   arch/mips/kernel/module.c | 14 +++++++++++++-
>   1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
> index 2adf572..f2de9b8 100644
> --- a/arch/mips/kernel/module.c
> +++ b/arch/mips/kernel/module.c
> @@ -183,13 +183,25 @@ out_danger:
>   	return -ENOEXEC;
>   }
>
> +static int apply_r_mips_pc16_rel(struct module *me, u32 *location, Elf_Addr v)
> +{
> +	u16 val;
> +
> +	val = *location;
> +	val += (v - (Elf_Addr)location) >> 2;
> +	*location = (*location & 0xffff0000) | val;
> +
> +	return 0;
> +}
> +
>   static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
>   				Elf_Addr v) = {
>   	[R_MIPS_NONE]		= apply_r_mips_none,
>   	[R_MIPS_32]		= apply_r_mips_32_rel,
>   	[R_MIPS_26]		= apply_r_mips_26_rel,
>   	[R_MIPS_HI16]		= apply_r_mips_hi16_rel,
> -	[R_MIPS_LO16]		= apply_r_mips_lo16_rel
> +	[R_MIPS_LO16]		= apply_r_mips_lo16_rel,
> +	[R_MIPS_PC16]		= apply_r_mips_pc16_rel,
>   };
>
>   int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,

MBR, Sergei

Re: [PATCH 4/5] MIPS: Support R_MIPS_PC16 rel-style reloc

[Paul Burton]

On Wed, Feb 03, 2016 at 01:25:57PM +0300, Sergei Shtylyov wrote:
> Hello.
> 
> On 2/3/2016 6:44 AM, Paul Burton wrote:
> >MIPS32 code uses rel-style relocs, and MIPS32r6 modules may include the
> >R_MIPS_PC16 relocation. We thus need to support R_MIPS_PC16 rel-style
> >relocations in order to load MIPS32r6 kernel modules. This patch adds
> >such support, which is similar to the rela-style R_MIPS_PC16 support but
> 
>     R_MIPS_LO16, you mean?

Hi Sergei,

No, I mean it's similar to the R_MIPS_PC16 code in module-rela.c. That
is, its rela-style equivalent (rather than rel-style as here).

Thanks,
    Paul

Re: [PATCH 4/5] MIPS: Support R_MIPS_PC16 rel-style reloc

[Sergei Shtylyov]

On 2/3/2016 1:32 PM, Paul Burton wrote:

>>> MIPS32 code uses rel-style relocs, and MIPS32r6 modules may include the
>>> R_MIPS_PC16 relocation. We thus need to support R_MIPS_PC16 rel-style
>>> relocations in order to load MIPS32r6 kernel modules. This patch adds
>>> such support, which is similar to the rela-style R_MIPS_PC16 support but
>>
>>      R_MIPS_LO16, you mean?
>
> Hi Sergei,
>
> No, I mean it's similar to the R_MIPS_PC16 code in module-rela.c. That
> is, its rela-style equivalent (rather than rel-style as here).

    But you're *adding* R_MIPS_PC16, no?

> Thanks,
>      Paul

MBR, Sergei

Re: [PATCH 4/5] MIPS: Support R_MIPS_PC16 rel-style reloc

[Paul Burton]

On Wed, Feb 03, 2016 at 01:36:00PM +0300, Sergei Shtylyov wrote:
> On 2/3/2016 1:32 PM, Paul Burton wrote:
> 
> >>>MIPS32 code uses rel-style relocs, and MIPS32r6 modules may include the
> >>>R_MIPS_PC16 relocation. We thus need to support R_MIPS_PC16 rel-style
> >>>relocations in order to load MIPS32r6 kernel modules. This patch adds
> >>>such support, which is similar to the rela-style R_MIPS_PC16 support but
> >>
> >>     R_MIPS_LO16, you mean?
> >
> >Hi Sergei,
> >
> >No, I mean it's similar to the R_MIPS_PC16 code in module-rela.c. That
> >is, its rela-style equivalent (rather than rel-style as here).
> 
>    But you're *adding* R_MIPS_PC16, no?

Yup, this patch is adding R_MIPS_PC16 to module.c & it's similar to
R_MIPS_PC16 in module-rela.c.

(Incidentally I think we could tidy up the duplication between the two
files, but that can come later...)

Thanks,
    Paul

Re: [PATCH 1/5] MIPS: Bail on unsupported module relocs

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 3532 bytes --]

On Wed, Feb 03, 2016 at 03:44:41AM +0000, Paul Burton wrote:
> When an unsupported reloc is encountered in a module, we currently
> blindly branch to whatever would be at its entry in the reloc handler
> function pointer arrays. This may be NULL, or if the unsupported reloc
> has a type greater than that of the supported reloc with the highest
> type then we'll dereference some value after the function pointer array
> & branch to that. The result is at best a kernel oops.
> 
> Fix this by checking that the reloc type has an entry in the function
> pointer array (ie. is less than the number of items in the array) and
> that the handler is non-NULL, returning an error code to fail the module
> load if no handler is found.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> Cc: James Hogan <james.hogan@imgtec.com>

Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> ---
> 
>  arch/mips/kernel/module-rela.c | 19 ++++++++++++++++---
>  arch/mips/kernel/module.c      | 19 ++++++++++++++++---
>  2 files changed, 32 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/mips/kernel/module-rela.c b/arch/mips/kernel/module-rela.c
> index 2b70723..769e316 100644
> --- a/arch/mips/kernel/module-rela.c
> +++ b/arch/mips/kernel/module-rela.c
> @@ -109,9 +109,10 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>  		       struct module *me)
>  {
>  	Elf_Mips_Rela *rel = (void *) sechdrs[relsec].sh_addr;
> +	int (*handler)(struct module *me, u32 *location, Elf_Addr v);
>  	Elf_Sym *sym;
>  	u32 *location;
> -	unsigned int i;
> +	unsigned int i, type;
>  	Elf_Addr v;
>  	int res;
>  
> @@ -134,9 +135,21 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>  			return -ENOENT;
>  		}
>  
> -		v = sym->st_value + rel[i].r_addend;
> +		type = ELF_MIPS_R_TYPE(rel[i]);
> +
> +		if (type < ARRAY_SIZE(reloc_handlers_rela))
> +			handler = reloc_handlers_rela[type];
> +		else
> +			handler = NULL;
>  
> -		res = reloc_handlers_rela[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
> +		if (!handler) {
> +			pr_warn("%s: Unknown relocation type %u\n",
> +				me->name, type);
> +			return -EINVAL;
> +		}
> +
> +		v = sym->st_value + rel[i].r_addend;
> +		res = handler(me, location, v);
>  		if (res)
>  			return res;
>  	}
> diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
> index 1833f51..2adf572 100644
> --- a/arch/mips/kernel/module.c
> +++ b/arch/mips/kernel/module.c
> @@ -197,9 +197,10 @@ int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
>  		   struct module *me)
>  {
>  	Elf_Mips_Rel *rel = (void *) sechdrs[relsec].sh_addr;
> +	int (*handler)(struct module *me, u32 *location, Elf_Addr v);
>  	Elf_Sym *sym;
>  	u32 *location;
> -	unsigned int i;
> +	unsigned int i, type;
>  	Elf_Addr v;
>  	int res;
>  
> @@ -223,9 +224,21 @@ int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
>  			return -ENOENT;
>  		}
>  
> -		v = sym->st_value;
> +		type = ELF_MIPS_R_TYPE(rel[i]);
> +
> +		if (type < ARRAY_SIZE(reloc_handlers_rel))
> +			handler = reloc_handlers_rel[type];
> +		else
> +			handler = NULL;
>  
> -		res = reloc_handlers_rel[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
> +		if (!handler) {
> +			pr_warn("%s: Unknown relocation type %u\n",
> +				me->name, type);
> +			return -EINVAL;
> +		}
> +
> +		v = sym->st_value;
> +		res = handler(me, location, v);
>  		if (res)
>  			return res;
>  	}
> -- 
> 2.7.0
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/5] MIPS: Bail on unsupported module relocs

[Maciej W. Rozycki]

On Wed, 3 Feb 2016, Paul Burton wrote:

> --- a/arch/mips/kernel/module-rela.c
> +++ b/arch/mips/kernel/module-rela.c
> @@ -134,9 +135,21 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>  			return -ENOENT;
>  		}
>  
> -		v = sym->st_value + rel[i].r_addend;
> +		type = ELF_MIPS_R_TYPE(rel[i]);
> +
> +		if (type < ARRAY_SIZE(reloc_handlers_rela))
> +			handler = reloc_handlers_rela[type];
> +		else
> +			handler = NULL;
>  
> -		res = reloc_handlers_rela[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
> +		if (!handler) {
> +			pr_warn("%s: Unknown relocation type %u\n",
> +				me->name, type);
> +			return -EINVAL;

 Hmm, this looks like a fatal error condition to me, the module won't 
load.  Why `pr_warn' rather than `pr_err' then?  Likewise in the other 
file.

  Maciej

Re: [PATCH 5/5] MIPS: Implement MIPSr6 R_MIPS_PC2x rel-style relocs

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 2867 bytes --]

On Wed, Feb 03, 2016 at 03:44:45AM +0000, Paul Burton wrote:
> MIPS32r6 code makes use of rel-stye relocations & may contain the new
> relocations R_MIPS_PC21_S2 or R_MIPS_PC26_S2 which were introduced with
> MIPSr6. Implement support for those relocations such that we can load
> MIPS32r6 kernel modules.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>

Looks good to me,
Reviewed-by: James Hogan <james.hogan@imgtec.com>

Cheers
James

> 
> ---
> 
>  arch/mips/kernel/module.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
> index f2de9b8..d62fd56 100644
> --- a/arch/mips/kernel/module.c
> +++ b/arch/mips/kernel/module.c
> @@ -194,6 +194,56 @@ static int apply_r_mips_pc16_rel(struct module *me, u32 *location, Elf_Addr v)
>  	return 0;
>  }
>  
> +static int apply_r_mips_pc21_rel(struct module *me, u32 *location, Elf_Addr v)
> +{
> +	long offset;
> +
> +	if (v % 4) {
> +		pr_err("module %s: dangerous R_MIPS_PC21 REL relocation\n",
> +		       me->name);
> +		return -ENOEXEC;
> +	}
> +
> +	/* retrieve & sign extend implicit addend */
> +	offset = *location & 0x1fffff;
> +	offset |= (offset & BIT(20)) ? (~0l & ~0x1fffffl) : 0;
> +
> +	offset += ((long)v - (long)location) >> 2;
> +	if ((offset >> 20) > 0 || (offset >> 20) < -1) {
> +		pr_err("module %s: relocation overflow\n", me->name);
> +		return -ENOEXEC;
> +	}
> +
> +	*location = (*location & ~0x001fffff) | (offset & 0x001fffff);
> +
> +	return 0;
> +}
> +
> +static int apply_r_mips_pc26_rel(struct module *me, u32 *location, Elf_Addr v)
> +{
> +	long offset;
> +
> +	if (v % 4) {
> +		pr_err("module %s: dangerous R_MIPS_PC26 REL relocation\n",
> +		       me->name);
> +		return -ENOEXEC;
> +	}
> +
> +	/* retrieve & sign extend implicit addend */
> +	offset = *location & 0x3ffffff;
> +	offset |= (offset & BIT(25)) ? (~0l & ~0x3ffffffl) : 0;
> +
> +	offset += ((long)v - (long)location) >> 2;
> +	if ((offset >> 25) > 0 || (offset >> 25) < -1) {
> +		pr_err("module %s: relocation overflow\n", me->name);
> +		return -ENOEXEC;
> +	}
> +
> +	*location = (*location & ~0x03ffffff) | (offset & 0x03ffffff);
> +
> +	return 0;
> +}
> +
>  static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
>  				Elf_Addr v) = {
>  	[R_MIPS_NONE]		= apply_r_mips_none,
> @@ -202,6 +252,8 @@ static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
>  	[R_MIPS_HI16]		= apply_r_mips_hi16_rel,
>  	[R_MIPS_LO16]		= apply_r_mips_lo16_rel,
>  	[R_MIPS_PC16]		= apply_r_mips_pc16_rel,
> +	[R_MIPS_PC21_S2]	= apply_r_mips_pc21_rel,
> +	[R_MIPS_PC26_S2]	= apply_r_mips_pc26_rel,
>  };
>  
>  int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
> -- 
> 2.7.0
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 4/5] MIPS: Support R_MIPS_PC16 rel-style reloc

[James Hogan]

[-- Attachment #1: Type: text/plain, Size: 1719 bytes --]

On Wed, Feb 03, 2016 at 03:44:44AM +0000, Paul Burton wrote:
> MIPS32 code uses rel-style relocs, and MIPS32r6 modules may include the
> R_MIPS_PC16 relocation. We thus need to support R_MIPS_PC16 rel-style
> relocations in order to load MIPS32r6 kernel modules. This patch adds
> such support, which is similar to the rela-style R_MIPS_PC16 support but
> making use of the implicit addend from the instruction encoding.
> 
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
> 
>  arch/mips/kernel/module.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/mips/kernel/module.c b/arch/mips/kernel/module.c
> index 2adf572..f2de9b8 100644
> --- a/arch/mips/kernel/module.c
> +++ b/arch/mips/kernel/module.c
> @@ -183,13 +183,25 @@ out_danger:
>  	return -ENOEXEC;
>  }
>  
> +static int apply_r_mips_pc16_rel(struct module *me, u32 *location, Elf_Addr v)
> +{
> +	u16 val;
> +
> +	val = *location;
> +	val += (v - (Elf_Addr)location) >> 2;
> +	*location = (*location & 0xffff0000) | val;

Looks correct, but presumably this could benefit from some sanity
checking like the other patches.

Cheers
James

> +
> +	return 0;
> +}
> +
>  static int (*reloc_handlers_rel[]) (struct module *me, u32 *location,
>  				Elf_Addr v) = {
>  	[R_MIPS_NONE]		= apply_r_mips_none,
>  	[R_MIPS_32]		= apply_r_mips_32_rel,
>  	[R_MIPS_26]		= apply_r_mips_26_rel,
>  	[R_MIPS_HI16]		= apply_r_mips_hi16_rel,
> -	[R_MIPS_LO16]		= apply_r_mips_lo16_rel
> +	[R_MIPS_LO16]		= apply_r_mips_lo16_rel,
> +	[R_MIPS_PC16]		= apply_r_mips_pc16_rel,
>  };
>  
>  int apply_relocate(Elf_Shdr *sechdrs, const char *strtab,
> -- 
> 2.7.0
> 
> 

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

Re: [PATCH 1/5] MIPS: Bail on unsupported module relocs

[Paul Burton]

On Wed, Feb 03, 2016 at 12:24:38PM +0000, Maciej W. Rozycki wrote:
> On Wed, 3 Feb 2016, Paul Burton wrote:
> 
> > --- a/arch/mips/kernel/module-rela.c
> > +++ b/arch/mips/kernel/module-rela.c
> > @@ -134,9 +135,21 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
> >  			return -ENOENT;
> >  		}
> >  
> > -		v = sym->st_value + rel[i].r_addend;
> > +		type = ELF_MIPS_R_TYPE(rel[i]);
> > +
> > +		if (type < ARRAY_SIZE(reloc_handlers_rela))
> > +			handler = reloc_handlers_rela[type];
> > +		else
> > +			handler = NULL;
> >  
> > -		res = reloc_handlers_rela[ELF_MIPS_R_TYPE(rel[i])](me, location, v);
> > +		if (!handler) {
> > +			pr_warn("%s: Unknown relocation type %u\n",
> > +				me->name, type);
> > +			return -EINVAL;
> 
>  Hmm, this looks like a fatal error condition to me, the module won't 
> load.  Why `pr_warn' rather than `pr_err' then?  Likewise in the other 
> file.
> 
>   Maciej

Hi Maciej,

To me fatality implies death, and nothing dies here. The module isn't
loaded but that's done gracefully & is not likely due to an error in the
kernel - it's far more likely that the module isn't valid. So to me,
warning seems appropriate rather than implying an error in the kernel.

Having said that I think it's a non-issue & don't really care either
way, so if Ralf wants it to be pr_err fine.

Thanks,
    Paul

Re: [PATCH 1/5] MIPS: Bail on unsupported module relocs

[Maciej W. Rozycki]

On Wed, 3 Feb 2016, Paul Burton wrote:

> >  Hmm, this looks like a fatal error condition to me, the module won't 
> > load.  Why `pr_warn' rather than `pr_err' then?  Likewise in the other 
> > file.
> 
> To me fatality implies death, and nothing dies here. The module isn't
> loaded but that's done gracefully & is not likely due to an error in the
> kernel - it's far more likely that the module isn't valid. So to me,
> warning seems appropriate rather than implying an error in the kernel.

 It may be bikeshedding, however these levels affect what goes to syslog 
and the console.  There are `crit', `alert' and `emerg' levels above, to 
raise more severe conditions.  As to `warn' I'd expect one on a succesful 
action made with some limitations, e.g. a compatibility mode of some kind, 
running with a performance limitation, some functionality disabled, etc.  
There's also `notice', which is lower, I'd use for normal actions that 
might require operator's attention, e.g. I'd put switching a network 
interface into the promiscuous mode there, due to its side effect on 
overall system performance.

 And I don't think it has to be a bug in the kernel to raise an `err' 
condition.  However I do agree the boundary here may be a bit fuzzy and 
code you've been changing doesn't seem consistent either.

 FWIW,

  Maciej