linux-kernel - Re: [PATCH] x86/AMD: Fix ASM constraints in amd_clear

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: <CAHk-=wiic4XViHp0=0pjLnQALRpfda+naY0ysZdK6Fia86hh2w@mail.gmail.com>
Date:   Wed, 9 Aug 2023 14:33:48 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Andrew Cooper <andrew.cooper3@...rix.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Borislav Petkov <bp@...en8.de>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org
Subject: Re: [PATCH] x86/AMD: Fix ASM constraints in amd_clear_divider()

On Wed, 9 Aug 2023 at 13:24, Andrew Cooper <andrew.cooper3@...rix.com> wrote:
>
> DIV writes its results into %eax and %edx, meaning that they need to be output
> constraints too.  It happens to be benign in this case as the registers don't
> change value, but the compiler should still know.
>
> Fixes: 77245f1c3c64 ("x86/CPU/AMD: Do not leak quotient data after a division by 0")

As mentioned earlier (html, not on list), I think it was intentional
and this "fix" doesn't really fix anything.

A comment might be good, of course, if this really bothers somebody.

That said, if the code wanted to be *really* clever, it could have done

        asm volatile(ALTERNATIVE("", "div %0", X86_BUG_DIV0)
                     :: "a" (1), "d" (0));

instead. One less register used, and the same "no change to register
state" approach.

Of course, who knows what early-out algorithm the divider uses, and
maybe it's cheaper to do 0/1 than it is to do 1/1. Not that I think we
should care. The main reason to be cute here would be just to be cute.

That said, if you were to use this pattern in *real* code (as opposed
to in that function that will never be called in reality because
nobody divides by zero in real code), the register clobbers might be
useful just to make sure the compiler doesn't re-use a zero register
content that is then behind the latency of the dummy divide. But
again, this particular code really doesn't _matter_ in that sense.

               Linus