Re: [PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread Josh Poimboeuf
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 
> > > ---
> > >  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


Re: [PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread Josh Poimboeuf
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 
> > > ---
> > >  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


Re: [PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread Artem Savkov
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 
> > ---
> >  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_SymElfW(Sym)
> >  
> > -static Elf_Ehdr ehdr;
> > +static Elf_Ehdrehdr;
> 
> 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


Re: [PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread Artem Savkov
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 
> > ---
> >  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_SymElfW(Sym)
> >  
> > -static Elf_Ehdr ehdr;
> > +static Elf_Ehdrehdr;
> 
> 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


Re: [PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread Josh Poimboeuf
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 
> ---
>  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 

-- 
Josh


Re: [PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread Josh Poimboeuf
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 
> ---
>  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 

-- 
Josh


[PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread 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 
---
 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_SymElfW(Sym)
 
-static Elf_Ehdr ehdr;
+static Elf_Ehdrehdr;
+static unsigned long   shnum;
+static unsigned intshstrndx;
 
 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 = "";
-   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 = [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(, 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 = [i];
if (fread(, 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 = [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 = [i];
if 

[PATCH] x86/tools/relocs: fix big section header tables

2018-11-29 Thread 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 
---
 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_SymElfW(Sym)
 
-static Elf_Ehdr ehdr;
+static Elf_Ehdrehdr;
+static unsigned long   shnum;
+static unsigned intshstrndx;
 
 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 = "";
-   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 = [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(, 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 = [i];
if (fread(, 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 = [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 = [i];
if