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

[Arnd Bergmann <arnd@arndb.de>]

[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
>