linux-kernel - Re: [PATCH v3 1/7] arm64: hyperv: Use SMC to detect hypervisor presence

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: <2ad7731a-b56b-415d-acb1-907fa7224671@linux.microsoft.com>
Date: Mon, 5 Aug 2024 14:44:17 -0700
From: Roman Kisel <romank@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.com>, "arnd@...db.de" <arnd@...db.de>,
 "bhelgaas@...gle.com" <bhelgaas@...gle.com>, "bp@...en8.de" <bp@...en8.de>,
 "catalin.marinas@....com" <catalin.marinas@....com>,
 "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
 "decui@...rosoft.com" <decui@...rosoft.com>,
 "haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
 "hpa@...or.com" <hpa@...or.com>, "kw@...ux.com" <kw@...ux.com>,
 "kys@...rosoft.com" <kys@...rosoft.com>, "lenb@...nel.org"
 <lenb@...nel.org>, "lpieralisi@...nel.org" <lpieralisi@...nel.org>,
 "mingo@...hat.com" <mingo@...hat.com>, "rafael@...nel.org"
 <rafael@...nel.org>, "robh@...nel.org" <robh@...nel.org>,
 "tglx@...utronix.de" <tglx@...utronix.de>,
 "wei.liu@...nel.org" <wei.liu@...nel.org>, "will@...nel.org"
 <will@...nel.org>, "linux-acpi@...r.kernel.org"
 <linux-acpi@...r.kernel.org>,
 "linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
 "linux-arm-kernel@...ts.infradead.org"
 <linux-arm-kernel@...ts.infradead.org>,
 "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
 "linux-pci@...r.kernel.org" <linux-pci@...r.kernel.org>,
 "x86@...nel.org" <x86@...nel.org>
Cc: "apais@...rosoft.com" <apais@...rosoft.com>,
 "benhill@...rosoft.com" <benhill@...rosoft.com>,
 "ssengar@...rosoft.com" <ssengar@...rosoft.com>,
 "sunilmut@...rosoft.com" <sunilmut@...rosoft.com>,
 "vdso@...bites.dev" <vdso@...bites.dev>
Subject: Re: [PATCH v3 1/7] arm64: hyperv: Use SMC to detect hypervisor
 presence



On 8/5/2024 1:30 PM, Michael Kelley wrote:
> From: Roman Kisel <romank@...ux.microsoft.com> Sent: Monday, August 5, 2024 9:51 AM
> 
> [snip]
> 
>>>> diff --git a/arch/arm64/include/asm/mshyperv.h
>>>> b/arch/arm64/include/asm/mshyperv.h
>>>> index a975e1a689dd..a7a3586f7cb1 100644
>>>> --- a/arch/arm64/include/asm/mshyperv.h
>>>> +++ b/arch/arm64/include/asm/mshyperv.h
>>>> @@ -51,4 +51,9 @@ static inline u64 hv_get_msr(unsigned int reg)
>>>>
>>>>    #include <asm-generic/mshyperv.h>
>>>>
>>>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_0	0x7948734d
>>>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_1	0x56726570
>>>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_2	0
>>>> +#define ARM_SMCCC_VENDOR_HYP_UID_HYPERV_REG_3	0
>>>> +
>>>
>>> Section 6.2 of the SMCCC specification says that the "Call UID Query"
>>> returns a UUID. The above #defines look like an ASCII string is being
>>> returned. Arguably the ASCII string can be treated as a set of 128 bits
>>> just like a UUID, but it doesn't meet the spirit of the spec. Can Hyper-V
>>> be changed to return a real UUID? While the distinction probably
>>> won't make a material difference here, we've had problems in the past
>>> with Hyper-V doing slightly weird things that later caused unexpected
>>> trouble. Please just get it right. :-)
>>>
>> The above values don't violate anything in the spec, and I think it
>> would be hard to give an example of what can be broken in the world by
>> using the above values.
> 
> Agreed.  However, note that UUIDs *do* have some internal structure.
> See https://www.uuidtools.com/decode, for example. The UUID above
> is:
> 
> 4d734879-7065-7256-0000-000000000000
> 
> It has a version digit of "7", which is "unknown".  I'm no expert on UUIDs,
> but apparently the "7" isn't necessarily invalid, though perhaps a bit unexpected.
> 
Understood! I made an assumption that's just an array of random bytes.
Thank you very much for explaining that to me :)

>> I do understand what you're saying when you
>> mention the UIDs & the spirit of the spec. Put on the quantitative
>> footing, the Shannon entropy of these values is much lower than that of
>> an UID. A cursory search in the kernel tree does turn up other UIDs that
>> don't look too random.
>>
>> As that is implemented only in the non-release versions, hardly someone
>> has taken a dependency on the specific values in their production code.
>> I guess that can be changed without causing any disturbance to the
>> customers, will report of any concerns though.
> 
> My last thought is that when dealing with the open source world, and
> with published standards, it's usually best to do what's normal and
> expected, rather than trying to make the case that something unexpected
> is allowed by the spec. Doing the latter can come back to bite you in
> completely unexpected ways. :-)
> 
Agreed, thanks for the discussion! I've discussed changing the values,
and a pull request to the hypervisor has been put up for that. So far, 
going well.

> With that, I won't make any further comments on the topic. You do
> whatever you can do in working with Hyper-V. Either way, it won't be
> a blocker to my giving "Reviewed-by" on the next version of the
> patch.
> 
Will be a badge of honor to me! Again, appreciate your words of advise 
and caution, and helping in bringing these patches into the best shape 
possible!

> Michael

-- 
Thank you,
Roman