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);
> > > + }
> > > } else {
> > > - 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 ;-)
