linux-kernel - Re: [PATCH] arm64: KVM: VHE: reset PSTATE.UAO when switch to host

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: <d01b49a2-e204-73af-23e3-5fd7f7202859@arm.com>
Date:   Thu, 7 Sep 2017 16:23:30 +0100
From:   Marc Zyngier <marc.zyngier@....com>
To:     gengdongjiu <gengdongjiu@...wei.com>,
        James Morse <james.morse@....com>
Cc:     "christoffer.dall@...aro.org" <christoffer.dall@...aro.org>,
        "vladimir.murzin@....com" <vladimir.murzin@....com>,
        "rkrcmar@...hat.com" <rkrcmar@...hat.com>,
        "catalin.marinas@....com" <catalin.marinas@....com>,
        "shankerd@...eaurora.org" <shankerd@...eaurora.org>,
        "linux-arm-kernel@...ts.infradead.org" 
        <linux-arm-kernel@...ts.infradead.org>,
        "kvmarm@...ts.cs.columbia.edu" <kvmarm@...ts.cs.columbia.edu>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "Zhanghaibin (Euler)" <zhanghaibin7@...wei.com>,
        Huangshaoyu <huangshaoyu@...wei.com>
Subject: Re: [PATCH] arm64: KVM: VHE: reset PSTATE.UAO when switch to host

On 07/09/17 16:03, gengdongjiu wrote:
>> On 07/09/17 12:49, gengdongjiu wrote:
>>>
>>>
>>> On 2017/9/7 18:13, Marc Zyngier wrote:
>>>> On 07/09/17 11:05, gengdongjiu wrote:
>>>>> Hi James,
>>>>>
>>>>> On 2017/9/7 17:20, James Morse wrote:
>>>>>> Hi Dongjiu Geng,
>>>>>>
>>>>>> On 07/09/17 06:54, Dongjiu Geng wrote:
>>>>>>> In VHE mode, host kernel runs in the EL2 and can enable 'User
>>>>>>> Access Override' when fs==KERNEL_DS so that it can access kernel
>>>>>>> memory. However, PSTATE.UAO is set to 0 on an exception taken from
>>>>>>> EL1 to EL2. Thus when VHE is used and exception taken from a guest
>>>>>>> UAO will be disabled and host will use the incorrect PSTATE.UAO.
>>>>>>> So check and reset the PSTATE.UAO when switching to host.
>>>>>>
>>>>>> This would only be a problem if KVM were calling into world-switch
>>>>>> with fs==KERNEL_DS. I can't see where this happens.
>>>>>  Not only KVM, may also kernel sets the fs == KERNEL_DS before
>>>>> calling into world-switch
>>>>
>>>> How? Please describe the exact sequence of event that lead to this
>>>> situation with the current code base.
>>>
>>> Hi Marc,
>>>
>>>    Different tasks have different fs, such as USER_DS or KERNEL_DS. In
>>> the context switch, it will restore the task's fs. Thus, that depends
>>> on task itself, as shown below code. UAO is different with PAN, PAN will be always enabled if hardware CPU supports PAN feature, but
>> UAO is dynamical change.
>>
>> You haven't answered my question: There is exactly one point where we enter the world-switch. Show me that, at this point, PSTATE.UAO
>> *before* the call is different from PSTATE.UAO after the call. Give me the exact sequence of event that leads to this situation. Show me a
>> stack trace.
> 
> Hi Marc,
> 
> If using current mainline KVM code + Qemu and modify nothing, may not
> exist broken issue,  because the Qemu progress FS should be USER_DS.

Thanks. You've just been wasting everybody's time for two days. In case
you haven't noticed, mainline KVM code is the *ONLY* thing we care about.

> But if I make a modification for user space( Qemu or KVM tools) to
> change its FS property to KERNEL_DS or use third party application
> with KERNEL_DS FS to run the guest, it will have problem(the
> KERNEL_DS is cleared). If you think my case is reasonable and should
> support, I can show you the calling stack trace. If you think, my
> case is not reasonable and KVM should not support the application
> with KERNEL_DS fs to run guest. You can ignore this patch, thanks.

I really cannot think of a good reason why we'd want to do that. Playing
with set_fs() is almost universally wrong, and I'm certainly going to
oppose to any change in that area unless the code that calls set_fs()
has been made public and properly reviewed. Until then, UAO/PAN will
stay as they are unless you prove that our current code is wrong.

	M.
-- 
Jazz is not dead. It just smells funny...