linux-wireless.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [RFC 1/2] x86: mark target address as output in 'insb' asm
@ 2017-07-10 14:44 Arnd Bergmann
  2017-07-10 14:44 ` [RFC 2/2] wl3501_cs: reduce stack size for KASAN Arnd Bergmann
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2017-07-10 14:44 UTC (permalink / raw)
  To: x86
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Kalle Valo,
	linux-kernel, linux-wireless, Arnd Bergmann

The -Wmaybe-uninitialized warning triggers for one driver using the output
of the 'insb' I/O helper on x86:

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]

Apparently the assember constraints are slightly off here, as marking the
'addr' argument as a memory output seems appropriate here and gets rid
of the warning. For consistency I'm also adding it as input for outsb().

Unfortunately, this fix triggers another problem when CONFIG_KASAN is
set, again only in this one driver:

drivers/net/wireless/wl3501_cs.c: In function 'wl3501_rx_interrupt':
drivers/net/wireless/wl3501_cs.c:1103:1: error: the frame size of 2232 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]

I'm not an x86 person and gcc inline assembly mystifies me all the time,
so please review this carefully and suggest a better way if this is not
how it should be done.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 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 7afb0e2f07f4..d107251eabd9 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), "m" (addr));\
 }									\
 									\
 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), "=m" (addr) : "d"(port));\
 }
 
 BUILDIO(b, b, char)
-- 
2.9.0

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

* [RFC 2/2] wl3501_cs: reduce stack size for KASAN
  2017-07-10 14:44 [RFC 1/2] x86: mark target address as output in 'insb' asm Arnd Bergmann
@ 2017-07-10 14:44 ` Arnd Bergmann
  2017-07-25 12:52   ` Kalle Valo
  0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2017-07-10 14:44 UTC (permalink / raw)
  To: x86
  Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Kalle Valo,
	linux-kernel, linux-wireless, Arnd Bergmann

Inlining functions with local variables can lead to excessive stack usage
with KASAN after a previous patch that modifies the outsb/insb helpers
on x86.

drivers/net/wireless/wl3501_cs.c: In function 'wl3501_rx_interrupt':
drivers/net/wireless/wl3501_cs.c:1103:1: error: the frame size of 2232 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]

Marking the two callers of insb/outb 'noinline' prevents the compiler
from adding up the stack usage for each of the local variables passed
into those, reducing the maximum stack frame size to 800 bytes with
KASAN again.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/wireless/wl3501_cs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/wl3501_cs.c b/drivers/net/wireless/wl3501_cs.c
index acec0d9ec422..2cce22571b4c 100644
--- a/drivers/net/wireless/wl3501_cs.c
+++ b/drivers/net/wireless/wl3501_cs.c
@@ -242,8 +242,8 @@ static int wl3501_get_flash_mac_addr(struct wl3501_card *this)
  *
  * Move 'size' bytes from PC to card. (Shouldn't be interrupted)
  */
-static void wl3501_set_to_wla(struct wl3501_card *this, u16 dest, void *src,
-			      int size)
+static noinline void wl3501_set_to_wla(struct wl3501_card *this,
+				       u16 dest, void *src, int size)
 {
 	/* switch to SRAM Page 0 */
 	wl3501_switch_page(this, (dest & 0x8000) ? WL3501_BSS_SPAGE1 :
@@ -264,8 +264,8 @@ static void wl3501_set_to_wla(struct wl3501_card *this, u16 dest, void *src,
  *
  * Move 'size' bytes from card to PC. (Shouldn't be interrupted)
  */
-static void wl3501_get_from_wla(struct wl3501_card *this, u16 src, void *dest,
-				int size)
+static noinline void wl3501_get_from_wla(struct wl3501_card *this,
+					 u16 src, void *dest, int size)
 {
 	/* switch to SRAM Page 0 */
 	wl3501_switch_page(this, (src & 0x8000) ? WL3501_BSS_SPAGE1 :
-- 
2.9.0

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

* Re: [RFC 2/2] wl3501_cs: reduce stack size for KASAN
  2017-07-10 14:44 ` [RFC 2/2] wl3501_cs: reduce stack size for KASAN Arnd Bergmann
@ 2017-07-25 12:52   ` Kalle Valo
  2017-07-25 14:50     ` Arnd Bergmann
  0 siblings, 1 reply; 4+ messages in thread
From: Kalle Valo @ 2017-07-25 12:52 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: x86, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, linux-kernel,
	linux-wireless

Arnd Bergmann <arnd@arndb.de> writes:

> Inlining functions with local variables can lead to excessive stack usage
> with KASAN after a previous patch that modifies the outsb/insb helpers
> on x86.
>
> drivers/net/wireless/wl3501_cs.c: In function 'wl3501_rx_interrupt':
> drivers/net/wireless/wl3501_cs.c:1103:1: error: the frame size of 2232 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>
> Marking the two callers of insb/outb 'noinline' prevents the compiler
> from adding up the stack usage for each of the local variables passed
> into those, reducing the maximum stack frame size to 800 bytes with
> KASAN again.
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Arnd, based on the discussion I'm dropping. Please let me know if I
should take this still.

-- 
Kalle Valo

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

* Re: [RFC 2/2] wl3501_cs: reduce stack size for KASAN
  2017-07-25 12:52   ` Kalle Valo
@ 2017-07-25 14:50     ` Arnd Bergmann
  0 siblings, 0 replies; 4+ messages in thread
From: Arnd Bergmann @ 2017-07-25 14:50 UTC (permalink / raw)
  To: Kalle Valo
  Cc: the arch/x86 maintainers, Thomas Gleixner, Ingo Molnar,
	H . Peter Anvin, Linux Kernel Mailing List, linux-wireless

On Tue, Jul 25, 2017 at 2:52 PM, Kalle Valo <kvalo@codeaurora.org> wrote:
> Arnd Bergmann <arnd@arndb.de> writes:
>
>> Inlining functions with local variables can lead to excessive stack usage
>> with KASAN after a previous patch that modifies the outsb/insb helpers
>> on x86.
>>
>> drivers/net/wireless/wl3501_cs.c: In function 'wl3501_rx_interrupt':
>> drivers/net/wireless/wl3501_cs.c:1103:1: error: the frame size of 2232 bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
>>
>> Marking the two callers of insb/outb 'noinline' prevents the compiler
>> from adding up the stack usage for each of the local variables passed
>> into those, reducing the maximum stack frame size to 800 bytes with
>> KASAN again.
>>
>> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
>
> Arnd, based on the discussion I'm dropping. Please let me know if I
> should take this still.

Thanks, that's good. The problem has become unreproducible and
I assume it's gone for good with the new x86 fix. In the unlikely
case some form of the problem comes back in another randconfig,
I'll post a new patch.

       Arnd

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

end of thread, other threads:[~2017-07-25 14:50 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-10 14:44 [RFC 1/2] x86: mark target address as output in 'insb' asm Arnd Bergmann
2017-07-10 14:44 ` [RFC 2/2] wl3501_cs: reduce stack size for KASAN Arnd Bergmann
2017-07-25 12:52   ` Kalle Valo
2017-07-25 14:50     ` Arnd Bergmann

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).