Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1691656 > unrolled thread
| Started by | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| First post | 2017-07-19 15:00 +0200 |
| Last post | 2017-07-20 12:40 +0200 |
| Articles | 4 — 3 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl Arnd Bergmann <arnd@arndb.de> - 2017-07-19 15:00 +0200
Re: [PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl Arnd Bergmann <arnd@arndb.de> - 2017-07-19 21:30 +0200
Re: [PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl Linus Torvalds <torvalds@linux-foundation.org> - 2017-07-19 21:50 +0200
[tip:x86/urgent] x86/io: Add "memory" clobber to insb/insw/insl/outsb/outsw/outsl tip-bot for Arnd Bergmann <tipbot@zytor.com> - 2017-07-20 12:40 +0200
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-07-19 15:00 +0200 |
| Subject | [PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl |
| Message-ID | <u4WK6-fH-25@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Arnd Bergmann <arnd@arndb.de> |
|---|---|
| Date | 2017-07-19 21:30 +0200 |
| Message-ID | <u52Pw-4BQ-13@gated-at.bofh.it> |
| In reply to | #1691656 |
[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
>
[toc] | [prev] | [next] | [standalone]
| From | Linus Torvalds <torvalds@linux-foundation.org> |
|---|---|
| Date | 2017-07-19 21:50 +0200 |
| Message-ID | <u538S-4JC-19@gated-at.bofh.it> |
| In reply to | #1691656 |
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
[toc] | [prev] | [next] | [standalone]
| From | tip-bot for Arnd Bergmann <tipbot@zytor.com> |
|---|---|
| Date | 2017-07-20 12:40 +0200 |
| Subject | [tip:x86/urgent] x86/io: Add "memory" clobber to insb/insw/insl/outsb/outsw/outsl |
| Message-ID | <u5h2a-63o-23@gated-at.bofh.it> |
| In reply to | #1691656 |
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)
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web