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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 19 Jul 2017 21:25:13 +0200
From:   Arnd Bergmann <arnd@...db.de>
To:     "the arch/x86 maintainers" <x86@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Arnd Bergmann <arnd@...db.de>, Borislav Petkov <bp@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        Linus Torvalds <torvalds@...ux-foundation.org>
Subject: Re: [PATCH 4/8] x86: io: add "memory" clobber to insb/insw/insl/outsb/outsw/outsl

[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@...db.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@...ux-foundation.org>
> Signed-off-by: Arnd Bergmann <arnd@...db.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
>