Re: [PATCH 1/4] target/loongarch/kvm: fix uninitialized val and unchecked GET in cpucfg2 check

[Tao Cui <cui.tao@linux.dev>]

在 2026/6/25 10:48, Bibo Mao 写道:
> 
> 
> On 2026/6/25 上午9:58, Tao Cui wrote:
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> kvm_check_cpucfg2() discards the return value of KVM_GET_DEVICE_ATTR and
>> then uses the local val (the host cpucfg2 mask) without checking whether
>> the read succeeded. val is also declared without an initializer.
>>
>> If GET fails, env->cpucfg[2] &= val uses an uninitialized value and can
>> silently clear feature bits (FP / LLFTP / LSX / LASX), since bitwise-AND
>> can only turn bits off.
>>
>> Check the GET return value, report the failure with error_report(), and
>> initialize val to 0.
>>
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>> ---
>>   target/loongarch/kvm/kvm.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/loongarch/kvm/kvm.c b/target/loongarch/kvm/kvm.c
>> index d6539c12ac..b7176ce53a 100644
>> --- a/target/loongarch/kvm/kvm.c
>> +++ b/target/loongarch/kvm/kvm.c
>> @@ -725,7 +725,7 @@ static int kvm_loongarch_get_cpucfg(CPUState *cs)
>>   static int kvm_check_cpucfg2(CPUState *cs)
>>   {
>>       int ret;
>> -    uint64_t val;
>> +    uint64_t val = 0;
>>       struct kvm_device_attr attr = {
>>           .group = KVM_LOONGARCH_VCPU_CPUCFG,
>>           .attr = 2,
>> @@ -736,7 +736,11 @@ static int kvm_check_cpucfg2(CPUState *cs)
>>       ret = kvm_vcpu_ioctl(cs, KVM_HAS_DEVICE_ATTR, &attr);
>>         if (!ret) {
>> -        kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
>> +        ret = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
>> +        if (ret) {
>> +            error_report("CPUCFG2: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
>> +            return ret;
> if it is successful with KVM_HAS_DEVICE_ATTR, however error with KVM_GET_DEVICE_ATTR. There should be mempy_from/to_user problem, maybe VM can continue to run without the following logic and operation. How about something like this?
> -        kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
> -        env->cpucfg[2] &= val;
> +        ret = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
> +        if (!ret)
> +            env->cpucfg[2] &= val;
> 
Right, the &= mask is best-effort negotiation, so failing the whole register
sync over it is heavier than needed. I'll rework 1/4 so a GET failure only
skips the mask, using a local variable so the GET error no longer propagates:

    if (!ret) {
        int r = kvm_vcpu_ioctl(cs, KVM_GET_DEVICE_ATTR, &attr);
        if (r) {
            warn_report("CPUCFG2: KVM_GET_DEVICE_ATTR: %s", strerror(errno));
        } else {
            env->cpucfg[2] &= val;
        }
        ...
    }
    return ret;

The guest then keeps running with the cpucfg2 it already has. val is still
initialized to 0.

Thanks,
Tao
> 
> sentence
> feature and operation env->cpucfg[2] &= val;
>> +        }
>>           env->cpucfg[2] &= val;
>>             if (FIELD_EX32(env->cpucfg[2], CPUCFG2, FP)) {
>>
>