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

From: Arnd Bergmann
Date: Wed Jul 19 2017 - 15:25:20 EST


[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@xxxxxxxx> 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@xxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> ---
> 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
>