[PATCH 0/8] x86: randconfig warning fixes

[PATCH 0/8] x86: randconfig warning fixes

[Arnd Bergmann]

Hi,

I've gone through old patches of mine that I carried in my randconfig
tree, to see which warnings are still present. Here is a set of
fixes for arch/x86. Most of them were sent before at some point
and missed out for one reason or another.

Please have another look and apply what you like.

	Arnd

Arnd Bergmann (8):
  perf/x86: shut up false-positive -Wmaybe-uninitialized warning
  x86: math-emu: possible uninitialized variable use
  x86: math-emu: avoid bogus -Wint-in-bool-context warning
  x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl
  x86: silence build with "make -s"
  x86: add MULTIUSER dependency for KVM
  x86: add PCI dependency for PUNIT_ATOM_DEBUG
  x86: intel-mid: fix a format string overflow warning

 arch/x86/Kconfig.debug                                   |  1 +
 arch/x86/boot/Makefile                                   |  5 +++--
 arch/x86/events/core.c                                   |  4 ++--
 arch/x86/include/asm/io.h                                |  4 ++--
 arch/x86/kvm/Kconfig                                     |  2 +-
 arch/x86/math-emu/Makefile                               |  4 ++--
 arch/x86/math-emu/fpu_emu.h                              |  2 +-
 arch/x86/math-emu/reg_compare.c                          | 16 ++++++++--------
 .../platform/intel-mid/device_libs/platform_max7315.c    |  6 ++++--
 9 files changed, 24 insertions(+), 20 deletions(-)

-- 
2.9.0

[PATCH 1/8] perf/x86: shut up false-positive -Wmaybe-uninitialized warning

[Arnd Bergmann]

The intialization function checks for various failure scenarios, but
unfortunately the compiler gets a little confused about the possible
combinations, leading to a false-positive build warning when
-Wmaybe-uninitialized is set:

arch/x86/events/core.c: In function ‘init_hw_perf_events’:
arch/x86/events/core.c:264:3: warning: ‘reg_fail’ may be used uninitialized in this function [-Wmaybe-uninitialized]
arch/x86/events/core.c:264:3: warning: ‘val_fail’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   pr_err(FW_BUG "the BIOS has corrupted hw-PMU resources (MSR %x is %Lx)\n",

We can't actually run into this case, so this shuts up the warning
by initializing the variables to a known-invalid state.

Link: https://patchwork.kernel.org/patch/9392595/
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: replaced original patch that reordered the code instead of
adding a fake initialization.
---
 arch/x86/events/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index ff1ea2fb9705..8e3db8f642a7 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -191,8 +191,8 @@ static void release_pmc_hardware(void) {}
 
 static bool check_hw_exists(void)
 {
-	u64 val, val_fail, val_new= ~0;
-	int i, reg, reg_fail, ret = 0;
+	u64 val, val_fail = -1, val_new= ~0;
+	int i, reg, reg_fail = -1, ret = 0;
 	int bios_fail = 0;
 	int reg_safe = -1;
 
-- 
2.9.0

[tip:x86/urgent] perf/x86: Shut up false-positive -Wmaybe-uninitialized warning

[tip-bot for Arnd Bergmann]

Commit-ID:  11d8b05855f3749bcb6c57e2c4052921b9605c77
Gitweb:     http://git.kernel.org/tip/11d8b05855f3749bcb6c57e2c4052921b9605c77
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 19 Jul 2017 14:52:59 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 10:46:23 +0200

perf/x86: Shut up false-positive -Wmaybe-uninitialized warning

The intialization function checks for various failure scenarios, but
unfortunately the compiler gets a little confused about the possible
combinations, leading to a false-positive build warning when
-Wmaybe-uninitialized is set:

  arch/x86/events/core.c: In function ‘init_hw_perf_events’:
  arch/x86/events/core.c:264:3: warning: ‘reg_fail’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  arch/x86/events/core.c:264:3: warning: ‘val_fail’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     pr_err(FW_BUG "the BIOS has corrupted hw-PMU resources (MSR %x is %Lx)\n",

We can't actually run into this case, so this shuts up the warning
by initializing the variables to a known-invalid state.

Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Alexander Shishkin <alexander.shishkin@linux.intel.com>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170719125310.2487451-2-arnd@arndb.de
Link: https://patchwork.kernel.org/patch/9392595/
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/events/core.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c
index ff1ea2f..8e3db8f 100644
--- a/arch/x86/events/core.c
+++ b/arch/x86/events/core.c
@@ -191,8 +191,8 @@ static void release_pmc_hardware(void) {}
 
 static bool check_hw_exists(void)
 {
-	u64 val, val_fail, val_new= ~0;
-	int i, reg, reg_fail, ret = 0;
+	u64 val, val_fail = -1, val_new= ~0;
+	int i, reg, reg_fail = -1, ret = 0;
 	int bios_fail = 0;
 	int reg_safe = -1;
 

[PATCH 2/8] x86: math-emu: possible uninitialized variable use

[Arnd Bergmann]

When building the kernel with "make EXTRA_CFLAGS=...", this overrides
the "PARANOID" preprocessor macro defined in arch/x86/math-emu/Makefile,
and we run into a build warning:

arch/x86/math-emu/reg_compare.c: In function ‘compare_i_st_st’:
arch/x86/math-emu/reg_compare.c:254:6: error: ‘f’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This fixes the implementation to work correctly even without the PARANOID
flag, and also fixes the Makefile to not use the EXTRA_CFLAGS variable
but instead use the ccflags-y variable in the Makefile that is meant
for this purpose.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Originally sent on Oct. 17 2016, resending unmodified
---
 arch/x86/math-emu/Makefile      |  4 ++--
 arch/x86/math-emu/reg_compare.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/math-emu/Makefile b/arch/x86/math-emu/Makefile
index 9b0c63b60302..1b2dac174321 100644
--- a/arch/x86/math-emu/Makefile
+++ b/arch/x86/math-emu/Makefile
@@ -5,8 +5,8 @@
 #DEBUG	= -DDEBUGGING
 DEBUG	=
 PARANOID = -DPARANOID
-EXTRA_CFLAGS	:= $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
-EXTRA_AFLAGS	:= $(PARANOID)
+ccflags-y += $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
+asflags-y += $(PARANOID)
 
 # From 'C' language sources:
 C_OBJS =fpu_entry.o errors.o \
diff --git a/arch/x86/math-emu/reg_compare.c b/arch/x86/math-emu/reg_compare.c
index b77360fdbf4a..19b33b50adfa 100644
--- a/arch/x86/math-emu/reg_compare.c
+++ b/arch/x86/math-emu/reg_compare.c
@@ -168,7 +168,7 @@ static int compare(FPU_REG const *b, int tagb)
 /* This function requires that st(0) is not empty */
 int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 {
-	int f = 0, c;
+	int f, c;
 
 	c = compare(loaded_data, loaded_tag);
 
@@ -189,12 +189,12 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x121);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -205,7 +205,7 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 
 static int compare_st_st(int nr)
 {
-	int f = 0, c;
+	int f, c;
 	FPU_REG *st_ptr;
 
 	if (!NOT_EMPTY(0) || !NOT_EMPTY(nr)) {
@@ -235,12 +235,12 @@ static int compare_st_st(int nr)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -283,12 +283,12 @@ static int compare_i_st_st(int nr)
 	case COMP_No_Comp:
 		f = X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF;
 		break;
-#ifdef PARANOID
 	default:
+#ifdef PARANOID
 		EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 		f = 0;
 		break;
-#endif /* PARANOID */
 	}
 	FPU_EFLAGS = (FPU_EFLAGS & ~(X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF)) | f;
 	if (c & COMP_Denormal) {
-- 
2.9.0

[tip:x86/urgent] x86/fpu/math-emu: Fix possible uninitialized variable use

[tip-bot for Arnd Bergmann]

Commit-ID:  75e2f0a6b16141cb347f442033ec907380d4d66e
Gitweb:     http://git.kernel.org/tip/75e2f0a6b16141cb347f442033ec907380d4d66e
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 19 Jul 2017 14:53:00 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 10:46:24 +0200

x86/fpu/math-emu: Fix possible uninitialized variable use

When building the kernel with "make EXTRA_CFLAGS=...", this overrides
the "PARANOID" preprocessor macro defined in arch/x86/math-emu/Makefile,
and we run into a build warning:

  arch/x86/math-emu/reg_compare.c: In function ‘compare_i_st_st’:
  arch/x86/math-emu/reg_compare.c:254:6: error: ‘f’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

This fixes the implementation to work correctly even without the PARANOID
flag, and also fixes the Makefile to not use the EXTRA_CFLAGS variable
but instead use the ccflags-y variable in the Makefile that is meant
for this purpose.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Bill Metzenthen <billm@melbpc.org.au>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170719125310.2487451-3-arnd@arndb.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/math-emu/Makefile      |  4 ++--
 arch/x86/math-emu/reg_compare.c | 16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/arch/x86/math-emu/Makefile b/arch/x86/math-emu/Makefile
index 9b0c63b..1b2dac1 100644
--- a/arch/x86/math-emu/Makefile
+++ b/arch/x86/math-emu/Makefile
@@ -5,8 +5,8 @@
 #DEBUG	= -DDEBUGGING
 DEBUG	=
 PARANOID = -DPARANOID
-EXTRA_CFLAGS	:= $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
-EXTRA_AFLAGS	:= $(PARANOID)
+ccflags-y += $(PARANOID) $(DEBUG) -fno-builtin $(MATH_EMULATION)
+asflags-y += $(PARANOID)
 
 # From 'C' language sources:
 C_OBJS =fpu_entry.o errors.o \
diff --git a/arch/x86/math-emu/reg_compare.c b/arch/x86/math-emu/reg_compare.c
index b77360f..19b33b5 100644
--- a/arch/x86/math-emu/reg_compare.c
+++ b/arch/x86/math-emu/reg_compare.c
@@ -168,7 +168,7 @@ static int compare(FPU_REG const *b, int tagb)
 /* This function requires that st(0) is not empty */
 int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 {
-	int f = 0, c;
+	int f, c;
 
 	c = compare(loaded_data, loaded_tag);
 
@@ -189,12 +189,12 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x121);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -205,7 +205,7 @@ int FPU_compare_st_data(FPU_REG const *loaded_data, u_char loaded_tag)
 
 static int compare_st_st(int nr)
 {
-	int f = 0, c;
+	int f, c;
 	FPU_REG *st_ptr;
 
 	if (!NOT_EMPTY(0) || !NOT_EMPTY(nr)) {
@@ -235,12 +235,12 @@ static int compare_st_st(int nr)
 		case COMP_No_Comp:
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#ifdef PARANOID
 		default:
+#ifdef PARANOID
 			EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 			f = SW_C3 | SW_C2 | SW_C0;
 			break;
-#endif /* PARANOID */
 		}
 	setcc(f);
 	if (c & COMP_Denormal) {
@@ -283,12 +283,12 @@ static int compare_i_st_st(int nr)
 	case COMP_No_Comp:
 		f = X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF;
 		break;
-#ifdef PARANOID
 	default:
+#ifdef PARANOID
 		EXCEPTION(EX_INTERNAL | 0x122);
+#endif /* PARANOID */
 		f = 0;
 		break;
-#endif /* PARANOID */
 	}
 	FPU_EFLAGS = (FPU_EFLAGS & ~(X86_EFLAGS_ZF | X86_EFLAGS_PF | X86_EFLAGS_CF)) | f;
 	if (c & COMP_Denormal) {

[PATCH 3/8] x86: math-emu: avoid bogus -Wint-in-bool-context warning

[Arnd Bergmann]

gcc-7.1.1 produces this warning:

arch/x86/math-emu/reg_add_sub.c: In function 'FPU_add':
arch/x86/math-emu/reg_add_sub.c:80:48: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context]

This appears to be a bug in gcc-7.1.1, and I have reported it as
PR81484. The compiler suggests that code written as

	if (a & b ? c : d)

is usually incorrect and should have been

	if (a & (b ? c : d))

However, in this case, we correctly write

	if ((a & b) ? c : d)

and should not get a warning for it.

This adds a dirty workaround for the problem, adding a comparison with
zero inside of the macro. The warning is currently disabled in the kernel,
so we may decide not to apply the patch, and instead wait for future gcc
releases to fix the problem. On the other hand, it seems to be the
only instance of this particular problem.

Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81484
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Originally sent on July 14, this is the same patch again with
an rewritten changelog.
---
 arch/x86/math-emu/fpu_emu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/math-emu/fpu_emu.h b/arch/x86/math-emu/fpu_emu.h
index afbc4d805d66..c9c320dccca1 100644
--- a/arch/x86/math-emu/fpu_emu.h
+++ b/arch/x86/math-emu/fpu_emu.h
@@ -157,7 +157,7 @@ extern u_char const data_sizes_16[32];
 
 #define signbyte(a) (((u_char *)(a))[9])
 #define getsign(a) (signbyte(a) & 0x80)
-#define setsign(a,b) { if (b) signbyte(a) |= 0x80; else signbyte(a) &= 0x7f; }
+#define setsign(a,b) { if ((b) != 0) signbyte(a) |= 0x80; else signbyte(a) &= 0x7f; }
 #define copysign(a,b) { if (getsign(a)) signbyte(b) |= 0x80; \
                         else signbyte(b) &= 0x7f; }
 #define changesign(a) { signbyte(a) ^= 0x80; }
-- 
2.9.0

[tip:x86/urgent] x86/fpu/math-emu: Avoid bogus -Wint-in-bool-context warning

[tip-bot for Arnd Bergmann]

Commit-ID:  5623452a0eaec1d44cc9f0770444a48847c9953f
Gitweb:     http://git.kernel.org/tip/5623452a0eaec1d44cc9f0770444a48847c9953f
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 19 Jul 2017 14:53:01 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 10:46:24 +0200

x86/fpu/math-emu: Avoid bogus -Wint-in-bool-context warning

gcc-7.1.1 produces this warning:

  arch/x86/math-emu/reg_add_sub.c: In function 'FPU_add':
  arch/x86/math-emu/reg_add_sub.c:80:48: error: ?: using integer constants in boolean context [-Werror=int-in-bool-context]

This appears to be a bug in gcc-7.1.1, and I have reported it as
PR81484. The compiler suggests that code written as

	if (a & b ? c : d)

is usually incorrect and should have been

	if (a & (b ? c : d))

However, in this case, we correctly write

	if ((a & b) ? c : d)

and should not get a warning for it.

This adds a dirty workaround for the problem, adding a comparison with
zero inside of the macro. The warning is currently disabled in the kernel,
so we may decide not to apply the patch, and instead wait for future gcc
releases to fix the problem. On the other hand, it seems to be the
only instance of this particular problem.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Bill Metzenthen <billm@melbpc.org.au>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170719125310.2487451-4-arnd@arndb.de
Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81484
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/math-emu/fpu_emu.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/math-emu/fpu_emu.h b/arch/x86/math-emu/fpu_emu.h
index afbc4d8..c9c320d 100644
--- a/arch/x86/math-emu/fpu_emu.h
+++ b/arch/x86/math-emu/fpu_emu.h
@@ -157,7 +157,7 @@ extern u_char const data_sizes_16[32];
 
 #define signbyte(a) (((u_char *)(a))[9])
 #define getsign(a) (signbyte(a) & 0x80)
-#define setsign(a,b) { if (b) signbyte(a) |= 0x80; else signbyte(a) &= 0x7f; }
+#define setsign(a,b) { if ((b) != 0) signbyte(a) |= 0x80; else signbyte(a) &= 0x7f; }
 #define copysign(a,b) { if (getsign(a)) signbyte(b) |= 0x80; \
                         else signbyte(b) &= 0x7f; }
 #define changesign(a) { signbyte(a) ^= 0x80; }

[PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl

[Arnd Bergmann]

The x86 version of insb/insw/insl uses an inline assembly that does
not have the target buffer listed as an output. This can confuse
the compiler, leading it to think that a subsequent access of the
buffer is uninitialized:

drivers/net/wireless/wl3501_cs.c: In function ‘wl3501_mgmt_scan_confirm’:
drivers/net/wireless/wl3501_cs.c:665:9: error: ‘sig.status’ is used uninitialized in this function [-Werror=uninitialized]
drivers/net/wireless/wl3501_cs.c:668:12: error: ‘sig.cap_info’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/net/sb1000.c: In function 'sb1000_rx':
drivers/net/sb1000.c:775:9: error: 'st[0]' is used uninitialized in this function [-Werror=uninitialized]
drivers/net/sb1000.c:776:10: error: 'st[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized]
drivers/net/sb1000.c:784:11: error: 'st[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized]

I tried to mark the exact input buffer as an output here, but couldn't
figure it out. As suggested by Linus, marking all memory as clobbered
however is good enough too. For the outs operations, I also add the
memory clobber, to force the input to be written to local variables.
This is probably already guaranteed by the "asm volatile", but it can't
hurt to do this for symmetry.

Link: https://lkml.org/lkml/2017/7/12/605
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
v2: replace broken original RFC patch with one that probably works.
    The crazy stack usage I reported in teh wl3501_cs driver with
    my original patch is gone too now.
---
 arch/x86/include/asm/io.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index e080a39b2108..4bc6f459a8b6 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -328,13 +328,13 @@ static inline unsigned type in##bwl##_p(int port)			\
 static inline void outs##bwl(int port, const void *addr, unsigned long count) \
 {									\
 	asm volatile("rep; outs" #bwl					\
-		     : "+S"(addr), "+c"(count) : "d"(port));		\
+		     : "+S"(addr), "+c"(count) : "d"(port) : "memory");	\
 }									\
 									\
 static inline void ins##bwl(int port, void *addr, unsigned long count)	\
 {									\
 	asm volatile("rep; ins" #bwl					\
-		     : "+D"(addr), "+c"(count) : "d"(port));		\
+		     : "+D"(addr), "+c"(count) : "d"(port) : "memory");	\
 }
 
 BUILDIO(b, b, char)
-- 
2.9.0

Re: [PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl

[Arnd Bergmann]

[adding Linus to Cc, apparently git send-email ignores the Suggested-by
tag for the cc-list]

On Wed, Jul 19, 2017 at 2:53 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> The x86 version of insb/insw/insl uses an inline assembly that does
> not have the target buffer listed as an output. This can confuse
> the compiler, leading it to think that a subsequent access of the
> buffer is uninitialized:
>
> drivers/net/wireless/wl3501_cs.c: In function ‘wl3501_mgmt_scan_confirm’:
> drivers/net/wireless/wl3501_cs.c:665:9: error: ‘sig.status’ is used uninitialized in this function [-Werror=uninitialized]
> drivers/net/wireless/wl3501_cs.c:668:12: error: ‘sig.cap_info’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/net/sb1000.c: In function 'sb1000_rx':
> drivers/net/sb1000.c:775:9: error: 'st[0]' is used uninitialized in this function [-Werror=uninitialized]
> drivers/net/sb1000.c:776:10: error: 'st[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized]
> drivers/net/sb1000.c:784:11: error: 'st[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> I tried to mark the exact input buffer as an output here, but couldn't
> figure it out. As suggested by Linus, marking all memory as clobbered
> however is good enough too. For the outs operations, I also add the
> memory clobber, to force the input to be written to local variables.
> This is probably already guaranteed by the "asm volatile", but it can't
> hurt to do this for symmetry.
>
> Link: https://lkml.org/lkml/2017/7/12/605
> Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> v2: replace broken original RFC patch with one that probably works.
>     The crazy stack usage I reported in teh wl3501_cs driver with
>     my original patch is gone too now.
> ---
>  arch/x86/include/asm/io.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
> index e080a39b2108..4bc6f459a8b6 100644
> --- a/arch/x86/include/asm/io.h
> +++ b/arch/x86/include/asm/io.h
> @@ -328,13 +328,13 @@ static inline unsigned type in##bwl##_p(int port)                 \
>  static inline void outs##bwl(int port, const void *addr, unsigned long count) \
>  {                                                                      \
>         asm volatile("rep; outs" #bwl                                   \
> -                    : "+S"(addr), "+c"(count) : "d"(port));            \
> +                    : "+S"(addr), "+c"(count) : "d"(port) : "memory"); \
>  }                                                                      \
>                                                                         \
>  static inline void ins##bwl(int port, void *addr, unsigned long count) \
>  {                                                                      \
>         asm volatile("rep; ins" #bwl                                    \
> -                    : "+D"(addr), "+c"(count) : "d"(port));            \
> +                    : "+D"(addr), "+c"(count) : "d"(port) : "memory"); \
>  }
>
>  BUILDIO(b, b, char)
> --
> 2.9.0
>

Re: [PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl

[Linus Torvalds]

On Wed, Jul 19, 2017 at 5:53 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>
> I tried to mark the exact input buffer as an output here, but couldn't
> figure it out. As suggested by Linus, marking all memory as clobbered
> however is good enough too. For the outs operations, I also add the
> memory clobber, to force the input to be written to local variables.
> This is probably already guaranteed by the "asm volatile", but it can't
> hurt to do this for symmetry.

Ack, obviously looks fine to me.

                 Linus

[tip:x86/urgent] x86/io: Add "memory" clobber to insb/insw/insl/outsb/outsw/outsl

[tip-bot for Arnd Bergmann]

Commit-ID:  7206f9bf108eb9513d170c73f151367a1bdf3dbf
Gitweb:     http://git.kernel.org/tip/7206f9bf108eb9513d170c73f151367a1bdf3dbf
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 19 Jul 2017 14:53:02 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 10:46:24 +0200

x86/io: Add "memory" clobber to insb/insw/insl/outsb/outsw/outsl

The x86 version of insb/insw/insl uses an inline assembly that does
not have the target buffer listed as an output. This can confuse
the compiler, leading it to think that a subsequent access of the
buffer is uninitialized:

  drivers/net/wireless/wl3501_cs.c: In function ‘wl3501_mgmt_scan_confirm’:
  drivers/net/wireless/wl3501_cs.c:665:9: error: ‘sig.status’ is used uninitialized in this function [-Werror=uninitialized]
  drivers/net/wireless/wl3501_cs.c:668:12: error: ‘sig.cap_info’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  drivers/net/sb1000.c: In function 'sb1000_rx':
  drivers/net/sb1000.c:775:9: error: 'st[0]' is used uninitialized in this function [-Werror=uninitialized]
  drivers/net/sb1000.c:776:10: error: 'st[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized]
  drivers/net/sb1000.c:784:11: error: 'st[1]' may be used uninitialized in this function [-Werror=maybe-uninitialized]

I tried to mark the exact input buffer as an output here, but couldn't
figure it out. As suggested by Linus, marking all memory as clobbered
however is good enough too. For the outs operations, I also add the
memory clobber, to force the input to be written to local variables.
This is probably already guaranteed by the "asm volatile", but it can't
hurt to do this for symmetry.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Borislav Petkov <bp@suse.de>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tom Lendacky <thomas.lendacky@amd.com>
Link: http://lkml.kernel.org/r/20170719125310.2487451-5-arnd@arndb.de
Link: https://lkml.org/lkml/2017/7/12/605
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/include/asm/io.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 7afb0e2..48febf0 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -328,13 +328,13 @@ static inline unsigned type in##bwl##_p(int port)			\
 static inline void outs##bwl(int port, const void *addr, unsigned long count) \
 {									\
 	asm volatile("rep; outs" #bwl					\
-		     : "+S"(addr), "+c"(count) : "d"(port));		\
+		     : "+S"(addr), "+c"(count) : "d"(port) : "memory");	\
 }									\
 									\
 static inline void ins##bwl(int port, void *addr, unsigned long count)	\
 {									\
 	asm volatile("rep; ins" #bwl					\
-		     : "+D"(addr), "+c"(count) : "d"(port));		\
+		     : "+D"(addr), "+c"(count) : "d"(port) : "memory");	\
 }
 
 BUILDIO(b, b, char)

[PATCH 5/8] x86: silence build with "make -s"

[Arnd Bergmann]

Every kernel build on x86 will result in some output:

Setup is 13084 bytes (padded to 13312 bytes).
System is 4833 kB
CRC 6d35fa35
Kernel: arch/x86/boot/bzImage is ready  (#2)

This shuts it up, so that 'make -s' is truely silent as long as
everything works. Building without '-s' should produce unchanged
output.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/boot/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 0d810fb15eac..d88a2fddba8c 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -73,12 +73,13 @@ UBSAN_SANITIZE := n
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
 
 quiet_cmd_image = BUILD   $@
+silent_redirect_image = >/dev/null
 cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
-			       $(obj)/zoffset.h $@
+			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
 
 $(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
 	$(call if_changed,image)
-	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
+	@$(kecho) 'Kernel: $@ is ready' ' (#'`cat .version`')'
 
 OBJCOPYFLAGS_vmlinux.bin := -O binary -R .note -R .comment -S
 $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE
-- 
2.9.0

[tip:x86/urgent] x86/build: Silence the build with "make -s"

[tip-bot for Arnd Bergmann]

Commit-ID:  d460131dd50599e0e9405d5f4ae02c27d529a44a
Gitweb:     http://git.kernel.org/tip/d460131dd50599e0e9405d5f4ae02c27d529a44a
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 19 Jul 2017 14:53:03 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 10:46:24 +0200

x86/build: Silence the build with "make -s"

Every kernel build on x86 will result in some output:

  Setup is 13084 bytes (padded to 13312 bytes).
  System is 4833 kB
  CRC 6d35fa35
  Kernel: arch/x86/boot/bzImage is ready  (#2)

This shuts it up, so that 'make -s' is truely silent as long as
everything works. Building without '-s' should produce unchanged
output.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170719125310.2487451-6-arnd@arndb.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/boot/Makefile | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index 0d810fb..d88a2fd 100644
--- a/arch/x86/boot/Makefile
+++ b/arch/x86/boot/Makefile
@@ -73,12 +73,13 @@ UBSAN_SANITIZE := n
 $(obj)/bzImage: asflags-y  := $(SVGA_MODE)
 
 quiet_cmd_image = BUILD   $@
+silent_redirect_image = >/dev/null
 cmd_image = $(obj)/tools/build $(obj)/setup.bin $(obj)/vmlinux.bin \
-			       $(obj)/zoffset.h $@
+			       $(obj)/zoffset.h $@ $($(quiet)redirect_image)
 
 $(obj)/bzImage: $(obj)/setup.bin $(obj)/vmlinux.bin $(obj)/tools/build FORCE
 	$(call if_changed,image)
-	@echo 'Kernel: $@ is ready' ' (#'`cat .version`')'
+	@$(kecho) 'Kernel: $@ is ready' ' (#'`cat .version`')'
 
 OBJCOPYFLAGS_vmlinux.bin := -O binary -R .note -R .comment -S
 $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE

[PATCH 6/8] x86: add MULTIUSER dependency for KVM

[Arnd Bergmann]

KVM tries to select 'TASKSTATS', which had additional dependencies:

warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER)

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/kvm/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
index 760433b2574a..2688c7dc5323 100644
--- a/arch/x86/kvm/Kconfig
+++ b/arch/x86/kvm/Kconfig
@@ -22,7 +22,7 @@ config KVM
 	depends on HAVE_KVM
 	depends on HIGH_RES_TIMERS
 	# for TASKSTATS/TASK_DELAY_ACCT:
-	depends on NET
+	depends on NET && MULTIUSER
 	select PREEMPT_NOTIFIERS
 	select MMU_NOTIFIER
 	select ANON_INODES
-- 
2.9.0

Re: [PATCH 6/8] x86: add MULTIUSER dependency for KVM

[Radim Krčmář]

2017-07-19 14:53+0200, Arnd Bergmann:
> KVM tries to select 'TASKSTATS', which had additional dependencies:
> 
> warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER)
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---

Hm, do you know why Kconfig warns instead of propagating the
dependencies?

> diff --git a/arch/x86/kvm/Kconfig b/arch/x86/kvm/Kconfig
> @@ -22,7 +22,7 @@ config KVM
>  	# for TASKSTATS/TASK_DELAY_ACCT:
> -	depends on NET
> +	depends on NET && MULTIUSER

The current condition goes halfway to nowhere, so the patch is
definitely an improvement, even if the result is not good ...

Applied, thanks.

Re: [PATCH 6/8] x86: add MULTIUSER dependency for KVM

[Arnd Bergmann]

On Wed, Jul 19, 2017 at 4:11 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> 2017-07-19 14:53+0200, Arnd Bergmann:
>> KVM tries to select 'TASKSTATS', which had additional dependencies:
>>
>> warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER)
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>> ---
>
> Hm, do you know why Kconfig warns instead of propagating the
> dependencies?

Kconfig propagates 'depends on' dependencies, but cannot turn a 'select'
into 'depends on', as those two mean different things.

Another solution to the problem would be to use 'depends on TASKSTATS'.

Generally speaking, using 'select' to turn on a user-visible option is a bad
idea, but blindly turning those 'select' into 'depends on' is also dangerous,
as it can break configurations of existing users that would here end up with
neither TASKSTATS nor KVM after a 'make oldconfig'.

      Arnd

Re: [PATCH 6/8] x86: add MULTIUSER dependency for KVM

[Radim Krčmář]

2017-07-19 16:18+0200, Arnd Bergmann:
> On Wed, Jul 19, 2017 at 4:11 PM, Radim Krčmář <rkrcmar@redhat.com> wrote:
> > 2017-07-19 14:53+0200, Arnd Bergmann:
> >> KVM tries to select 'TASKSTATS', which had additional dependencies:
> >>
> >> warning: (KVM) selects TASKSTATS which has unmet direct dependencies (NET && MULTIUSER)
> >>
> >> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> >> ---
> >
> > Hm, do you know why Kconfig warns instead of propagating the
> > dependencies?
> 
> Kconfig propagates 'depends on' dependencies, but cannot turn a 'select'
> into 'depends on', as those two mean different things.
> 
> Another solution to the problem would be to use 'depends on TASKSTATS'.

Good point, 'select' seems misused here.

There is no reason to depend on TASKSTATS (nor NET+MULTIUSER), we only
suggest to enable it with KVM.  KVM uses sched_info_on() to handle any
any possible resulting configuration, c9aaa8957f20 ("KVM: Steal time
implementation").

KVM would work as intended if 'select' would not enable the option if
its dependencies failed (instead of unconditionally forcing the option).

Is the preferred way to encode it:

  'default y if KVM' in config TASK_DELAY_ACCT
  (that adds a non-local and enigmatic dependency and also needlessly
   expands the possible configuration space)

or

  'select TASKSTATS if NET && MULTIUSER' in config KVM
  (that is going to break when dependencies of TASKSTATS change again)

?

thanks.

Re: [PATCH 6/8] x86: add MULTIUSER dependency for KVM

[Paolo Bonzini]

On 19/07/2017 18:13, Radim Krčmář wrote:
> Good point, 'select' seems misused here.
> 
> There is no reason to depend on TASKSTATS (nor NET+MULTIUSER), we only
> suggest to enable it with KVM.  KVM uses sched_info_on() to handle any
> any possible resulting configuration, c9aaa8957f20 ("KVM: Steal time
> implementation").
> 
> KVM would work as intended if 'select' would not enable the option if
> its dependencies failed (instead of unconditionally forcing the option).
> 
> Is the preferred way to encode it:
> 
>   'default y if KVM' in config TASK_DELAY_ACCT
>   (that adds a non-local and enigmatic dependency and also needlessly
>    expands the possible configuration space)
> 
> or
> 
>   'select TASKSTATS if NET && MULTIUSER' in config KVM
>   (that is going to break when dependencies of TASKSTATS change again)
> 
> ?

I think the former is the closest to what the user actually wants, and
it would let us clean up arch/x86/kvm/Kconfig.  However it should be
"default y if KVM && X86'.

Maybe there is room for a new operator "suggest Y" which, when added
inside "config X", operates as if "config Y" had a "default y if X".

In this case, kvm could do

-	depends on NET && MULTIUSER
-	select TASKSTATS
+	suggest TASKSTATS

Thanks,

Paolo

[PATCH 7/8] x86: add PCI dependency for PUNIT_ATOM_DEBUG

[Arnd Bergmann]

The IOSF_MBI option requires PCI support, without it we get a harmless
Kconfig warning when it gets selected by PUNIT_ATOM_DEBUG:

warning: (X86_INTEL_LPSS && SND_SST_IPC_ACPI && MMC_SDHCI_ACPI && PUNIT_ATOM_DEBUG) selects IOSF_MBI which has unmet direct dependencies (PCI)

This adds another dependency to avoid the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/Kconfig.debug | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 353ed09f5fba..1fc519f3c49e 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -346,6 +346,7 @@ config X86_DEBUG_FPU
 
 config PUNIT_ATOM_DEBUG
 	tristate "ATOM Punit debug driver"
+	depends on PCI
 	select DEBUG_FS
 	select IOSF_MBI
 	---help---
-- 
2.9.0

[tip:x86/urgent] x86/platform: Add PCI dependency for PUNIT_ATOM_DEBUG

[tip-bot for Arnd Bergmann]

Commit-ID:  d689c64d189e43d782fec5649fb0afe303c5b3f9
Gitweb:     http://git.kernel.org/tip/d689c64d189e43d782fec5649fb0afe303c5b3f9
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 19 Jul 2017 14:53:05 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 10:46:24 +0200

x86/platform: Add PCI dependency for PUNIT_ATOM_DEBUG

The IOSF_MBI option requires PCI support, without it we get a harmless
Kconfig warning when it gets selected by PUNIT_ATOM_DEBUG:

  warning: (X86_INTEL_LPSS && SND_SST_IPC_ACPI && MMC_SDHCI_ACPI && PUNIT_ATOM_DEBUG) selects IOSF_MBI which has unmet direct dependencies (PCI)

This adds another dependency to avoid the warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Andy Lutomirski <luto@kernel.org>
Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170719125310.2487451-8-arnd@arndb.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/Kconfig.debug | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index fcb7604..cd20ca0 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -348,6 +348,7 @@ config X86_DEBUG_FPU
 
 config PUNIT_ATOM_DEBUG
 	tristate "ATOM Punit debug driver"
+	depends on PCI
 	select DEBUG_FS
 	select IOSF_MBI
 	---help---

[PATCH 8/8] x86: intel-mid: fix a format string overflow warning

[Arnd Bergmann]

We have space for exactly three characters for the index in "max7315_%d_base",
but as gcc points out having more would cause an string overflow:

arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In function 'max7315_platform_data':
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26: error: '%d' directive writing between 1 and 11 bytes into a region of size 9 [-Werror=format-overflow=]
   sprintf(base_pin_name, "max7315_%d_base", nr);
                          ^~~~~~~~~~~~~~~~~
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26: note: directive argument in the range [-2147483647, 2147483647]
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:3: note: 'sprintf' output between 15 and 25 bytes into a destination of size 17
   sprintf(base_pin_name, "max7315_%d_base", nr);

This makes it use an snprintf() to truncate the string if that happened
rather than overflowing the stack. In practice, this is safe, because
there won't be a large number of max7315 devices in the systems, and
both the format and the length are defined by the firmware interface.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Originally submitted on July 14, this is the same patch with slightly
improved changelog.
---
 arch/x86/platform/intel-mid/device_libs/platform_max7315.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
index 6e075afa7877..58337b2bc682 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
@@ -38,8 +38,10 @@ static void __init *max7315_platform_data(void *info)
 	 */
 	strcpy(i2c_info->type, "max7315");
 	if (nr++) {
-		sprintf(base_pin_name, "max7315_%d_base", nr);
-		sprintf(intr_pin_name, "max7315_%d_int", nr);
+		snprintf(base_pin_name, sizeof(base_pin_name),
+			 "max7315_%d_base", nr);
+		snprintf(intr_pin_name, sizeof(intr_pin_name),
+			 "max7315_%d_int", nr);
 	} else {
 		strcpy(base_pin_name, "max7315_base");
 		strcpy(intr_pin_name, "max7315_int");
-- 
2.9.0

[tip:x86/urgent] x86/platform/intel-mid: Fix a format string overflow warning

[tip-bot for Arnd Bergmann]

Commit-ID:  0bc73048d7baecf94117d1a948853a627e6ba5c8
Gitweb:     http://git.kernel.org/tip/0bc73048d7baecf94117d1a948853a627e6ba5c8
Author:     Arnd Bergmann <arnd@arndb.de>
AuthorDate: Wed, 19 Jul 2017 14:53:06 +0200
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Thu, 20 Jul 2017 10:46:25 +0200

x86/platform/intel-mid: Fix a format string overflow warning

We have space for exactly three characters for the index in "max7315_%d_base",
but as GCC points out having more would cause an string overflow:

  arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In function 'max7315_platform_data':
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26: error: '%d' directive writing between 1 and 11 bytes into a region of size 9 [-Werror=format-overflow=]
     sprintf(base_pin_name, "max7315_%d_base", nr);
                          ^~~~~~~~~~~~~~~~~
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26: note: directive argument in the range [-2147483647, 2147483647]
  arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:3: note: 'sprintf' output between 15 and 25 bytes into a destination of size 17
     sprintf(base_pin_name, "max7315_%d_base", nr);

This makes it use an snprintf() to truncate the string if that happened
rather than overflowing the stack. In practice, this is safe, because
there won't be a large number of max7315 devices in the systems, and
both the format and the length are defined by the firmware interface.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/20170719125310.2487451-9-arnd@arndb.de
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 arch/x86/platform/intel-mid/device_libs/platform_max7315.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
index 6e075af..58337b2 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
@@ -38,8 +38,10 @@ static void __init *max7315_platform_data(void *info)
 	 */
 	strcpy(i2c_info->type, "max7315");
 	if (nr++) {
-		sprintf(base_pin_name, "max7315_%d_base", nr);
-		sprintf(intr_pin_name, "max7315_%d_int", nr);
+		snprintf(base_pin_name, sizeof(base_pin_name),
+			 "max7315_%d_base", nr);
+		snprintf(intr_pin_name, sizeof(intr_pin_name),
+			 "max7315_%d_int", nr);
 	} else {
 		strcpy(base_pin_name, "max7315_base");
 		strcpy(intr_pin_name, "max7315_int");