linux-kernel - Re: [PATCH 1/1] KVM: use KVM_HVA_ERR

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: <04398f4e-6098-4559-9604-b9810753801e@intel.com>
Date: Fri, 1 Mar 2024 10:53:22 +1300
From: "Huang, Kai" <kai.huang@...el.com>
To: Dongli Zhang <dongli.zhang@...cle.com>, <kvm@...r.kernel.org>
CC: <pbonzini@...hat.com>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 1/1] KVM: use KVM_HVA_ERR_BAD to check bad hva

On 1/03/2024 10:25 am, Dongli Zhang wrote:
> Replace PAGE_OFFSET with KVM_HVA_ERR_BAD, to facilitate the cscope when
> looking for where KVM_HVA_ERR_BAD is used.
> 
> Every time I use cscope to query the functions that are impacted by the
> return value (KVM_HVA_ERR_BAD) of __gfn_to_hva_many(), I may miss
> kvm_is_error_hva().

I am not sure "to facilitate cscope" could be a justification to do some 
code change in the kernel.

> 
> Signed-off-by: Dongli Zhang <dongli.zhang@...cle.com>
> ---
>   include/linux/kvm_host.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 7e7fd25b09b3..4dc0300e7766 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -143,7 +143,7 @@ static inline bool is_noslot_pfn(kvm_pfn_t pfn)
>   
>   static inline bool kvm_is_error_hva(unsigned long addr)
>   {
> -	return addr >= PAGE_OFFSET;
> +	return addr >= KVM_HVA_ERR_BAD;
>   }
>   
>   #endif

Also, IIUC the KVM_HVA_ERR_BAD _theoretically_ can be any random value 
that can make kvm_is_error_hva() return false, while kvm_is_error_hva() 
must catch all error HVAs.

E.g., if we ever change KVM_HVA_ERR_BAD to use any other value (although 
I don't see why this could ever happen), then using KVM_HVA_ERR_BAD in 
kvm_is_error_hva() would be broken.

In other words, it seems to me we should just use PAGE_OFFSET in 
kvm_is_error_hva().