linux-kernel.vger.kernel.org archive mirror
 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).