On Thu, Feb 08, 2024, Paul Durrant wrote:
On 07/02/2024 04:10, Sean Christopherson wrote:
On Mon, Jan 15, 2024, Paul Durrant wrote:
@@ -638,20 +637,32 @@ int kvm_xen_hvm_set_attr(struct kvm *kvm, struct kvm_xen_hvm_attr *data) } break;
- case KVM_XEN_ATTR_TYPE_SHARED_INFO: {
- case KVM_XEN_ATTR_TYPE_SHARED_INFO:
- case KVM_XEN_ATTR_TYPE_SHARED_INFO_HVA: { int idx; mutex_lock(&kvm->arch.xen.xen_lock); idx = srcu_read_lock(&kvm->srcu);
if (data->u.shared_info.gfn == KVM_XEN_INVALID_GFN) {kvm_gpc_deactivate(&kvm->arch.xen.shinfo_cache);r = 0;
if (data->type == KVM_XEN_ATTR_TYPE_SHARED_INFO) {if (data->u.shared_info.gfn == KVM_XEN_INVALID_GFN) {kvm_gpc_deactivate(&kvm->arch.xen.shinfo_cache);r = 0;} else {r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,gfn_to_gpa(data->u.shared_info.gfn),PAGE_SIZE);}
r = kvm_gpc_activate(&kvm->arch.xen.shinfo_cache,gfn_to_gpa(data->u.shared_info.gfn),PAGE_SIZE);
if (data->u.shared_info.hva == 0) {I know I said I don't care about the KVM Xen ABI, but I still think using '0' as "invalid" is ridiculous.
With the benefit of some sleep, I'm wondering why 0 is a 'ridiculous' invalid value for a *virtual* address? Surely it's essentially a numerical cast of the canonically invalid NULL pointer?
It's legal to mmap() virtual address '0', albeit not by default:
config DEFAULT_MMAP_MIN_ADDR int "Low address space to protect from user allocation" depends on MMU default 4096 help This is the portion of low virtual memory which should be protected from userspace allocation. Keeping a user from writing to low pages can help reduce the impact of kernel NULL pointer bugs.
For most ppc64 and x86 users with lots of address space a value of 65536 is reasonable and should cause no problems. On arm and other archs it should not be higher than 32768. Programs which use vm86 functionality or have some need to map this low address space will need CAP_SYS_RAWIO or disable this protection by setting the value to 0.
This value can be changed after boot using the /proc/sys/vm/mmap_min_addr tunable.
Obviously it's equally ridiculous that userspace would ever mmap() '0' and pass that as the shared_info, but given that this is x86-only, there are architecturally illegal addresses that can be used, at least until Intel adds LA64 ;-)