All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/tools/relocs: fix big section header tables
@ 2018-11-29 13:51 Artem Savkov
  2018-11-29 14:23 ` Josh Poimboeuf
  0 siblings, 1 reply; 8+ messages in thread
From: Artem Savkov @ 2018-11-29 13:51 UTC (permalink / raw)
  To: x86; +Cc: Thomas Gleixner, Eric W . Biederman, linux-kernel, Artem Savkov

In case when the number of entries in the section header table is larger
then or equal to SHN_LORESERVE the size of the table is held in the sh_size
member of the initial entry in section header table instead of e_shnum.
Same with the string table index which is located in sh_link instead of
e_shstrndx.

This case is easily reproducible with KCFLAGS="-ffunction-sections",
bzImage build fails with "String table index out of bounds" error.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
 arch/x86/tools/relocs.c | 58 +++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index b629f6992d9f..5275ea0a0d13 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -11,7 +11,9 @@
 #define Elf_Shdr		ElfW(Shdr)
 #define Elf_Sym			ElfW(Sym)
 
-static Elf_Ehdr ehdr;
+static Elf_Ehdr	ehdr;
+static unsigned long	shnum;
+static unsigned int	shstrndx;
 
 struct relocs {
 	uint32_t	*offset;
@@ -241,9 +243,9 @@ static const char *sec_name(unsigned shndx)
 {
 	const char *sec_strtab;
 	const char *name;
-	sec_strtab = secs[ehdr.e_shstrndx].strtab;
+	sec_strtab = secs[shstrndx].strtab;
 	name = "<noname>";
-	if (shndx < ehdr.e_shnum) {
+	if (shndx < shnum) {
 		name = sec_strtab + secs[shndx].shdr.sh_name;
 	}
 	else if (shndx == SHN_ABS) {
@@ -271,7 +273,7 @@ static const char *sym_name(const char *sym_strtab, Elf_Sym *sym)
 static Elf_Sym *sym_lookup(const char *symname)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		long nsyms;
 		char *strtab;
@@ -366,6 +368,9 @@ static void read_ehdr(FILE *fp)
 	ehdr.e_shnum     = elf_half_to_cpu(ehdr.e_shnum);
 	ehdr.e_shstrndx  = elf_half_to_cpu(ehdr.e_shstrndx);
 
+	shnum = ehdr.e_shnum;
+	shstrndx = ehdr.e_shstrndx;
+
 	if ((ehdr.e_type != ET_EXEC) && (ehdr.e_type != ET_DYN)) {
 		die("Unsupported ELF header type\n");
 	}
@@ -384,7 +389,26 @@ static void read_ehdr(FILE *fp)
 	if (ehdr.e_shentsize != sizeof(Elf_Shdr)) {
 		die("Bad section header entry\n");
 	}
-	if (ehdr.e_shstrndx >= ehdr.e_shnum) {
+
+	if (shnum == SHN_UNDEF || shstrndx == SHN_XINDEX) {
+		Elf_Shdr shdr;
+
+		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
+			die("Seek to %d failed: %s\n",
+				ehdr.e_shoff, strerror(errno));
+
+		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
+			die("Cannot read initial ELF section header: %s\n",
+			    strerror(errno));
+
+		if (shnum == SHN_UNDEF)
+			shnum = elf_xword_to_cpu(shdr.sh_size);
+
+		if (shstrndx == SHN_XINDEX)
+			shstrndx = elf_word_to_cpu(shdr.sh_link);
+	}
+
+	if (shstrndx >= shnum) {
 		die("String table index out of bounds\n");
 	}
 }
@@ -394,20 +418,20 @@ static void read_shdrs(FILE *fp)
 	int i;
 	Elf_Shdr shdr;
 
-	secs = calloc(ehdr.e_shnum, sizeof(struct section));
+	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
 		die("Unable to allocate %d section headers\n",
-		    ehdr.e_shnum);
+		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
 		die("Seek to %d failed: %s\n",
 			ehdr.e_shoff, strerror(errno));
 	}
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
 			die("Cannot read ELF section headers %d/%d: %s\n",
-			    i, ehdr.e_shnum, strerror(errno));
+			    i, shnum, strerror(errno));
 		sec->shdr.sh_name      = elf_word_to_cpu(shdr.sh_name);
 		sec->shdr.sh_type      = elf_word_to_cpu(shdr.sh_type);
 		sec->shdr.sh_flags     = elf_xword_to_cpu(shdr.sh_flags);
@@ -418,7 +442,7 @@ static void read_shdrs(FILE *fp)
 		sec->shdr.sh_info      = elf_word_to_cpu(shdr.sh_info);
 		sec->shdr.sh_addralign = elf_xword_to_cpu(shdr.sh_addralign);
 		sec->shdr.sh_entsize   = elf_xword_to_cpu(shdr.sh_entsize);
-		if (sec->shdr.sh_link < ehdr.e_shnum)
+		if (sec->shdr.sh_link < shnum)
 			sec->link = &secs[sec->shdr.sh_link];
 	}
 
@@ -427,7 +451,7 @@ static void read_shdrs(FILE *fp)
 static void read_strtabs(FILE *fp)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_STRTAB) {
 			continue;
@@ -452,7 +476,7 @@ static void read_strtabs(FILE *fp)
 static void read_symtabs(FILE *fp)
 {
 	int i,j;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_SYMTAB) {
 			continue;
@@ -485,7 +509,7 @@ static void read_symtabs(FILE *fp)
 static void read_relocs(FILE *fp)
 {
 	int i,j;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_REL_TYPE) {
 			continue;
@@ -528,7 +552,7 @@ static void print_absolute_symbols(void)
 
 	printf("Absolute symbols\n");
 	printf(" Num:    Value Size  Type       Bind        Visibility  Name\n");
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		char *sym_strtab;
 		int j;
@@ -566,7 +590,7 @@ static void print_absolute_relocs(void)
 	else
 		format = "%08"PRIx32" %08"PRIx32" %10s %08"PRIx32"  %s\n";
 
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		struct section *sec_applies, *sec_symtab;
 		char *sym_strtab;
@@ -650,7 +674,7 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 {
 	int i;
 	/* Walk through the relocations */
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		char *sym_strtab;
 		Elf_Sym *sh_symtab;
 		struct section *sec_applies, *sec_symtab;
@@ -706,7 +730,7 @@ static Elf_Addr per_cpu_load_addr;
 static void percpu_init(void)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		ElfW(Sym) *sym;
 		if (strcmp(sec_name(i), ".data..percpu"))
 			continue;
-- 
2.19.1


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

* Re: [PATCH] x86/tools/relocs: fix big section header tables
  2018-11-29 13:51 [PATCH] x86/tools/relocs: fix big section header tables Artem Savkov
@ 2018-11-29 14:23 ` Josh Poimboeuf
  2018-11-29 15:22   ` Artem Savkov
  0 siblings, 1 reply; 8+ messages in thread
From: Josh Poimboeuf @ 2018-11-29 14:23 UTC (permalink / raw)
  To: Artem Savkov; +Cc: x86, Thomas Gleixner, Eric W . Biederman, linux-kernel

On Thu, Nov 29, 2018 at 02:51:33PM +0100, Artem Savkov wrote:
> In case when the number of entries in the section header table is larger
> then or equal to SHN_LORESERVE the size of the table is held in the sh_size
> member of the initial entry in section header table instead of e_shnum.
> Same with the string table index which is located in sh_link instead of
> e_shstrndx.
> 
> This case is easily reproducible with KCFLAGS="-ffunction-sections",
> bzImage build fails with "String table index out of bounds" error.
> 
> Signed-off-by: Artem Savkov <asavkov@redhat.com>
> ---
>  arch/x86/tools/relocs.c | 58 +++++++++++++++++++++++++++++------------
>  1 file changed, 41 insertions(+), 17 deletions(-)
> 
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index b629f6992d9f..5275ea0a0d13 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -11,7 +11,9 @@
>  #define Elf_Shdr		ElfW(Shdr)
>  #define Elf_Sym			ElfW(Sym)
>  
> -static Elf_Ehdr ehdr;
> +static Elf_Ehdr	ehdr;

I think there's a tab missing here, it doesn't line up with the other
variables.

> +static unsigned long	shnum;
> +static unsigned int	shstrndx;

Otherwise the patch looks good to me.

Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

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

* Re: [PATCH] x86/tools/relocs: fix big section header tables
  2018-11-29 14:23 ` Josh Poimboeuf
@ 2018-11-29 15:22   ` Artem Savkov
  2018-11-29 15:32     ` Josh Poimboeuf
  0 siblings, 1 reply; 8+ messages in thread
From: Artem Savkov @ 2018-11-29 15:22 UTC (permalink / raw)
  To: Josh Poimboeuf; +Cc: x86, Thomas Gleixner, Eric W . Biederman, linux-kernel

On Thu, Nov 29, 2018 at 08:23:12AM -0600, Josh Poimboeuf wrote:
> On Thu, Nov 29, 2018 at 02:51:33PM +0100, Artem Savkov wrote:
> > In case when the number of entries in the section header table is larger
> > then or equal to SHN_LORESERVE the size of the table is held in the sh_size
> > member of the initial entry in section header table instead of e_shnum.
> > Same with the string table index which is located in sh_link instead of
> > e_shstrndx.
> > 
> > This case is easily reproducible with KCFLAGS="-ffunction-sections",
> > bzImage build fails with "String table index out of bounds" error.
> > 
> > Signed-off-by: Artem Savkov <asavkov@redhat.com>
> > ---
> >  arch/x86/tools/relocs.c | 58 +++++++++++++++++++++++++++++------------
> >  1 file changed, 41 insertions(+), 17 deletions(-)
> > 
> > diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> > index b629f6992d9f..5275ea0a0d13 100644
> > --- a/arch/x86/tools/relocs.c
> > +++ b/arch/x86/tools/relocs.c
> > @@ -11,7 +11,9 @@
> >  #define Elf_Shdr		ElfW(Shdr)
> >  #define Elf_Sym			ElfW(Sym)
> >  
> > -static Elf_Ehdr ehdr;
> > +static Elf_Ehdr	ehdr;
> 
> I think there's a tab missing here, it doesn't line up with the other
> variables.

This seems to be a vim bug. It aligns perfectly in
cat/less/lore.kernel.org which all seem to use tabstop=8 by default, but
it does not align in vim, but it does align with tabstop=7 in vim.


-- 
 Artem

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

* Re: [PATCH] x86/tools/relocs: fix big section header tables
  2018-11-29 15:22   ` Artem Savkov
@ 2018-11-29 15:32     ` Josh Poimboeuf
  2018-11-29 15:56       ` [PATCH v2] " Artem Savkov
  0 siblings, 1 reply; 8+ messages in thread
From: Josh Poimboeuf @ 2018-11-29 15:32 UTC (permalink / raw)
  To: Artem Savkov; +Cc: x86, Thomas Gleixner, Eric W . Biederman, linux-kernel

On Thu, Nov 29, 2018 at 04:22:00PM +0100, Artem Savkov wrote:
> On Thu, Nov 29, 2018 at 08:23:12AM -0600, Josh Poimboeuf wrote:
> > On Thu, Nov 29, 2018 at 02:51:33PM +0100, Artem Savkov wrote:
> > > In case when the number of entries in the section header table is larger
> > > then or equal to SHN_LORESERVE the size of the table is held in the sh_size
> > > member of the initial entry in section header table instead of e_shnum.
> > > Same with the string table index which is located in sh_link instead of
> > > e_shstrndx.
> > > 
> > > This case is easily reproducible with KCFLAGS="-ffunction-sections",
> > > bzImage build fails with "String table index out of bounds" error.
> > > 
> > > Signed-off-by: Artem Savkov <asavkov@redhat.com>
> > > ---
> > >  arch/x86/tools/relocs.c | 58 +++++++++++++++++++++++++++++------------
> > >  1 file changed, 41 insertions(+), 17 deletions(-)
> > > 
> > > diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> > > index b629f6992d9f..5275ea0a0d13 100644
> > > --- a/arch/x86/tools/relocs.c
> > > +++ b/arch/x86/tools/relocs.c
> > > @@ -11,7 +11,9 @@
> > >  #define Elf_Shdr		ElfW(Shdr)
> > >  #define Elf_Sym			ElfW(Sym)
> > >  
> > > -static Elf_Ehdr ehdr;
> > > +static Elf_Ehdr	ehdr;
> > 
> > I think there's a tab missing here, it doesn't line up with the other
> > variables.
> 
> This seems to be a vim bug. It aligns perfectly in
> cat/less/lore.kernel.org which all seem to use tabstop=8 by default, but
> it does not align in vim, but it does align with tabstop=7 in vim.

Are you looking at the patch?  Or the file itself?

"less arch/x86/tools/relocs.c" shows the same issue.

-- 
Josh

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

* [PATCH v2] x86/tools/relocs: fix big section header tables
  2018-11-29 15:32     ` Josh Poimboeuf
@ 2018-11-29 15:56       ` Artem Savkov
  2018-11-29 15:57         ` Josh Poimboeuf
  2019-04-19 19:01         ` [tip:x86/build] x86/tools/relocs: Fix " tip-bot for Artem Savkov
  0 siblings, 2 replies; 8+ messages in thread
From: Artem Savkov @ 2018-11-29 15:56 UTC (permalink / raw)
  To: x86
  Cc: Thomas Gleixner, Eric W . Biederman, linux-kernel,
	Josh Poimboeuf, Artem Savkov

In case when the number of entries in the section header table is larger
then or equal to SHN_LORESERVE the size of the table is held in the sh_size
member of the initial entry in section header table instead of e_shnum.
Same with the string table index which is located in sh_link instead of
e_shstrndx.

This case is easily reproducible with KCFLAGS="-ffunction-sections",
bzImage build fails with "String table index out of bounds" error.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
---
 arch/x86/tools/relocs.c | 58 +++++++++++++++++++++++++++++------------
 1 file changed, 41 insertions(+), 17 deletions(-)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index b629f6992d9f..1f5dcec15d4e 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -11,7 +11,9 @@
 #define Elf_Shdr		ElfW(Shdr)
 #define Elf_Sym			ElfW(Sym)
 
-static Elf_Ehdr ehdr;
+static Elf_Ehdr		ehdr;
+static unsigned long	shnum;
+static unsigned int	shstrndx;
 
 struct relocs {
 	uint32_t	*offset;
@@ -241,9 +243,9 @@ static const char *sec_name(unsigned shndx)
 {
 	const char *sec_strtab;
 	const char *name;
-	sec_strtab = secs[ehdr.e_shstrndx].strtab;
+	sec_strtab = secs[shstrndx].strtab;
 	name = "<noname>";
-	if (shndx < ehdr.e_shnum) {
+	if (shndx < shnum) {
 		name = sec_strtab + secs[shndx].shdr.sh_name;
 	}
 	else if (shndx == SHN_ABS) {
@@ -271,7 +273,7 @@ static const char *sym_name(const char *sym_strtab, Elf_Sym *sym)
 static Elf_Sym *sym_lookup(const char *symname)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		long nsyms;
 		char *strtab;
@@ -366,6 +368,9 @@ static void read_ehdr(FILE *fp)
 	ehdr.e_shnum     = elf_half_to_cpu(ehdr.e_shnum);
 	ehdr.e_shstrndx  = elf_half_to_cpu(ehdr.e_shstrndx);
 
+	shnum = ehdr.e_shnum;
+	shstrndx = ehdr.e_shstrndx;
+
 	if ((ehdr.e_type != ET_EXEC) && (ehdr.e_type != ET_DYN)) {
 		die("Unsupported ELF header type\n");
 	}
@@ -384,7 +389,26 @@ static void read_ehdr(FILE *fp)
 	if (ehdr.e_shentsize != sizeof(Elf_Shdr)) {
 		die("Bad section header entry\n");
 	}
-	if (ehdr.e_shstrndx >= ehdr.e_shnum) {
+
+	if (shnum == SHN_UNDEF || shstrndx == SHN_XINDEX) {
+		Elf_Shdr shdr;
+
+		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
+			die("Seek to %d failed: %s\n",
+				ehdr.e_shoff, strerror(errno));
+
+		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
+			die("Cannot read initial ELF section header: %s\n",
+			    strerror(errno));
+
+		if (shnum == SHN_UNDEF)
+			shnum = elf_xword_to_cpu(shdr.sh_size);
+
+		if (shstrndx == SHN_XINDEX)
+			shstrndx = elf_word_to_cpu(shdr.sh_link);
+	}
+
+	if (shstrndx >= shnum) {
 		die("String table index out of bounds\n");
 	}
 }
@@ -394,20 +418,20 @@ static void read_shdrs(FILE *fp)
 	int i;
 	Elf_Shdr shdr;
 
-	secs = calloc(ehdr.e_shnum, sizeof(struct section));
+	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
 		die("Unable to allocate %d section headers\n",
-		    ehdr.e_shnum);
+		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
 		die("Seek to %d failed: %s\n",
 			ehdr.e_shoff, strerror(errno));
 	}
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
 			die("Cannot read ELF section headers %d/%d: %s\n",
-			    i, ehdr.e_shnum, strerror(errno));
+			    i, shnum, strerror(errno));
 		sec->shdr.sh_name      = elf_word_to_cpu(shdr.sh_name);
 		sec->shdr.sh_type      = elf_word_to_cpu(shdr.sh_type);
 		sec->shdr.sh_flags     = elf_xword_to_cpu(shdr.sh_flags);
@@ -418,7 +442,7 @@ static void read_shdrs(FILE *fp)
 		sec->shdr.sh_info      = elf_word_to_cpu(shdr.sh_info);
 		sec->shdr.sh_addralign = elf_xword_to_cpu(shdr.sh_addralign);
 		sec->shdr.sh_entsize   = elf_xword_to_cpu(shdr.sh_entsize);
-		if (sec->shdr.sh_link < ehdr.e_shnum)
+		if (sec->shdr.sh_link < shnum)
 			sec->link = &secs[sec->shdr.sh_link];
 	}
 
@@ -427,7 +451,7 @@ static void read_shdrs(FILE *fp)
 static void read_strtabs(FILE *fp)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_STRTAB) {
 			continue;
@@ -452,7 +476,7 @@ static void read_strtabs(FILE *fp)
 static void read_symtabs(FILE *fp)
 {
 	int i,j;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_SYMTAB) {
 			continue;
@@ -485,7 +509,7 @@ static void read_symtabs(FILE *fp)
 static void read_relocs(FILE *fp)
 {
 	int i,j;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_REL_TYPE) {
 			continue;
@@ -528,7 +552,7 @@ static void print_absolute_symbols(void)
 
 	printf("Absolute symbols\n");
 	printf(" Num:    Value Size  Type       Bind        Visibility  Name\n");
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		char *sym_strtab;
 		int j;
@@ -566,7 +590,7 @@ static void print_absolute_relocs(void)
 	else
 		format = "%08"PRIx32" %08"PRIx32" %10s %08"PRIx32"  %s\n";
 
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		struct section *sec_applies, *sec_symtab;
 		char *sym_strtab;
@@ -650,7 +674,7 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 {
 	int i;
 	/* Walk through the relocations */
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		char *sym_strtab;
 		Elf_Sym *sh_symtab;
 		struct section *sec_applies, *sec_symtab;
@@ -706,7 +730,7 @@ static Elf_Addr per_cpu_load_addr;
 static void percpu_init(void)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		ElfW(Sym) *sym;
 		if (strcmp(sec_name(i), ".data..percpu"))
 			continue;
-- 
2.19.1


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

* Re: [PATCH v2] x86/tools/relocs: fix big section header tables
  2018-11-29 15:56       ` [PATCH v2] " Artem Savkov
@ 2018-11-29 15:57         ` Josh Poimboeuf
  2019-01-11 19:09           ` Joe Lawrence
  2019-04-19 19:01         ` [tip:x86/build] x86/tools/relocs: Fix " tip-bot for Artem Savkov
  1 sibling, 1 reply; 8+ messages in thread
From: Josh Poimboeuf @ 2018-11-29 15:57 UTC (permalink / raw)
  To: Artem Savkov; +Cc: x86, Thomas Gleixner, Eric W . Biederman, linux-kernel

On Thu, Nov 29, 2018 at 04:56:15PM +0100, Artem Savkov wrote:
> In case when the number of entries in the section header table is larger
> then or equal to SHN_LORESERVE the size of the table is held in the sh_size
> member of the initial entry in section header table instead of e_shnum.
> Same with the string table index which is located in sh_link instead of
> e_shstrndx.
> 
> This case is easily reproducible with KCFLAGS="-ffunction-sections",
> bzImage build fails with "String table index out of bounds" error.
> 
> Signed-off-by: Artem Savkov <asavkov@redhat.com>

Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>

-- 
Josh

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

* Re: [PATCH v2] x86/tools/relocs: fix big section header tables
  2018-11-29 15:57         ` Josh Poimboeuf
@ 2019-01-11 19:09           ` Joe Lawrence
  0 siblings, 0 replies; 8+ messages in thread
From: Joe Lawrence @ 2019-01-11 19:09 UTC (permalink / raw)
  To: Josh Poimboeuf
  Cc: Artem Savkov, x86, Thomas Gleixner, Eric W . Biederman,
	linux-kernel, Ingo Molnar

On Thu, Nov 29, 2018 at 09:57:50AM -0600, Josh Poimboeuf wrote:
> On Thu, Nov 29, 2018 at 04:56:15PM +0100, Artem Savkov wrote:
> > In case when the number of entries in the section header table is larger
> > then or equal to SHN_LORESERVE the size of the table is held in the sh_size
> > member of the initial entry in section header table instead of e_shnum.
> > Same with the string table index which is located in sh_link instead of
> > e_shstrndx.
> > 
> > This case is easily reproducible with KCFLAGS="-ffunction-sections",
> > bzImage build fails with "String table index out of bounds" error.
> > 
> > Signed-off-by: Artem Savkov <asavkov@redhat.com>
> 
> Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
> 

Gentle ping to nudge this patch along (I didn't see it in anyone's tree).

Thanks,

--- Joe

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

* [tip:x86/build] x86/tools/relocs: Fix big section header tables
  2018-11-29 15:56       ` [PATCH v2] " Artem Savkov
  2018-11-29 15:57         ` Josh Poimboeuf
@ 2019-04-19 19:01         ` tip-bot for Artem Savkov
  1 sibling, 0 replies; 8+ messages in thread
From: tip-bot for Artem Savkov @ 2019-04-19 19:01 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: torvalds, ebiederm, hpa, jpoimboe, linux-kernel, asavkov, tglx,
	peterz, mingo, joe.lawrence

Commit-ID:  f36e7495dd3990d6848e6d6703c78f1f17a97538
Gitweb:     https://git.kernel.org/tip/f36e7495dd3990d6848e6d6703c78f1f17a97538
Author:     Artem Savkov <asavkov@redhat.com>
AuthorDate: Thu, 29 Nov 2018 16:56:15 +0100
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 19 Apr 2019 20:54:07 +0200

x86/tools/relocs: Fix big section header tables

In case when the number of entries in the section header table is larger
then or equal to SHN_LORESERVE the size of the table is held in the sh_size
member of the initial entry in section header table instead of e_shnum.
Same with the string table index which is located in sh_link instead of
e_shstrndx.

This case is easily reproducible with KCFLAGS="-ffunction-sections",
bzImage build fails with "String table index out of bounds" error.

Signed-off-by: Artem Savkov <asavkov@redhat.com>
Reviewed-by: Josh Poimboeuf <jpoimboe@redhat.com>
Acked-by: Joe Lawrence <joe.lawrence@redhat.com>
Cc: Eric W . Biederman <ebiederm@xmission.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20181129155615.2594-1-asavkov@redhat.com
[ Simplify the die() lines. ]
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/tools/relocs.c | 74 ++++++++++++++++++++++++++++++-------------------
 1 file changed, 45 insertions(+), 29 deletions(-)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index b629f6992d9f..f345586f5e50 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -11,7 +11,9 @@
 #define Elf_Shdr		ElfW(Shdr)
 #define Elf_Sym			ElfW(Sym)
 
-static Elf_Ehdr ehdr;
+static Elf_Ehdr		ehdr;
+static unsigned long	shnum;
+static unsigned int	shstrndx;
 
 struct relocs {
 	uint32_t	*offset;
@@ -241,9 +243,9 @@ static const char *sec_name(unsigned shndx)
 {
 	const char *sec_strtab;
 	const char *name;
-	sec_strtab = secs[ehdr.e_shstrndx].strtab;
+	sec_strtab = secs[shstrndx].strtab;
 	name = "<noname>";
-	if (shndx < ehdr.e_shnum) {
+	if (shndx < shnum) {
 		name = sec_strtab + secs[shndx].shdr.sh_name;
 	}
 	else if (shndx == SHN_ABS) {
@@ -271,7 +273,7 @@ static const char *sym_name(const char *sym_strtab, Elf_Sym *sym)
 static Elf_Sym *sym_lookup(const char *symname)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		long nsyms;
 		char *strtab;
@@ -366,27 +368,41 @@ static void read_ehdr(FILE *fp)
 	ehdr.e_shnum     = elf_half_to_cpu(ehdr.e_shnum);
 	ehdr.e_shstrndx  = elf_half_to_cpu(ehdr.e_shstrndx);
 
-	if ((ehdr.e_type != ET_EXEC) && (ehdr.e_type != ET_DYN)) {
+	shnum = ehdr.e_shnum;
+	shstrndx = ehdr.e_shstrndx;
+
+	if ((ehdr.e_type != ET_EXEC) && (ehdr.e_type != ET_DYN))
 		die("Unsupported ELF header type\n");
-	}
-	if (ehdr.e_machine != ELF_MACHINE) {
+	if (ehdr.e_machine != ELF_MACHINE)
 		die("Not for %s\n", ELF_MACHINE_NAME);
-	}
-	if (ehdr.e_version != EV_CURRENT) {
+	if (ehdr.e_version != EV_CURRENT)
 		die("Unknown ELF version\n");
-	}
-	if (ehdr.e_ehsize != sizeof(Elf_Ehdr)) {
+	if (ehdr.e_ehsize != sizeof(Elf_Ehdr))
 		die("Bad Elf header size\n");
-	}
-	if (ehdr.e_phentsize != sizeof(Elf_Phdr)) {
+	if (ehdr.e_phentsize != sizeof(Elf_Phdr))
 		die("Bad program header entry\n");
-	}
-	if (ehdr.e_shentsize != sizeof(Elf_Shdr)) {
+	if (ehdr.e_shentsize != sizeof(Elf_Shdr))
 		die("Bad section header entry\n");
+
+
+	if (shnum == SHN_UNDEF || shstrndx == SHN_XINDEX) {
+		Elf_Shdr shdr;
+
+		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
+			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
+
+		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
+			die("Cannot read initial ELF section header: %s\n", strerror(errno));
+
+		if (shnum == SHN_UNDEF)
+			shnum = elf_xword_to_cpu(shdr.sh_size);
+
+		if (shstrndx == SHN_XINDEX)
+			shstrndx = elf_word_to_cpu(shdr.sh_link);
 	}
-	if (ehdr.e_shstrndx >= ehdr.e_shnum) {
+
+	if (shstrndx >= shnum)
 		die("String table index out of bounds\n");
-	}
 }
 
 static void read_shdrs(FILE *fp)
@@ -394,20 +410,20 @@ static void read_shdrs(FILE *fp)
 	int i;
 	Elf_Shdr shdr;
 
-	secs = calloc(ehdr.e_shnum, sizeof(struct section));
+	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
 		die("Unable to allocate %d section headers\n",
-		    ehdr.e_shnum);
+		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
 		die("Seek to %d failed: %s\n",
 			ehdr.e_shoff, strerror(errno));
 	}
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
 			die("Cannot read ELF section headers %d/%d: %s\n",
-			    i, ehdr.e_shnum, strerror(errno));
+			    i, shnum, strerror(errno));
 		sec->shdr.sh_name      = elf_word_to_cpu(shdr.sh_name);
 		sec->shdr.sh_type      = elf_word_to_cpu(shdr.sh_type);
 		sec->shdr.sh_flags     = elf_xword_to_cpu(shdr.sh_flags);
@@ -418,7 +434,7 @@ static void read_shdrs(FILE *fp)
 		sec->shdr.sh_info      = elf_word_to_cpu(shdr.sh_info);
 		sec->shdr.sh_addralign = elf_xword_to_cpu(shdr.sh_addralign);
 		sec->shdr.sh_entsize   = elf_xword_to_cpu(shdr.sh_entsize);
-		if (sec->shdr.sh_link < ehdr.e_shnum)
+		if (sec->shdr.sh_link < shnum)
 			sec->link = &secs[sec->shdr.sh_link];
 	}
 
@@ -427,7 +443,7 @@ static void read_shdrs(FILE *fp)
 static void read_strtabs(FILE *fp)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_STRTAB) {
 			continue;
@@ -452,7 +468,7 @@ static void read_strtabs(FILE *fp)
 static void read_symtabs(FILE *fp)
 {
 	int i,j;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_SYMTAB) {
 			continue;
@@ -485,7 +501,7 @@ static void read_symtabs(FILE *fp)
 static void read_relocs(FILE *fp)
 {
 	int i,j;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		if (sec->shdr.sh_type != SHT_REL_TYPE) {
 			continue;
@@ -528,7 +544,7 @@ static void print_absolute_symbols(void)
 
 	printf("Absolute symbols\n");
 	printf(" Num:    Value Size  Type       Bind        Visibility  Name\n");
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		char *sym_strtab;
 		int j;
@@ -566,7 +582,7 @@ static void print_absolute_relocs(void)
 	else
 		format = "%08"PRIx32" %08"PRIx32" %10s %08"PRIx32"  %s\n";
 
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		struct section *sec = &secs[i];
 		struct section *sec_applies, *sec_symtab;
 		char *sym_strtab;
@@ -650,7 +666,7 @@ static void walk_relocs(int (*process)(struct section *sec, Elf_Rel *rel,
 {
 	int i;
 	/* Walk through the relocations */
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		char *sym_strtab;
 		Elf_Sym *sh_symtab;
 		struct section *sec_applies, *sec_symtab;
@@ -706,7 +722,7 @@ static Elf_Addr per_cpu_load_addr;
 static void percpu_init(void)
 {
 	int i;
-	for (i = 0; i < ehdr.e_shnum; i++) {
+	for (i = 0; i < shnum; i++) {
 		ElfW(Sym) *sym;
 		if (strcmp(sec_name(i), ".data..percpu"))
 			continue;

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

end of thread, other threads:[~2019-04-19 19:01 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-29 13:51 [PATCH] x86/tools/relocs: fix big section header tables Artem Savkov
2018-11-29 14:23 ` Josh Poimboeuf
2018-11-29 15:22   ` Artem Savkov
2018-11-29 15:32     ` Josh Poimboeuf
2018-11-29 15:56       ` [PATCH v2] " Artem Savkov
2018-11-29 15:57         ` Josh Poimboeuf
2019-01-11 19:09           ` Joe Lawrence
2019-04-19 19:01         ` [tip:x86/build] x86/tools/relocs: Fix " tip-bot for Artem Savkov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.