linux-kernel - Re: [PATCH] x86/acrn: Improve ACRN hypercalls

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: <CAFULd4a3oY6HPtccj0tAH0H9h=mhZkbUUimqq4tWBZU2E6oS2g@mail.gmail.com>
Date:   Thu, 4 Aug 2022 21:05:57 +0200
From:   Uros Bizjak <ubizjak@...il.com>
To:     Sean Christopherson <seanjc@...gle.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        "H. Peter Anvin" <hpa@...or.com>
Subject: Re: [PATCH] x86/acrn: Improve ACRN hypercalls

On Thu, Aug 4, 2022 at 8:52 PM Sean Christopherson <seanjc@...gle.com> wrote:
>
> On Thu, Aug 04, 2022, Uros Bizjak wrote:
> > As explained in section 6.47.5.2, "Specifying Registers for Local Variables"
> > of the GCC info documentation, the correct way to specify register for
> > input operands when calling Extended 'asm' is to define a local register
> > variable and associate it with a specified register:
> >
> >       register unsigned long r8 asm ("r8") = hcall_id;
>
> Using "register" for input operands is subtly dangerous.  The compiler (at least
> some versions of GCC) isn't smart enough to realize that it must not clobber the
> register between setting the register and passing it into asm().  Or maybe that's
> the designed/intended behavior, but if so, IMO it's a terrible design.
>
> And since kernel style is to put declarations at the beginning of functions, it's
> quite easy for a developer/debugger to mess this up (speaking from multiple
> experiences).

All you said there is true, and these cases are even covered in the
GCC documentation (please see section 6.47.5.2, "Specifying Registers
for Local Variables"). But in this particular case, we have a static
inline function, which encapsulates low-level access, so we *can* add
the definition just above the asm.

Uros.