linux-kernel - Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall definitions

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 for Android: free password hash cracker in your pocket

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

Message-ID: <b82e190e-6887-b95a-a99a-176f22c57b7b@shipmail.org>
Date:   Tue, 27 Aug 2019 21:19:03 +0200
From:   Thomas Hellström (VMware) 
        <thomas_os@...pmail.org>
To:     Borislav Petkov <bp@...en8.de>
Cc:     linux-kernel@...r.kernel.org, pv-drivers@...are.com,
        linux-graphics-maintainer@...are.com,
        Thomas Hellstrom <thellstrom@...are.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        dri-devel@...ts.freedesktop.org, Doug Covelli <dcovelli@...are.com>
Subject: Re: [PATCH v2 2/4] x86/vmware: Add a header file for hypercall
 definitions

On 8/27/19 5:44 PM, Borislav Petkov wrote:
> On Fri, Aug 23, 2019 at 10:13:14AM +0200, Thomas Hellström (VMware) wrote:
>> +/*
>> + * The high bandwidth out call. The low word of edx is presumed to have the
>> + * HB and OUT bits set.
>> + */
>> +#define VMWARE_HYPERCALL_HB_OUT						\
>> +	ALTERNATIVE_2("movw $" VMWARE_HYPERVISOR_PORT_HB ", %%dx; rep outsb", \
> Hmm, that looks fishy:
>
> This call in vmw_port_hb_out(), for example, gets converted to the asm
> below (I've left in the asm touching only rDX).
>
> # drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:160:              VMW_PORT_HB_OUT(
> #NO_APP
>          movzwl  0(%rbp), %edx   # channel_20(D)->channel_id, channel_20(D)->channel_id
>
> 	...
>
>          sall    $16, %edx       #, tmp172
>          orl     $3, %edx        #, tmp173
>
> this is adding channel_id and flags:
>
>                          VMWARE_HYPERVISOR_HB | (channel->channel_id << 16) |
>                          VMWARE_HYPERVISOR_OUT,
>
> the $3 being (VMWARE_HYPERVISOR_HB | VMWARE_HYPERVISOR_OUT).
>
>          movslq  %edx, %rdx      # tmp173, tmp174
>
> Here it is sign-extending it.
>
> #APP
> # 160 "drivers/gpu/drm/vmwgfx/vmwgfx_msg.c" 1
>          push %rbp;mov %r8, %rbp;# ALT: oldinstr2        # bp
> 661:
>          movw $0x5659, %dx; rep outsb
>
> And now here you're overwriting the low word of %edx. And now it
> contains:
>
> 0x[channel_id]5659
>
> and the low word doesn't contain the 3, i.e., (VMWARE_HYPERVISOR_HB |
> VMWARE_HYPERVISOR_OUT) anymore. And that's before you do the hypercall
> so I'm guessing that cannot be right.
>
> Or?
>
It should be correct. The flags VMWARE_HYPERVISOR_HB and 
VMWARE_HYPERVISOR_OUT are only valid for the vmcall / vmmcall versions.

For the legacy version, the direction is toggled by the instruction (in 
vs out) and LB vs HB is toggled by the port number (0x5658 vs 0x5659)

So in essence the low word definition of %edx is different in the two 
versions. I've chosen to use the new vmcall/vmmcall definition in the 
driver code.

/Thomas