Linux kbuild/kconfig development
 help / color / mirror / Atom feed
* [PATCH 1/3] modpost: factor out inst location calculation to section_rel()
@ 2023-06-20 12:05 Masahiro Yamada
  2023-06-20 12:05 ` [PATCH 2/3] modpost: factor out Elf_Sym pointer " Masahiro Yamada
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Masahiro Yamada @ 2023-06-20 12:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Nathan Chancellor,
	Nick Desaulniers, Nicolas Schier

All the addend_*_rel() functions calculate the instruction location in
the same way.

Factor out the similar code to the caller. Squash reloc_location() too.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 28 ++++++++++++----------------
 1 file changed, 12 insertions(+), 16 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 6e0b8be32648..2551ac9d5bd3 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1256,16 +1256,9 @@ static void check_section_mismatch(struct module *mod, struct elf_info *elf,
 				 tosec, taddr);
 }
 
-static unsigned int *reloc_location(struct elf_info *elf,
-				    Elf_Shdr *sechdr, Elf_Rela *r)
-{
-	return sym_get_data_by_offset(elf, sechdr->sh_info, r->r_offset);
-}
-
-static int addend_386_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
+static int addend_386_rel(uint32_t *location, Elf_Rela *r)
 {
 	unsigned int r_typ = ELF_R_TYPE(r->r_info);
-	unsigned int *location = reloc_location(elf, sechdr, r);
 
 	switch (r_typ) {
 	case R_386_32:
@@ -1302,11 +1295,10 @@ static int32_t sign_extend32(int32_t value, int index)
 	return (int32_t)(value << shift) >> shift;
 }
 
-static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
+static int addend_arm_rel(void *loc, struct elf_info *elf, Elf_Rela *r)
 {
 	unsigned int r_typ = ELF_R_TYPE(r->r_info);
 	Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
-	void *loc = reloc_location(elf, sechdr, r);
 	uint32_t inst, upper, lower, sign, j1, j2;
 	int32_t offset;
 
@@ -1396,11 +1388,10 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
 	return 0;
 }
 
-static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
+static int addend_mips_rel(uint32_t *location, Elf_Rela *r)
 {
 	unsigned int r_typ = ELF_R_TYPE(r->r_info);
-	unsigned int *location = reloc_location(elf, sechdr, r);
-	unsigned int inst;
+	uint32_t inst;
 
 	if (r_typ == R_MIPS_HI16)
 		return 1;	/* skip this */
@@ -1502,6 +1493,8 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 		return;
 
 	for (rel = start; rel < stop; rel++) {
+		void *loc;
+
 		r.r_offset = TO_NATIVE(rel->r_offset);
 #if KERNEL_ELFCLASS == ELFCLASS64
 		if (elf->hdr->e_machine == EM_MIPS) {
@@ -1519,17 +1512,20 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 		r_sym = ELF_R_SYM(r.r_info);
 #endif
 		r.r_addend = 0;
+
+		loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);
+
 		switch (elf->hdr->e_machine) {
 		case EM_386:
-			if (addend_386_rel(elf, sechdr, &r))
+			if (addend_386_rel(loc, &r))
 				continue;
 			break;
 		case EM_ARM:
-			if (addend_arm_rel(elf, sechdr, &r))
+			if (addend_arm_rel(loc, elf, &r))
 				continue;
 			break;
 		case EM_MIPS:
-			if (addend_mips_rel(elf, sechdr, &r))
+			if (addend_mips_rel(loc, &r))
 				continue;
 			break;
 		default:
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/3] modpost: factor out Elf_Sym pointer calculation to section_rel()
  2023-06-20 12:05 [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Masahiro Yamada
@ 2023-06-20 12:05 ` Masahiro Yamada
  2023-06-22 19:01   ` Nick Desaulniers
  2023-06-20 12:05 ` [PATCH 3/3] modpost: continue even with unknown relocation type Masahiro Yamada
  2023-06-22 18:25 ` [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Nick Desaulniers
  2 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2023-06-20 12:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Nathan Chancellor,
	Nick Desaulniers, Nicolas Schier

Pass the Elf_Sym pointer to addend_arm_rel() as well as to
check_section_mismatch().

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 2551ac9d5bd3..ffe45c54f024 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1295,10 +1295,9 @@ static int32_t sign_extend32(int32_t value, int index)
 	return (int32_t)(value << shift) >> shift;
 }
 
-static int addend_arm_rel(void *loc, struct elf_info *elf, Elf_Rela *r)
+static int addend_arm_rel(void *loc, Elf_Sym *sym, Elf_Rela *r)
 {
 	unsigned int r_typ = ELF_R_TYPE(r->r_info);
-	Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
 	uint32_t inst, upper, lower, sign, j1, j2;
 	int32_t offset;
 
@@ -1493,6 +1492,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 		return;
 
 	for (rel = start; rel < stop; rel++) {
+		Elf_Sym *tsym;
 		void *loc;
 
 		r.r_offset = TO_NATIVE(rel->r_offset);
@@ -1514,6 +1514,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 		r.r_addend = 0;
 
 		loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);
+		tsym = elf->symtab_start + ELF_R_SYM(r.r_info);
 
 		switch (elf->hdr->e_machine) {
 		case EM_386:
@@ -1521,7 +1522,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 				continue;
 			break;
 		case EM_ARM:
-			if (addend_arm_rel(loc, elf, &r))
+			if (addend_arm_rel(loc, tsym, &r))
 				continue;
 			break;
 		case EM_MIPS:
@@ -1532,7 +1533,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 			fatal("Please add code to calculate addend for this architecture\n");
 		}
 
-		check_section_mismatch(mod, elf, elf->symtab_start + r_sym,
+		check_section_mismatch(mod, elf, tsym,
 				       fsecndx, fromsec, r.r_offset, r.r_addend);
 	}
 }
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/3] modpost: continue even with unknown relocation type
  2023-06-20 12:05 [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Masahiro Yamada
  2023-06-20 12:05 ` [PATCH 2/3] modpost: factor out Elf_Sym pointer " Masahiro Yamada
@ 2023-06-20 12:05 ` Masahiro Yamada
  2023-06-22 18:25 ` [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Nick Desaulniers
  2 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2023-06-20 12:05 UTC (permalink / raw)
  To: linux-kbuild
  Cc: linux-kernel, Masahiro Yamada, Nathan Chancellor,
	Nick Desaulniers, Nicolas Schier

Currently, unknown relocation types are just skipped.

The value of r_addend is only needed to get the symbol name in case
is_valid_name(elf, sym) returns false.

Even if we do not know how to calculate r_addend, we should continue.
At worst, we will get "(unknown)" as the symbol name, but it is better
than failing to detect section mismatches.

Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
---

 scripts/mod/modpost.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index ffe45c54f024..7e92380d4f1f 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1267,6 +1267,8 @@ static int addend_386_rel(uint32_t *location, Elf_Rela *r)
 	case R_386_PC32:
 		r->r_addend = TO_NATIVE(*location) + 4;
 		break;
+	default:
+		r->r_addend = (Elf_Addr)(-1);
 	}
 	return 0;
 }
@@ -1382,7 +1384,7 @@ static int addend_arm_rel(void *loc, Elf_Sym *sym, Elf_Rela *r)
 		r->r_addend = offset + sym->st_value + 4;
 		break;
 	default:
-		return 1;
+		r->r_addend = (Elf_Addr)(-1);
 	}
 	return 0;
 }
@@ -1392,8 +1394,6 @@ static int addend_mips_rel(uint32_t *location, Elf_Rela *r)
 	unsigned int r_typ = ELF_R_TYPE(r->r_info);
 	uint32_t inst;
 
-	if (r_typ == R_MIPS_HI16)
-		return 1;	/* skip this */
 	inst = TO_NATIVE(*location);
 	switch (r_typ) {
 	case R_MIPS_LO16:
@@ -1405,6 +1405,8 @@ static int addend_mips_rel(uint32_t *location, Elf_Rela *r)
 	case R_MIPS_32:
 		r->r_addend = inst;
 		break;
+	default:
+		r->r_addend = (Elf_Addr)(-1);
 	}
 	return 0;
 }
@@ -1514,20 +1516,17 @@ static void section_rel(struct module *mod, struct elf_info *elf,
 		r.r_addend = 0;
 
 		loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);
-		tsym = elf->symtab_start + ELF_R_SYM(r.r_info);
+		tsym = elf->symtab_start + r_sym;
 
 		switch (elf->hdr->e_machine) {
 		case EM_386:
-			if (addend_386_rel(loc, &r))
-				continue;
+			addend_386_rel(loc, &r);
 			break;
 		case EM_ARM:
-			if (addend_arm_rel(loc, tsym, &r))
-				continue;
+			addend_arm_rel(loc, tsym, &r);
 			break;
 		case EM_MIPS:
-			if (addend_mips_rel(loc, &r))
-				continue;
+			addend_mips_rel(loc, &r);
 			break;
 		default:
 			fatal("Please add code to calculate addend for this architecture\n");
-- 
2.39.2


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] modpost: factor out inst location calculation to section_rel()
  2023-06-20 12:05 [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Masahiro Yamada
  2023-06-20 12:05 ` [PATCH 2/3] modpost: factor out Elf_Sym pointer " Masahiro Yamada
  2023-06-20 12:05 ` [PATCH 3/3] modpost: continue even with unknown relocation type Masahiro Yamada
@ 2023-06-22 18:25 ` Nick Desaulniers
  2023-06-23  5:37   ` Masahiro Yamada
  2 siblings, 1 reply; 8+ messages in thread
From: Nick Desaulniers @ 2023-06-22 18:25 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier

On Tue, Jun 20, 2023 at 5:05 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> All the addend_*_rel() functions calculate the instruction location in
> the same way.
>
> Factor out the similar code to the caller. Squash reloc_location() too.
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> ---
>
>  scripts/mod/modpost.c | 28 ++++++++++++----------------
>  1 file changed, 12 insertions(+), 16 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 6e0b8be32648..2551ac9d5bd3 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1256,16 +1256,9 @@ static void check_section_mismatch(struct module *mod, struct elf_info *elf,
>                                  tosec, taddr);
>  }
>
> -static unsigned int *reloc_location(struct elf_info *elf,
> -                                   Elf_Shdr *sechdr, Elf_Rela *r)
> -{
> -       return sym_get_data_by_offset(elf, sechdr->sh_info, r->r_offset);
> -}
> -
> -static int addend_386_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> +static int addend_386_rel(uint32_t *location, Elf_Rela *r)
>  {
>         unsigned int r_typ = ELF_R_TYPE(r->r_info);
> -       unsigned int *location = reloc_location(elf, sechdr, r);
>
>         switch (r_typ) {
>         case R_386_32:
> @@ -1302,11 +1295,10 @@ static int32_t sign_extend32(int32_t value, int index)
>         return (int32_t)(value << shift) >> shift;
>  }
>
> -static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> +static int addend_arm_rel(void *loc, struct elf_info *elf, Elf_Rela *r)
>  {
>         unsigned int r_typ = ELF_R_TYPE(r->r_info);
>         Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> -       void *loc = reloc_location(elf, sechdr, r);
>         uint32_t inst, upper, lower, sign, j1, j2;
>         int32_t offset;
>
> @@ -1396,11 +1388,10 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
>         return 0;
>  }
>
> -static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> +static int addend_mips_rel(uint32_t *location, Elf_Rela *r)
>  {
>         unsigned int r_typ = ELF_R_TYPE(r->r_info);
> -       unsigned int *location = reloc_location(elf, sechdr, r);
> -       unsigned int inst;
> +       uint32_t inst;
>
>         if (r_typ == R_MIPS_HI16)
>                 return 1;       /* skip this */
> @@ -1502,6 +1493,8 @@ static void section_rel(struct module *mod, struct elf_info *elf,
>                 return;
>
>         for (rel = start; rel < stop; rel++) {
> +               void *loc;
> +
>                 r.r_offset = TO_NATIVE(rel->r_offset);
>  #if KERNEL_ELFCLASS == ELFCLASS64
>                 if (elf->hdr->e_machine == EM_MIPS) {
> @@ -1519,17 +1512,20 @@ static void section_rel(struct module *mod, struct elf_info *elf,
>                 r_sym = ELF_R_SYM(r.r_info);
>  #endif
>                 r.r_addend = 0;
> +
> +               loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);

Can we compute `loc` only for the three machine types?

> +
>                 switch (elf->hdr->e_machine) {
>                 case EM_386:
> -                       if (addend_386_rel(elf, sechdr, &r))
> +                       if (addend_386_rel(loc, &r))
>                                 continue;
>                         break;
>                 case EM_ARM:
> -                       if (addend_arm_rel(elf, sechdr, &r))
> +                       if (addend_arm_rel(loc, elf, &r))
>                                 continue;
>                         break;
>                 case EM_MIPS:
> -                       if (addend_mips_rel(elf, sechdr, &r))
> +                       if (addend_mips_rel(loc, &r))
>                                 continue;
>                         break;
>                 default:
> --
> 2.39.2
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 2/3] modpost: factor out Elf_Sym pointer calculation to section_rel()
  2023-06-20 12:05 ` [PATCH 2/3] modpost: factor out Elf_Sym pointer " Masahiro Yamada
@ 2023-06-22 19:01   ` Nick Desaulniers
  0 siblings, 0 replies; 8+ messages in thread
From: Nick Desaulniers @ 2023-06-22 19:01 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier

On Tue, Jun 20, 2023 at 5:05 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> Pass the Elf_Sym pointer to addend_arm_rel() as well as to
> check_section_mismatch().
>
> Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>

Thanks for the patch!
Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>

> ---
>
>  scripts/mod/modpost.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> index 2551ac9d5bd3..ffe45c54f024 100644
> --- a/scripts/mod/modpost.c
> +++ b/scripts/mod/modpost.c
> @@ -1295,10 +1295,9 @@ static int32_t sign_extend32(int32_t value, int index)
>         return (int32_t)(value << shift) >> shift;
>  }
>
> -static int addend_arm_rel(void *loc, struct elf_info *elf, Elf_Rela *r)
> +static int addend_arm_rel(void *loc, Elf_Sym *sym, Elf_Rela *r)
>  {
>         unsigned int r_typ = ELF_R_TYPE(r->r_info);
> -       Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
>         uint32_t inst, upper, lower, sign, j1, j2;
>         int32_t offset;
>
> @@ -1493,6 +1492,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
>                 return;
>
>         for (rel = start; rel < stop; rel++) {
> +               Elf_Sym *tsym;
>                 void *loc;
>
>                 r.r_offset = TO_NATIVE(rel->r_offset);
> @@ -1514,6 +1514,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
>                 r.r_addend = 0;
>
>                 loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);
> +               tsym = elf->symtab_start + ELF_R_SYM(r.r_info);
>
>                 switch (elf->hdr->e_machine) {
>                 case EM_386:
> @@ -1521,7 +1522,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
>                                 continue;
>                         break;
>                 case EM_ARM:
> -                       if (addend_arm_rel(loc, elf, &r))
> +                       if (addend_arm_rel(loc, tsym, &r))
>                                 continue;
>                         break;
>                 case EM_MIPS:
> @@ -1532,7 +1533,7 @@ static void section_rel(struct module *mod, struct elf_info *elf,
>                         fatal("Please add code to calculate addend for this architecture\n");
>                 }
>
> -               check_section_mismatch(mod, elf, elf->symtab_start + r_sym,
> +               check_section_mismatch(mod, elf, tsym,
>                                        fsecndx, fromsec, r.r_offset, r.r_addend);
>         }
>  }
> --
> 2.39.2
>


-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] modpost: factor out inst location calculation to section_rel()
  2023-06-22 18:25 ` [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Nick Desaulniers
@ 2023-06-23  5:37   ` Masahiro Yamada
  2023-06-23 17:00     ` Nick Desaulniers
  0 siblings, 1 reply; 8+ messages in thread
From: Masahiro Yamada @ 2023-06-23  5:37 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier

On Fri, Jun 23, 2023 at 3:25 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Tue, Jun 20, 2023 at 5:05 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > All the addend_*_rel() functions calculate the instruction location in
> > the same way.
> >
> > Factor out the similar code to the caller. Squash reloc_location() too.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> >  scripts/mod/modpost.c | 28 ++++++++++++----------------
> >  1 file changed, 12 insertions(+), 16 deletions(-)
> >
> > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > index 6e0b8be32648..2551ac9d5bd3 100644
> > --- a/scripts/mod/modpost.c
> > +++ b/scripts/mod/modpost.c
> > @@ -1256,16 +1256,9 @@ static void check_section_mismatch(struct module *mod, struct elf_info *elf,
> >                                  tosec, taddr);
> >  }
> >
> > -static unsigned int *reloc_location(struct elf_info *elf,
> > -                                   Elf_Shdr *sechdr, Elf_Rela *r)
> > -{
> > -       return sym_get_data_by_offset(elf, sechdr->sh_info, r->r_offset);
> > -}
> > -
> > -static int addend_386_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > +static int addend_386_rel(uint32_t *location, Elf_Rela *r)
> >  {
> >         unsigned int r_typ = ELF_R_TYPE(r->r_info);
> > -       unsigned int *location = reloc_location(elf, sechdr, r);
> >
> >         switch (r_typ) {
> >         case R_386_32:
> > @@ -1302,11 +1295,10 @@ static int32_t sign_extend32(int32_t value, int index)
> >         return (int32_t)(value << shift) >> shift;
> >  }
> >
> > -static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > +static int addend_arm_rel(void *loc, struct elf_info *elf, Elf_Rela *r)
> >  {
> >         unsigned int r_typ = ELF_R_TYPE(r->r_info);
> >         Elf_Sym *sym = elf->symtab_start + ELF_R_SYM(r->r_info);
> > -       void *loc = reloc_location(elf, sechdr, r);
> >         uint32_t inst, upper, lower, sign, j1, j2;
> >         int32_t offset;
> >
> > @@ -1396,11 +1388,10 @@ static int addend_arm_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> >         return 0;
> >  }
> >
> > -static int addend_mips_rel(struct elf_info *elf, Elf_Shdr *sechdr, Elf_Rela *r)
> > +static int addend_mips_rel(uint32_t *location, Elf_Rela *r)
> >  {
> >         unsigned int r_typ = ELF_R_TYPE(r->r_info);
> > -       unsigned int *location = reloc_location(elf, sechdr, r);
> > -       unsigned int inst;
> > +       uint32_t inst;
> >
> >         if (r_typ == R_MIPS_HI16)
> >                 return 1;       /* skip this */
> > @@ -1502,6 +1493,8 @@ static void section_rel(struct module *mod, struct elf_info *elf,
> >                 return;
> >
> >         for (rel = start; rel < stop; rel++) {
> > +               void *loc;
> > +
> >                 r.r_offset = TO_NATIVE(rel->r_offset);
> >  #if KERNEL_ELFCLASS == ELFCLASS64
> >                 if (elf->hdr->e_machine == EM_MIPS) {
> > @@ -1519,17 +1512,20 @@ static void section_rel(struct module *mod, struct elf_info *elf,
> >                 r_sym = ELF_R_SYM(r.r_info);
> >  #endif
> >                 r.r_addend = 0;
> > +
> > +               loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);
>
> Can we compute `loc` only for the three machine types?



I believe you can compute the location in the same way for any architecture
because it is mentioned in ELF spec.

https://refspecs.linuxfoundation.org/elf/elf.pdf

Page 36.


r_offset
  This member gives the location at which to apply the relocation action. For
  a relocatable file, the value is the byte offset from the beginning of the
  section to the storage unit affected by the relocation.







-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] modpost: factor out inst location calculation to section_rel()
  2023-06-23  5:37   ` Masahiro Yamada
@ 2023-06-23 17:00     ` Nick Desaulniers
  2023-06-24  8:55       ` Masahiro Yamada
  0 siblings, 1 reply; 8+ messages in thread
From: Nick Desaulniers @ 2023-06-23 17:00 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier

On Thu, Jun 22, 2023 at 10:38 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
>
> On Fri, Jun 23, 2023 at 3:25 AM Nick Desaulniers
> <ndesaulniers@google.com> wrote:
> >
> > On Tue, Jun 20, 2023 at 5:05 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > >
> > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > index 6e0b8be32648..2551ac9d5bd3 100644
> > > --- a/scripts/mod/modpost.c
> > > +++ b/scripts/mod/modpost.c
> > > @@ -1519,17 +1512,20 @@ static void section_rel(struct module *mod, struct elf_info *elf,
> > >                 r_sym = ELF_R_SYM(r.r_info);
> > >  #endif
> > >                 r.r_addend = 0;
> > > +
> > > +               loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);
> >
> > Can we compute `loc` only for the three machine types?
>
>
>
> I believe you can compute the location in the same way for any architecture
> because it is mentioned in ELF spec.

Sure, but perhaps it's wasted work for other machine types?
-- 
Thanks,
~Nick Desaulniers

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/3] modpost: factor out inst location calculation to section_rel()
  2023-06-23 17:00     ` Nick Desaulniers
@ 2023-06-24  8:55       ` Masahiro Yamada
  0 siblings, 0 replies; 8+ messages in thread
From: Masahiro Yamada @ 2023-06-24  8:55 UTC (permalink / raw)
  To: Nick Desaulniers
  Cc: linux-kbuild, linux-kernel, Nathan Chancellor, Nicolas Schier

On Sat, Jun 24, 2023 at 2:01 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Thu, Jun 22, 2023 at 10:38 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > On Fri, Jun 23, 2023 at 3:25 AM Nick Desaulniers
> > <ndesaulniers@google.com> wrote:
> > >
> > > On Tue, Jun 20, 2023 at 5:05 AM Masahiro Yamada <masahiroy@kernel.org> wrote:
> > > >
> > > > diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
> > > > index 6e0b8be32648..2551ac9d5bd3 100644
> > > > --- a/scripts/mod/modpost.c
> > > > +++ b/scripts/mod/modpost.c
> > > > @@ -1519,17 +1512,20 @@ static void section_rel(struct module *mod, struct elf_info *elf,
> > > >                 r_sym = ELF_R_SYM(r.r_info);
> > > >  #endif
> > > >                 r.r_addend = 0;
> > > > +
> > > > +               loc = sym_get_data_by_offset(elf, fsecndx, r.r_offset);
> > >
> > > Can we compute `loc` only for the three machine types?
> >
> >
> >
> > I believe you can compute the location in the same way for any architecture
> > because it is mentioned in ELF spec.
>
> Sure, but perhaps it's wasted work for other machine types?



I guess you missed the following code:


switch (elf->hdr->e_machine) {
case EM_386:
        ...
        break;
case EM_ARM:
        ...
        break;
case EM_MIPS:
        ...
        break;
default:
        fatal("Please add code to calculate addend for this architecture\n");
}




I believe other machines never call this function.
If it occurred, fatal() would immediately errors out,
but I have not heard such a breakage for far.

I believe only i386, mips and arm use REL.
The other architectures are RELA.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2023-06-24  8:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-20 12:05 [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Masahiro Yamada
2023-06-20 12:05 ` [PATCH 2/3] modpost: factor out Elf_Sym pointer " Masahiro Yamada
2023-06-22 19:01   ` Nick Desaulniers
2023-06-20 12:05 ` [PATCH 3/3] modpost: continue even with unknown relocation type Masahiro Yamada
2023-06-22 18:25 ` [PATCH 1/3] modpost: factor out inst location calculation to section_rel() Nick Desaulniers
2023-06-23  5:37   ` Masahiro Yamada
2023-06-23 17:00     ` Nick Desaulniers
2023-06-24  8:55       ` Masahiro Yamada

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox