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]
Date: Thu, 22 Feb 2024 17:45:56 +0800
From: WANG Xuerui <kernel@...0n.name>
To: maobibo <maobibo@...ngson.cn>, Paolo Bonzini <pbonzini@...hat.com>,
 Huacai Chen <chenhuacai@...nel.org>
Cc: Tianrui Zhao <zhaotianrui@...ngson.cn>, kvm@...r.kernel.org,
 loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org,
 WANG Xuerui <git@...0n.name>
Subject: Re: [PATCH for-6.8 v3 1/3] LoongArch: KVM: Fix input validation of
 _kvm_get_cpucfg and kvm_check_cpucfg

Hi,

On 2/17/24 11:03, maobibo wrote:
> Hi Xuerui,
> 
> Good catch, and thank for your patch.
> 
> On 2024/2/16 下午4:58, WANG Xuerui wrote:
>> [snip]
>> @@ -324,31 +319,33 @@ static int _kvm_get_cpucfg(int id, u64 *v)
>>           if (cpu_has_lasx)
>>               *v |= CPUCFG2_LASX;
>> -        break;
>> +        return 0;
>> +    case 0 ... 1:
>> +    case 3 ... KVM_MAX_CPUCFG_REGS - 1:
>> +        /* no restrictions on other CPUCFG IDs' values */
>> +        *v = U64_MAX;
>> +        return 0;
> how about something like this?
>      default:
>          /* no restrictions on other CPUCFG IDs' values */
>          *v = U64_MAX;
>          return 0;

I don't think this version correctly expresses the intent. Note that the 
CPUCFG ID range check is squashed into the switch as well, so one switch 
conveniently expresses the three intended cases at once:

* the special treatment of CPUCFG2,
* all-allow rules for other in-range CPUCFG IDs, and
* rejection for out-of-range IDs.

Yet the suggestion here is conflating the latter two cases, with the 
effect of allowing every ID that's not 2 to take any value (as expressed 
by the U64_MAX mask), and *removing the range check* (because no return 
path returns -EINVAL with this change).

So I'd like to stick to the current version, but thanks anyway for your 
kind review and suggestion.

-- 
WANG "xen0n" Xuerui

Linux/LoongArch mailing list: https://lore.kernel.org/loongarch/


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ