linux-kernel - Re: [tip:x86/urgent] x86/io: Mark target address as output in 'insb()' asm

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, 12 Jul 2017 15:43:59 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Arnd Bergmann <arnd@...db.de>
Cc:     Andrew Lutomirski <luto@...nel.org>,
        Denys Vlasenko <dvlasenk@...hat.com>,
        Borislav Petkov <bp@...en8.de>,
        Josh Poimboeuf <jpoimboe@...hat.com>,
        Kalle Valo <kvalo@...eaurora.org>, Peter Anvin <hpa@...or.com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Ingo Molnar <mingo@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Brian Gerst <brgerst@...il.com>,
        "linux-tip-commits@...r.kernel.org" 
        <linux-tip-commits@...r.kernel.org>
Subject: Re: [tip:x86/urgent] x86/io: Mark target address as output in
 'insb()' asm

On Wed, Jul 12, 2017 at 2:47 PM, Arnd Bergmann <arnd@...db.de> wrote:
>>
>> The real fix is probably to just mark them as "clobbers memory" (ie
>> just add "memory" to the clobber list).
>>
>> If you want to be fancy, you can try to do what <asm/uaccess.h> does,
>> which is a disgusting hack, but has traditionally worked;
>>
>>   struct __large_struct { unsigned long buf[100]; };
>>   #define __m(x) (*(struct __large_struct __user *)(x))
>>
>> and then use your approach with "m" and "=m".
>
> Ok, I'll try both tomorrow and see where I end up.

Note that just adding the "memory" thing to the clobbers likely causes
slightly worse code generation (it basically says that the asm can
clobber anything at all, so cause re-loads etc that are entirely
unrelated to the asm).

But for something like "rep in/out", that really doesn't much matter.
PIO is very slow due to being fully serialized, and "rep ins/outs" is
just about the slowest thing you can do on a machine. So nobody really
does it, the main traditional user was the legacy PIO data transfer
for ST-506 disks. And, as you noticed a few *really* old network card
drivers.

So I suspect the big hammer memory clobber is the right thing to do.

> Would you expect that the missing clobber causes actual
> runtime bugs and the fix needs to be backported to stable
> kernels?

Probably not. "asm volatile" is already pretty serialized. It's not
entirely obvious what gcc will move around it, but almost certainly no
operations that matter for the network buffer.

And honestly, the wt3501 is basically an ISA card in PCMCIA format. I
don't think it's even cardbus (Cardbus aka Yenta is basically "hotplug
PCI"). So I don't think anybody even has that hardware.

It was a good card for its time. But its time was basically two decades ago.

                  Linus