linux-kernel - Re: [PATCH] KVM: arm64: Fix NULL pointer access issue

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] [day] [month] [year] [list]

Message-ID: <8eb373a0-0d73-4555-9e64-a62245727d56@quicinc.com>
Date: Tue, 2 Sep 2025 11:30:26 +0800
From: "Yingchao Deng (Consultant)" <quic_yingdeng@...cinc.com>
To: James Clark <james.clark@...aro.org>, Marc Zyngier <maz@...nel.org>,
        Yingchao Deng <yingchao.deng@....qualcomm.com>
CC: <linux-arm-kernel@...ts.infradead.org>, <kvmarm@...ts.linux.dev>,
        <linux-kernel@...r.kernel.org>, <jinlong.mao@....qualcomm.com>,
        <tingwei.zhang@....qualcomm.com>,
        Oliver Upton <oliver.upton@...ux.dev>,
        "Joey Gouly" <joey.gouly@....com>,
        Suzuki K Poulose <suzuki.poulose@....com>,
        Zenghui Yu <yuzenghui@...wei.com>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will@...nel.org>
Subject: Re: [PATCH] KVM: arm64: Fix NULL pointer access issue

Thanks James & Marc
Will upload v2 and move the warning to the end for better readability.

Thanks,
Yingchao
On 9/1/2025 10:16 PM, James Clark wrote:
>
>
> On 01/09/2025 2:30 pm, Marc Zyngier wrote:
>> On Mon, 01 Sep 2025 13:31:23 +0100,
>> James Clark <james.clark@...aro.org> wrote:
>>>
>>>
>>>
>>> On 01/09/2025 1:24 pm, Marc Zyngier wrote:
>>>> On Mon, 01 Sep 2025 11:36:11 +0100,
>>>> James Clark <james.clark@...aro.org> wrote:
>>>>>
>>>>>
>>>>>
>>>>> On 01/09/2025 11:01 am, Yingchao Deng wrote:
>>>>>> When linux is booted in EL1, macro "host_data_ptr()" is a wrapper 
>>>>>> that
>>>>>> resolves to "&per_cpu_ptr_nvhe_sym(kvm_host_data, cpu)",
>>>>>> is_hyp_mode_available() return false during kvm_arm_init, the 
>>>>>> per-CPU base
>>>>>> pointer __kvm_nvhe_kvm_arm_hyp_percpu_base[cpu] remains 
>>>>>> uninitialized.
>>>>>> Consequently, any access via per_cpu_ptr_nvhe_sym(kvm_host_data, 
>>>>>> cpu)
>>>>>> will result in a NULL pointer.
>>>>>>
>>>>>> Add is_kvm_arm_initialised() condition check to ensure that 
>>>>>> kvm_arm_init
>>>>>> completes all necessary initialization steps, including 
>>>>>> init_hyp_mode.
>>>>>>
>>>>>> Fixes: 054b88391bbe2 ("KVM: arm64: Support trace filtering for 
>>>>>> guests")
>>>>>> Signed-off-by: Yingchao Deng <yingchao.deng@....qualcomm.com>
>>>>>> ---
>>>>>> Add a check to prevent accessing uninitialized per-CPU data.
>>>>>> ---
>>>>>>     arch/arm64/kvm/debug.c | 7 ++++---
>>>>>>     1 file changed, 4 insertions(+), 3 deletions(-)
>>>>>>
>>>>>> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
>>>>>> index 
>>>>>> 381382c19fe4741980c79b08bbdab6a1bcd825ad..add58056297293b4eb337028773b1b018ecc9d35 
>>>>>> 100644
>>>>>> --- a/arch/arm64/kvm/debug.c
>>>>>> +++ b/arch/arm64/kvm/debug.c
>>>>>> @@ -233,7 +233,7 @@ void kvm_debug_handle_oslar(struct kvm_vcpu 
>>>>>> *vcpu, u64 val)
>>>>>>     void kvm_enable_trbe(void)
>>>>>>     {
>>>>>>         if (has_vhe() || is_protected_kvm_enabled() ||
>>>>>> -        WARN_ON_ONCE(preemptible()))
>>>>>> +        WARN_ON_ONCE(preemptible()) || !is_kvm_arm_initialised())
>>>>>
>>>>> Hi Yingchao,
>>>>>
>>>>> There shouldn't be a warning for this, at least for the case where
>>>>> it's not initialized and never will be. If you're never going to 
>>>>> run a
>>>>> guest these functions can all skip, the same way for !has_vhe() etc.
>>>>
>>>> It's not a warning. It's a bona-fide crash:
>>>>
>>>> void kvm_enable_trbe(void)
>>>> {
>>>>     if (has_vhe() || is_protected_kvm_enabled() ||
>>>>         WARN_ON_ONCE(preemptible()))
>>>>         return;
>>>>
>>>>     host_data_set_flag(TRBE_ENABLED); <--- Explodes here
>>>> }
>>>>
>>>> So the write of the flag has to be skipped if KVM is available, even
>>>> if KVM is compiled in.
>>>>
>>>>     M.
>>>>
>>>
>>> Yeah. And just in case there is any confusion, I didn't mean that we
>>> should not have the check entirely, just that it shouldn't be in the
>>> WARN_ON_ONCE(). We should put it in the part that makes the functions
>>> silently skip:
>>>
>>>    if (has_vhe() || is_protected_kvm_enabled() ||
>>>        !is_kvm_arm_initialised() ||
>>>        WARN_ON_ONCE(preemptible()))
>>>            return;
>>
>> Which is exactly what the OP wrote, except for swapping the last two
>> terms.
>>
>>     M.
>>
>
> Hah! So it is. Being on the same line as the warning really threw me 
> despite looking at it 10 times.
>
> Not sure if it's just me but I think having the warning at the end or 
> on its own line is more readable.
>
> Either way:
>
> Reviewed-by: James Clark <james.clark@...aro.org>
>
>
>
>