All of lore.kernel.org
 help / color / mirror / Atom feed
* [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
@ 2021-06-24 11:58 Greg Kroah-Hartman
  2021-06-24 12:32 ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-24 11:58 UTC (permalink / raw)
  To: x86
  Cc: Greg Kroah-Hartman, Thomas Gleixner, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, linux-kernel

There are a number of printf "mismatches" in the use of die() in
x86/tools/relocs.c.  Fix them up and add the printf attribute to the
reloc.h header file to prevent them from ever coming back.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/x86/tools/relocs.c | 21 +++++++++++----------
 arch/x86/tools/relocs.h |  1 +
 2 files changed, 12 insertions(+), 10 deletions(-)

Originally sent back in Feb, but it seems to have been missed:
	https://lore.kernel.org/r/20210227095356.603513-1-gregkh@linuxfoundation.org


diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index ce7188cbdae5..c3105a8c6cde 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -389,7 +389,8 @@ static void read_ehdr(FILE *fp)
 		Elf_Shdr shdr;
 
 		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
-			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
+			die("Seek to %d failed: %s\n",
+			    (int)ehdr.e_shoff, strerror(errno));
 
 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
 			die("Cannot read initial ELF section header: %s\n", strerror(errno));
@@ -412,17 +413,17 @@ static void read_shdrs(FILE *fp)
 
 	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
-		die("Unable to allocate %d section headers\n",
+		die("Unable to allocate %ld section headers\n",
 		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
 		die("Seek to %d failed: %s\n",
-			ehdr.e_shoff, strerror(errno));
+			(int)ehdr.e_shoff, strerror(errno));
 	}
 	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",
+			die("Cannot read ELF section headers %d/%ld: %s\n",
 			    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);
@@ -451,11 +452,11 @@ static void read_strtabs(FILE *fp)
 		sec->strtab = malloc(sec->shdr.sh_size);
 		if (!sec->strtab) {
 			die("malloc of %d bytes for strtab failed\n",
-				sec->shdr.sh_size);
+				(int)sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
 			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+				(int)sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -476,11 +477,11 @@ static void read_symtabs(FILE *fp)
 		sec->symtab = malloc(sec->shdr.sh_size);
 		if (!sec->symtab) {
 			die("malloc of %d bytes for symtab failed\n",
-				sec->shdr.sh_size);
+				(int)sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
 			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+				(int)sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -509,11 +510,11 @@ static void read_relocs(FILE *fp)
 		sec->reltab = malloc(sec->shdr.sh_size);
 		if (!sec->reltab) {
 			die("malloc of %d bytes for relocs failed\n",
-				sec->shdr.sh_size);
+				(int)sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
 			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+				(int)sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
diff --git a/arch/x86/tools/relocs.h b/arch/x86/tools/relocs.h
index 43c83c0fd22c..4c49c82446eb 100644
--- a/arch/x86/tools/relocs.h
+++ b/arch/x86/tools/relocs.h
@@ -17,6 +17,7 @@
 #include <regex.h>
 #include <tools/le_byteshift.h>
 
+__attribute__((__format__(printf, 1, 2)))
 void die(char *fmt, ...) __attribute__((noreturn));
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-- 
2.30.1


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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-24 11:58 [RESEND PATCH] x86/tools/relocs: add __printf attribute to die() Greg Kroah-Hartman
@ 2021-06-24 12:32 ` Borislav Petkov
  2021-06-24 14:44   ` Greg Kroah-Hartman
  2021-06-25 12:06   ` Greg Kroah-Hartman
  0 siblings, 2 replies; 14+ messages in thread
From: Borislav Petkov @ 2021-06-24 12:32 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, linux-kernel

On Thu, Jun 24, 2021 at 01:58:03PM +0200, Greg Kroah-Hartman wrote:
> There are a number of printf "mismatches" in the use of die() in
> x86/tools/relocs.c.  Fix them up and add the printf attribute to the
> reloc.h header file to prevent them from ever coming back.
> 
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Borislav Petkov <bp@alien8.de>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  arch/x86/tools/relocs.c | 21 +++++++++++----------
>  arch/x86/tools/relocs.h |  1 +
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> Originally sent back in Feb, but it seems to have been missed:
> 	https://lore.kernel.org/r/20210227095356.603513-1-gregkh@linuxfoundation.org
> 
> 
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index ce7188cbdae5..c3105a8c6cde 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -389,7 +389,8 @@ static void read_ehdr(FILE *fp)
>  		Elf_Shdr shdr;
>  
>  		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
> -			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
> +			die("Seek to %d failed: %s\n",
> +			    (int)ehdr.e_shoff, strerror(errno));

Instead of casting all those, I think you should use %zu as, apparently,
we're using unsigned types for Elf{32,64}_Off and Elf{32,64}_Xword, etc.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-24 12:32 ` Borislav Petkov
@ 2021-06-24 14:44   ` Greg Kroah-Hartman
  2021-06-25 16:17     ` H. Peter Anvin
  2021-06-25 12:06   ` Greg Kroah-Hartman
  1 sibling, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-24 14:44 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, linux-kernel

On Thu, Jun 24, 2021 at 02:32:43PM +0200, Borislav Petkov wrote:
> On Thu, Jun 24, 2021 at 01:58:03PM +0200, Greg Kroah-Hartman wrote:
> > There are a number of printf "mismatches" in the use of die() in
> > x86/tools/relocs.c.  Fix them up and add the printf attribute to the
> > reloc.h header file to prevent them from ever coming back.
> > 
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  arch/x86/tools/relocs.c | 21 +++++++++++----------
> >  arch/x86/tools/relocs.h |  1 +
> >  2 files changed, 12 insertions(+), 10 deletions(-)
> > 
> > Originally sent back in Feb, but it seems to have been missed:
> > 	https://lore.kernel.org/r/20210227095356.603513-1-gregkh@linuxfoundation.org
> > 
> > 
> > diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> > index ce7188cbdae5..c3105a8c6cde 100644
> > --- a/arch/x86/tools/relocs.c
> > +++ b/arch/x86/tools/relocs.c
> > @@ -389,7 +389,8 @@ static void read_ehdr(FILE *fp)
> >  		Elf_Shdr shdr;
> >  
> >  		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
> > -			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
> > +			die("Seek to %d failed: %s\n",
> > +			    (int)ehdr.e_shoff, strerror(errno));
> 
> Instead of casting all those, I think you should use %zu as, apparently,
> we're using unsigned types for Elf{32,64}_Off and Elf{32,64}_Xword, etc.

Odd, I thought I tried that and something did not work.  Let me try it
again...

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-24 12:32 ` Borislav Petkov
  2021-06-24 14:44   ` Greg Kroah-Hartman
@ 2021-06-25 12:06   ` Greg Kroah-Hartman
  2021-06-25 14:12     ` Borislav Petkov
  1 sibling, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-25 12:06 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, linux-kernel

On Thu, Jun 24, 2021 at 02:32:43PM +0200, Borislav Petkov wrote:
> On Thu, Jun 24, 2021 at 01:58:03PM +0200, Greg Kroah-Hartman wrote:
> > There are a number of printf "mismatches" in the use of die() in
> > x86/tools/relocs.c.  Fix them up and add the printf attribute to the
> > reloc.h header file to prevent them from ever coming back.
> > 
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
> > Cc: Borislav Petkov <bp@alien8.de>
> > Cc: "H. Peter Anvin" <hpa@zytor.com>
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  arch/x86/tools/relocs.c | 21 +++++++++++----------
> >  arch/x86/tools/relocs.h |  1 +
> >  2 files changed, 12 insertions(+), 10 deletions(-)
> > 
> > Originally sent back in Feb, but it seems to have been missed:
> > 	https://lore.kernel.org/r/20210227095356.603513-1-gregkh@linuxfoundation.org
> > 
> > 
> > diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> > index ce7188cbdae5..c3105a8c6cde 100644
> > --- a/arch/x86/tools/relocs.c
> > +++ b/arch/x86/tools/relocs.c
> > @@ -389,7 +389,8 @@ static void read_ehdr(FILE *fp)
> >  		Elf_Shdr shdr;
> >  
> >  		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
> > -			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
> > +			die("Seek to %d failed: %s\n",
> > +			    (int)ehdr.e_shoff, strerror(errno));
> 
> Instead of casting all those, I think you should use %zu as, apparently,
> we're using unsigned types for Elf{32,64}_Off and Elf{32,64}_Xword, etc.

Ah, that does not work, I tried it and I get:

arch/x86/tools/relocs.c: In function ‘read_ehdr’:
arch/x86/tools/relocs.c:392:40: warning: format ‘%zu’ expects argument of type ‘size_t’, but argument 2 has type ‘Elf32_Off’ {aka ‘unsigned int’} [-Wformat=]
  392 |                         die("Seek to %zu failed: %s\n",
      |                                      ~~^
      |                                        |
      |                                        long unsigned int
      |                                      %u
  393 |                             ehdr.e_shoff, strerror(errno));
      |                             ~~~~~~~~~~~~
      |                                 |
      |                                 Elf32_Off {aka unsigned int}

Casting seems to be the only way to make this "quiet" that I can tell.

Unless someone else has a good idea?

thanks,

greg k-h

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-25 12:06   ` Greg Kroah-Hartman
@ 2021-06-25 14:12     ` Borislav Petkov
  2021-06-25 16:19       ` H. Peter Anvin
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2021-06-25 14:12 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: x86, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, linux-kernel

On Fri, Jun 25, 2021 at 02:06:59PM +0200, Greg Kroah-Hartman wrote:
> Casting seems to be the only way to make this "quiet" that I can tell.
> 
> Unless someone else has a good idea?

Hmm, so in Documentation/core-api/printk-formats.rst we say that for
printk() with different size types, we should "use a format specifier of
its largest possible type and explicitly cast to it."

And that kinda sounds ok to me because we don't potentially lose through
the casting.

IOW, I guess something like this below.

---
diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 04c5a44b9682..42b0f425a2c7 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -389,7 +389,7 @@ static void read_ehdr(FILE *fp)
 		Elf_Shdr shdr;
 
 		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
-			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
+			die("Seek to %lu failed: %s\n", (unsigned long)ehdr.e_shoff, strerror(errno));
 
 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
 			die("Cannot read initial ELF section header: %s\n", strerror(errno));
@@ -412,17 +412,17 @@ static void read_shdrs(FILE *fp)
 
 	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
-		die("Unable to allocate %d section headers\n",
+		die("Unable to allocate %ld section headers\n",
 		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
-		die("Seek to %d failed: %s\n",
-			ehdr.e_shoff, strerror(errno));
+		die("Seek to %lu failed: %s\n",
+		    (unsigned long)ehdr.e_shoff, strerror(errno));
 	}
 	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",
+			die("Cannot read ELF section headers %d/%ld: %s\n",
 			    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);
@@ -450,12 +450,12 @@ static void read_strtabs(FILE *fp)
 		}
 		sec->strtab = malloc(sec->shdr.sh_size);
 		if (!sec->strtab) {
-			die("malloc of %d bytes for strtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %lu bytes for strtab failed\n",
+			    (unsigned long)sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %lu failed: %s\n",
+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -475,12 +475,12 @@ static void read_symtabs(FILE *fp)
 		}
 		sec->symtab = malloc(sec->shdr.sh_size);
 		if (!sec->symtab) {
-			die("malloc of %d bytes for symtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %lu bytes for symtab failed\n",
+			    (unsigned long)sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %lu failed: %s\n",
+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -508,12 +508,12 @@ static void read_relocs(FILE *fp)
 		}
 		sec->reltab = malloc(sec->shdr.sh_size);
 		if (!sec->reltab) {
-			die("malloc of %d bytes for relocs failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %lu bytes for relocs failed\n",
+			    (unsigned long)sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %lu failed: %s\n",
+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
diff --git a/arch/x86/tools/relocs.h b/arch/x86/tools/relocs.h
index 43c83c0fd22c..4c49c82446eb 100644
--- a/arch/x86/tools/relocs.h
+++ b/arch/x86/tools/relocs.h
@@ -17,6 +17,7 @@
 #include <regex.h>
 #include <tools/le_byteshift.h>
 
+__attribute__((__format__(printf, 1, 2)))
 void die(char *fmt, ...) __attribute__((noreturn));
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-24 14:44   ` Greg Kroah-Hartman
@ 2021-06-25 16:17     ` H. Peter Anvin
  0 siblings, 0 replies; 14+ messages in thread
From: H. Peter Anvin @ 2021-06-25 16:17 UTC (permalink / raw)
  To: Greg Kroah-Hartman, Borislav Petkov
  Cc: x86, Thomas Gleixner, Ingo Molnar, linux-kernel

%zu wouldn't DTRT for cross build.

On June 24, 2021 7:44:02 AM PDT, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>On Thu, Jun 24, 2021 at 02:32:43PM +0200, Borislav Petkov wrote:
>> On Thu, Jun 24, 2021 at 01:58:03PM +0200, Greg Kroah-Hartman wrote:
>> > There are a number of printf "mismatches" in the use of die() in
>> > x86/tools/relocs.c.  Fix them up and add the printf attribute to
>the
>> > reloc.h header file to prevent them from ever coming back.
>> > 
>> > Cc: Thomas Gleixner <tglx@linutronix.de>
>> > Cc: Ingo Molnar <mingo@redhat.com>
>> > Cc: Borislav Petkov <bp@alien8.de>
>> > Cc: "H. Peter Anvin" <hpa@zytor.com>
>> > Cc: linux-kernel@vger.kernel.org
>> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>> > ---
>> >  arch/x86/tools/relocs.c | 21 +++++++++++----------
>> >  arch/x86/tools/relocs.h |  1 +
>> >  2 files changed, 12 insertions(+), 10 deletions(-)
>> > 
>> > Originally sent back in Feb, but it seems to have been missed:
>> >
>	https://lore.kernel.org/r/20210227095356.603513-1-gregkh@linuxfoundation.org
>> > 
>> > 
>> > diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
>> > index ce7188cbdae5..c3105a8c6cde 100644
>> > --- a/arch/x86/tools/relocs.c
>> > +++ b/arch/x86/tools/relocs.c
>> > @@ -389,7 +389,8 @@ static void read_ehdr(FILE *fp)
>> >  		Elf_Shdr shdr;
>> >  
>> >  		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
>> > -			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
>> > +			die("Seek to %d failed: %s\n",
>> > +			    (int)ehdr.e_shoff, strerror(errno));
>> 
>> Instead of casting all those, I think you should use %zu as,
>apparently,
>> we're using unsigned types for Elf{32,64}_Off and Elf{32,64}_Xword,
>etc.
>
>Odd, I thought I tried that and something did not work.  Let me try it
>again...

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-25 14:12     ` Borislav Petkov
@ 2021-06-25 16:19       ` H. Peter Anvin
  2021-06-25 16:53         ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: H. Peter Anvin @ 2021-06-25 16:19 UTC (permalink / raw)
  To: Borislav Petkov, Greg Kroah-Hartman
  Cc: x86, Thomas Gleixner, Ingo Molnar, linux-kernel

This is a user space build time tool.

You can use PRIu32/64 or cast to unsigned long long; it's not like the performance for this case is going to matter one iota.

On June 25, 2021 7:12:36 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>On Fri, Jun 25, 2021 at 02:06:59PM +0200, Greg Kroah-Hartman wrote:
>> Casting seems to be the only way to make this "quiet" that I can
>tell.
>> 
>> Unless someone else has a good idea?
>
>Hmm, so in Documentation/core-api/printk-formats.rst we say that for
>printk() with different size types, we should "use a format specifier
>of
>its largest possible type and explicitly cast to it."
>
>And that kinda sounds ok to me because we don't potentially lose
>through
>the casting.
>
>IOW, I guess something like this below.
>
>---
>diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
>index 04c5a44b9682..42b0f425a2c7 100644
>--- a/arch/x86/tools/relocs.c
>+++ b/arch/x86/tools/relocs.c
>@@ -389,7 +389,7 @@ static void read_ehdr(FILE *fp)
> 		Elf_Shdr shdr;
> 
> 		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
>-			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
>+			die("Seek to %lu failed: %s\n", (unsigned long)ehdr.e_shoff,
>strerror(errno));
> 
> 		if (fread(&shdr, sizeof(shdr), 1, fp) != 1)
>			die("Cannot read initial ELF section header: %s\n",
>strerror(errno));
>@@ -412,17 +412,17 @@ static void read_shdrs(FILE *fp)
> 
> 	secs = calloc(shnum, sizeof(struct section));
> 	if (!secs) {
>-		die("Unable to allocate %d section headers\n",
>+		die("Unable to allocate %ld section headers\n",
> 		    shnum);
> 	}
> 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
>-		die("Seek to %d failed: %s\n",
>-			ehdr.e_shoff, strerror(errno));
>+		die("Seek to %lu failed: %s\n",
>+		    (unsigned long)ehdr.e_shoff, strerror(errno));
> 	}
> 	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",
>+			die("Cannot read ELF section headers %d/%ld: %s\n",
> 			    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);
>@@ -450,12 +450,12 @@ static void read_strtabs(FILE *fp)
> 		}
> 		sec->strtab = malloc(sec->shdr.sh_size);
> 		if (!sec->strtab) {
>-			die("malloc of %d bytes for strtab failed\n",
>-				sec->shdr.sh_size);
>+			die("malloc of %lu bytes for strtab failed\n",
>+			    (unsigned long)sec->shdr.sh_size);
> 		}
> 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
>-			die("Seek to %d failed: %s\n",
>-				sec->shdr.sh_offset, strerror(errno));
>+			die("Seek to %lu failed: %s\n",
>+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
> 		}
> 		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
> 		    != sec->shdr.sh_size) {
>@@ -475,12 +475,12 @@ static void read_symtabs(FILE *fp)
> 		}
> 		sec->symtab = malloc(sec->shdr.sh_size);
> 		if (!sec->symtab) {
>-			die("malloc of %d bytes for symtab failed\n",
>-				sec->shdr.sh_size);
>+			die("malloc of %lu bytes for symtab failed\n",
>+			    (unsigned long)sec->shdr.sh_size);
> 		}
> 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
>-			die("Seek to %d failed: %s\n",
>-				sec->shdr.sh_offset, strerror(errno));
>+			die("Seek to %lu failed: %s\n",
>+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
> 		}
> 		if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
> 		    != sec->shdr.sh_size) {
>@@ -508,12 +508,12 @@ static void read_relocs(FILE *fp)
> 		}
> 		sec->reltab = malloc(sec->shdr.sh_size);
> 		if (!sec->reltab) {
>-			die("malloc of %d bytes for relocs failed\n",
>-				sec->shdr.sh_size);
>+			die("malloc of %lu bytes for relocs failed\n",
>+			    (unsigned long)sec->shdr.sh_size);
> 		}
> 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
>-			die("Seek to %d failed: %s\n",
>-				sec->shdr.sh_offset, strerror(errno));
>+			die("Seek to %lu failed: %s\n",
>+			    (unsigned long)sec->shdr.sh_offset, strerror(errno));
> 		}
> 		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
> 		    != sec->shdr.sh_size) {
>diff --git a/arch/x86/tools/relocs.h b/arch/x86/tools/relocs.h
>index 43c83c0fd22c..4c49c82446eb 100644
>--- a/arch/x86/tools/relocs.h
>+++ b/arch/x86/tools/relocs.h
>@@ -17,6 +17,7 @@
> #include <regex.h>
> #include <tools/le_byteshift.h>
> 
>+__attribute__((__format__(printf, 1, 2)))
> void die(char *fmt, ...) __attribute__((noreturn));
> 
> #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-25 16:19       ` H. Peter Anvin
@ 2021-06-25 16:53         ` Borislav Petkov
  2021-06-25 20:38           ` H. Peter Anvin
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2021-06-25 16:53 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Greg Kroah-Hartman, x86, Thomas Gleixner, Ingo Molnar, linux-kernel

On Fri, Jun 25, 2021 at 09:19:38AM -0700, H. Peter Anvin wrote:
> You can use PRIu32/64 or cast to unsigned long long; it's not like the
> performance for this case is going to matter one iota.

Why "unsigned long long"?

Those fields are typedeffed as:

typedef __u32	Elf32_Off;

or

typedef __u64	Elf64_Off;

respectively so they should fit in an "unsigned long" on the respective
width.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-25 16:53         ` Borislav Petkov
@ 2021-06-25 20:38           ` H. Peter Anvin
  2021-06-25 21:07             ` Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: H. Peter Anvin @ 2021-06-25 20:38 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: Greg Kroah-Hartman, x86, Thomas Gleixner, Ingo Molnar, linux-kernel

64-bit cross build on a 32-bit platform... or Windows.

On June 25, 2021 9:53:10 AM PDT, Borislav Petkov <bp@alien8.de> wrote:
>On Fri, Jun 25, 2021 at 09:19:38AM -0700, H. Peter Anvin wrote:
>> You can use PRIu32/64 or cast to unsigned long long; it's not like
>the
>> performance for this case is going to matter one iota.
>
>Why "unsigned long long"?
>
>Those fields are typedeffed as:
>
>typedef __u32	Elf32_Off;
>
>or
>
>typedef __u64	Elf64_Off;
>
>respectively so they should fit in an "unsigned long" on the respective
>width.

-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-25 20:38           ` H. Peter Anvin
@ 2021-06-25 21:07             ` Borislav Petkov
  2021-06-27 15:01               ` Greg Kroah-Hartman
  0 siblings, 1 reply; 14+ messages in thread
From: Borislav Petkov @ 2021-06-25 21:07 UTC (permalink / raw)
  To: H. Peter Anvin
  Cc: Greg Kroah-Hartman, x86, Thomas Gleixner, Ingo Molnar, linux-kernel

On Fri, Jun 25, 2021 at 01:38:51PM -0700, H. Peter Anvin wrote:
> 64-bit cross build on a 32-bit platform... or Windows.

Meh, nobody cares about those... :)

Hmm, so looking at the PRI* inttypes.h things again, they're C99 and
they kinda look more elegant as they don't make us cast stuff.

So how does that look?

---

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 04c5a44b9682..2582991ba216 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -26,6 +26,9 @@ static struct relocs relocs32;
 #if ELF_BITS == 64
 static struct relocs relocs32neg;
 static struct relocs relocs64;
+#define FMT PRIu64
+#else
+#define FMT PRIu32
 #endif
 
 struct section {
@@ -389,7 +392,7 @@ static void read_ehdr(FILE *fp)
 		Elf_Shdr shdr;
 
 		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
-			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
+			die("Seek to %" FMT " 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));
@@ -412,17 +415,17 @@ static void read_shdrs(FILE *fp)
 
 	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
-		die("Unable to allocate %d section headers\n",
+		die("Unable to allocate %ld section headers\n",
 		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
-		die("Seek to %d failed: %s\n",
-			ehdr.e_shoff, strerror(errno));
+		die("Seek to %" FMT " failed: %s\n",
+		    ehdr.e_shoff, strerror(errno));
 	}
 	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",
+			die("Cannot read ELF section headers %d/%ld: %s\n",
 			    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);
@@ -450,12 +453,12 @@ static void read_strtabs(FILE *fp)
 		}
 		sec->strtab = malloc(sec->shdr.sh_size);
 		if (!sec->strtab) {
-			die("malloc of %d bytes for strtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for strtab failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -475,12 +478,12 @@ static void read_symtabs(FILE *fp)
 		}
 		sec->symtab = malloc(sec->shdr.sh_size);
 		if (!sec->symtab) {
-			die("malloc of %d bytes for symtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for symtab failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -508,12 +511,12 @@ static void read_relocs(FILE *fp)
 		}
 		sec->reltab = malloc(sec->shdr.sh_size);
 		if (!sec->reltab) {
-			die("malloc of %d bytes for relocs failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for relocs failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
diff --git a/arch/x86/tools/relocs.h b/arch/x86/tools/relocs.h
index 43c83c0fd22c..4c49c82446eb 100644
--- a/arch/x86/tools/relocs.h
+++ b/arch/x86/tools/relocs.h
@@ -17,6 +17,7 @@
 #include <regex.h>
 #include <tools/le_byteshift.h>
 
+__attribute__((__format__(printf, 1, 2)))
 void die(char *fmt, ...) __attribute__((noreturn));
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [RESEND PATCH] x86/tools/relocs: add __printf attribute to die()
  2021-06-25 21:07             ` Borislav Petkov
@ 2021-06-27 15:01               ` Greg Kroah-Hartman
  2021-06-28 14:25                 ` [PATCH] x86/tools/relocs: Mark die() with the printf function attr format Borislav Petkov
  0 siblings, 1 reply; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-27 15:01 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, x86, Thomas Gleixner, Ingo Molnar, linux-kernel

On Fri, Jun 25, 2021 at 11:07:28PM +0200, Borislav Petkov wrote:
> On Fri, Jun 25, 2021 at 01:38:51PM -0700, H. Peter Anvin wrote:
> > 64-bit cross build on a 32-bit platform... or Windows.
> 
> Meh, nobody cares about those... :)
> 
> Hmm, so looking at the PRI* inttypes.h things again, they're C99 and
> they kinda look more elegant as they don't make us cast stuff.
> 
> So how does that look?
> 
> ---
> 
> diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
> index 04c5a44b9682..2582991ba216 100644
> --- a/arch/x86/tools/relocs.c
> +++ b/arch/x86/tools/relocs.c
> @@ -26,6 +26,9 @@ static struct relocs relocs32;
>  #if ELF_BITS == 64
>  static struct relocs relocs32neg;
>  static struct relocs relocs64;
> +#define FMT PRIu64
> +#else
> +#define FMT PRIu32
>  #endif

<snip>

This works for me!  It should fix the static checking tool that keeps
tripping over this pointless warning :)

Want to turn it into a real patch?

thanks,

greg k-h

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

* [PATCH] x86/tools/relocs: Mark die() with the printf function attr format
  2021-06-27 15:01               ` Greg Kroah-Hartman
@ 2021-06-28 14:25                 ` Borislav Petkov
  2021-06-28 14:34                   ` Greg Kroah-Hartman
  2021-08-23  5:12                   ` [tip: x86/build] " tip-bot2 for Borislav Petkov
  0 siblings, 2 replies; 14+ messages in thread
From: Borislav Petkov @ 2021-06-28 14:25 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: H. Peter Anvin, x86, Thomas Gleixner, Ingo Molnar, linux-kernel

On Sun, Jun 27, 2021 at 05:01:28PM +0200, Greg Kroah-Hartman wrote:
> This works for me!  It should fix the static checking tool that keeps
> tripping over this pointless warning :)
> 
> Want to turn it into a real patch?

How's that?

---
From: Borislav Petkov <bp@suse.de>

Mark die() as a function which accepts printf-style arguments so that
the compiler can typecheck them against the supplied format string.

Use the C99 inttypes.h format specifiers as relocs.c gets built for both
32- and 64-bit.

Original version of the patch by Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Signed-off-by: Borislav Petkov <bp@suse.de>
---
 arch/x86/tools/relocs.c | 37 ++++++++++++++++++++-----------------
 arch/x86/tools/relocs.h |  1 +
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 04c5a44b9682..2582991ba216 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -26,6 +26,9 @@ static struct relocs relocs32;
 #if ELF_BITS == 64
 static struct relocs relocs32neg;
 static struct relocs relocs64;
+#define FMT PRIu64
+#else
+#define FMT PRIu32
 #endif
 
 struct section {
@@ -389,7 +392,7 @@ static void read_ehdr(FILE *fp)
 		Elf_Shdr shdr;
 
 		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
-			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
+			die("Seek to %" FMT " 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));
@@ -412,17 +415,17 @@ static void read_shdrs(FILE *fp)
 
 	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
-		die("Unable to allocate %d section headers\n",
+		die("Unable to allocate %ld section headers\n",
 		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
-		die("Seek to %d failed: %s\n",
-			ehdr.e_shoff, strerror(errno));
+		die("Seek to %" FMT " failed: %s\n",
+		    ehdr.e_shoff, strerror(errno));
 	}
 	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",
+			die("Cannot read ELF section headers %d/%ld: %s\n",
 			    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);
@@ -450,12 +453,12 @@ static void read_strtabs(FILE *fp)
 		}
 		sec->strtab = malloc(sec->shdr.sh_size);
 		if (!sec->strtab) {
-			die("malloc of %d bytes for strtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for strtab failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -475,12 +478,12 @@ static void read_symtabs(FILE *fp)
 		}
 		sec->symtab = malloc(sec->shdr.sh_size);
 		if (!sec->symtab) {
-			die("malloc of %d bytes for symtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for symtab failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -508,12 +511,12 @@ static void read_relocs(FILE *fp)
 		}
 		sec->reltab = malloc(sec->shdr.sh_size);
 		if (!sec->reltab) {
-			die("malloc of %d bytes for relocs failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for relocs failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
diff --git a/arch/x86/tools/relocs.h b/arch/x86/tools/relocs.h
index 43c83c0fd22c..4c49c82446eb 100644
--- a/arch/x86/tools/relocs.h
+++ b/arch/x86/tools/relocs.h
@@ -17,6 +17,7 @@
 #include <regex.h>
 #include <tools/le_byteshift.h>
 
+__attribute__((__format__(printf, 1, 2)))
 void die(char *fmt, ...) __attribute__((noreturn));
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))
-- 
2.29.2

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

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

* Re: [PATCH] x86/tools/relocs: Mark die() with the printf function attr format
  2021-06-28 14:25                 ` [PATCH] x86/tools/relocs: Mark die() with the printf function attr format Borislav Petkov
@ 2021-06-28 14:34                   ` Greg Kroah-Hartman
  2021-08-23  5:12                   ` [tip: x86/build] " tip-bot2 for Borislav Petkov
  1 sibling, 0 replies; 14+ messages in thread
From: Greg Kroah-Hartman @ 2021-06-28 14:34 UTC (permalink / raw)
  To: Borislav Petkov
  Cc: H. Peter Anvin, x86, Thomas Gleixner, Ingo Molnar, linux-kernel

On Mon, Jun 28, 2021 at 04:25:45PM +0200, Borislav Petkov wrote:
> On Sun, Jun 27, 2021 at 05:01:28PM +0200, Greg Kroah-Hartman wrote:
> > This works for me!  It should fix the static checking tool that keeps
> > tripping over this pointless warning :)
> > 
> > Want to turn it into a real patch?
> 
> How's that?
> 
> ---
> From: Borislav Petkov <bp@suse.de>
> 
> Mark die() as a function which accepts printf-style arguments so that
> the compiler can typecheck them against the supplied format string.
> 
> Use the C99 inttypes.h format specifiers as relocs.c gets built for both
> 32- and 64-bit.
> 
> Original version of the patch by Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> 
> Signed-off-by: Borislav Petkov <bp@suse.de>
> ---
>  arch/x86/tools/relocs.c | 37 ++++++++++++++++++++-----------------
>  arch/x86/tools/relocs.h |  1 +
>  2 files changed, 21 insertions(+), 17 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

* [tip: x86/build] x86/tools/relocs: Mark die() with the printf function attr format
  2021-06-28 14:25                 ` [PATCH] x86/tools/relocs: Mark die() with the printf function attr format Borislav Petkov
  2021-06-28 14:34                   ` Greg Kroah-Hartman
@ 2021-08-23  5:12                   ` tip-bot2 for Borislav Petkov
  1 sibling, 0 replies; 14+ messages in thread
From: tip-bot2 for Borislav Petkov @ 2021-08-23  5:12 UTC (permalink / raw)
  To: linux-tip-commits; +Cc: Borislav Petkov, Greg Kroah-Hartman, x86, linux-kernel

The following commit has been merged into the x86/build branch of tip:

Commit-ID:     03dca99e200f4d268f70079cf54e3b1200c9eb9d
Gitweb:        https://git.kernel.org/tip/03dca99e200f4d268f70079cf54e3b1200c9eb9d
Author:        Borislav Petkov <bp@suse.de>
AuthorDate:    Fri, 25 Jun 2021 17:10:16 +02:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 23 Aug 2021 05:58:02 +02:00

x86/tools/relocs: Mark die() with the printf function attr format

Mark die() as a function which accepts printf-style arguments so that
the compiler can typecheck them against the supplied format string.

Use the C99 inttypes.h format specifiers as relocs.c gets built for both
32- and 64-bit.

Original version of the patch by Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Link: http://lkml.kernel.org/r/YNnb6Q4QHtNYC049@zn.tnic
---
 arch/x86/tools/relocs.c | 37 ++++++++++++++++++++-----------------
 arch/x86/tools/relocs.h |  1 +
 2 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/arch/x86/tools/relocs.c b/arch/x86/tools/relocs.c
index 9ba700d..27c8220 100644
--- a/arch/x86/tools/relocs.c
+++ b/arch/x86/tools/relocs.c
@@ -26,6 +26,9 @@ static struct relocs relocs32;
 #if ELF_BITS == 64
 static struct relocs relocs32neg;
 static struct relocs relocs64;
+#define FMT PRIu64
+#else
+#define FMT PRIu32
 #endif
 
 struct section {
@@ -389,7 +392,7 @@ static void read_ehdr(FILE *fp)
 		Elf_Shdr shdr;
 
 		if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0)
-			die("Seek to %d failed: %s\n", ehdr.e_shoff, strerror(errno));
+			die("Seek to %" FMT " 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));
@@ -412,17 +415,17 @@ static void read_shdrs(FILE *fp)
 
 	secs = calloc(shnum, sizeof(struct section));
 	if (!secs) {
-		die("Unable to allocate %d section headers\n",
+		die("Unable to allocate %ld section headers\n",
 		    shnum);
 	}
 	if (fseek(fp, ehdr.e_shoff, SEEK_SET) < 0) {
-		die("Seek to %d failed: %s\n",
-			ehdr.e_shoff, strerror(errno));
+		die("Seek to %" FMT " failed: %s\n",
+		    ehdr.e_shoff, strerror(errno));
 	}
 	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",
+			die("Cannot read ELF section headers %d/%ld: %s\n",
 			    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);
@@ -450,12 +453,12 @@ static void read_strtabs(FILE *fp)
 		}
 		sec->strtab = malloc(sec->shdr.sh_size);
 		if (!sec->strtab) {
-			die("malloc of %d bytes for strtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for strtab failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -475,12 +478,12 @@ static void read_symtabs(FILE *fp)
 		}
 		sec->symtab = malloc(sec->shdr.sh_size);
 		if (!sec->symtab) {
-			die("malloc of %d bytes for symtab failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for symtab failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->symtab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
@@ -508,12 +511,12 @@ static void read_relocs(FILE *fp)
 		}
 		sec->reltab = malloc(sec->shdr.sh_size);
 		if (!sec->reltab) {
-			die("malloc of %d bytes for relocs failed\n",
-				sec->shdr.sh_size);
+			die("malloc of %" FMT " bytes for relocs failed\n",
+			    sec->shdr.sh_size);
 		}
 		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0) {
-			die("Seek to %d failed: %s\n",
-				sec->shdr.sh_offset, strerror(errno));
+			die("Seek to %" FMT " failed: %s\n",
+			    sec->shdr.sh_offset, strerror(errno));
 		}
 		if (fread(sec->reltab, 1, sec->shdr.sh_size, fp)
 		    != sec->shdr.sh_size) {
diff --git a/arch/x86/tools/relocs.h b/arch/x86/tools/relocs.h
index 43c83c0..4c49c82 100644
--- a/arch/x86/tools/relocs.h
+++ b/arch/x86/tools/relocs.h
@@ -17,6 +17,7 @@
 #include <regex.h>
 #include <tools/le_byteshift.h>
 
+__attribute__((__format__(printf, 1, 2)))
 void die(char *fmt, ...) __attribute__((noreturn));
 
 #define ARRAY_SIZE(x) (sizeof(x) / sizeof((x)[0]))

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

end of thread, other threads:[~2021-08-23  5:12 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-24 11:58 [RESEND PATCH] x86/tools/relocs: add __printf attribute to die() Greg Kroah-Hartman
2021-06-24 12:32 ` Borislav Petkov
2021-06-24 14:44   ` Greg Kroah-Hartman
2021-06-25 16:17     ` H. Peter Anvin
2021-06-25 12:06   ` Greg Kroah-Hartman
2021-06-25 14:12     ` Borislav Petkov
2021-06-25 16:19       ` H. Peter Anvin
2021-06-25 16:53         ` Borislav Petkov
2021-06-25 20:38           ` H. Peter Anvin
2021-06-25 21:07             ` Borislav Petkov
2021-06-27 15:01               ` Greg Kroah-Hartman
2021-06-28 14:25                 ` [PATCH] x86/tools/relocs: Mark die() with the printf function attr format Borislav Petkov
2021-06-28 14:34                   ` Greg Kroah-Hartman
2021-08-23  5:12                   ` [tip: x86/build] " tip-bot2 for Borislav Petkov

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.