linux-kernel - Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

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]

Message-ID: <AANLkTi=jwrt2VE5tnL9NR2WhGDnQtv+k6rapnHfsy_aL@mail.gmail.com>
Date:	Mon, 15 Nov 2010 08:04:49 -0800
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Jakub Jelinek <jakub@...hat.com>
Cc:	Andi Kleen <andi@...stfloor.org>, James Cloos <cloos@...loos.com>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Andreas Schwab <schwab@...hat.com>,
	Michael Matz <matz@...e.de>,
	Dave Korn <dave.korn.cygwin@...il.com>,
	Richard Guenther <richard.guenther@...il.com>, gcc@....gnu.org,
	Jim Bos <jim876@...all.nl>
Subject: Re: gcc 4.5.1 / as 2.20.51.0.11 miscompiling drivers/char/i8k.c ?

On Mon, Nov 15, 2010 at 3:16 AM, Jakub Jelinek <jakub@...hat.com> wrote:
>
> I don't see any problems on the assembly level.  i8k_smm is
> not inlined in this case and checks all 3 conditions.

If it really is related to gcc not understanding that "*regs" has
changed due to the memory being an automatic variable, and passing in
"regs" itself as a pointer to that automatic variable together with
the "memory" clobber not being sufficient, than I think it's the lack
of inlining that will automatically hide the bug.

(Side note: and I think this does show how much of a gcc bug it is not
to consider "memory" together with passing in a pointer to an asm to
always be a clobber).

Because if it isn't inlined, then "regs" will be seen a a real pointer
to some external memory (the caller) rather than being optimized to
just be the auto structure on the stack. Because *mem is auto only
within the context of the caller.

Which actually points to a possible simpler:
 - remove the "+m" since it adds too much register pressure
 - mark the i8k_smm() as "noinline" instead.

Quite frankly, I'd hate to add even more crud to that inline asm (to
save/restore the registers manually). It's already not the prettiest
thing around.

So does the attached patch work for everybody?

                             Linus

View attachment "patch.diff" of type "text/x-patch" (1019 bytes)