On Wed, Dec 19, 2012 at 11:20:18AM +0100, Christian Borntraeger wrote: > On 19/12/12 10:44, Heiko Carstens wrote: > > On Fri, Dec 07, 2012 at 01:30:22PM +0100, Cornelia Huck wrote: > >> + rc = put_guest_u64(vcpu, __LC_MCCK_CODE, inti->mchk.mcic); > >> + if (rc == -EFAULT) > >> + exception = 1; > >> + > >> + rc = copy_to_guest(vcpu, __LC_MCK_OLD_PSW, > >> + &vcpu->arch.sie_block->gpsw, sizeof(psw_t)); > >> + if (rc == -EFAULT) > >> + exception = 1; > > > > Please don't add more explicit -EFAULT checks on guest access paths. Just > > make this like normal user space accesses. That is return code != 0 means > > an error occured: > > > > rc = put_guest_u64(vcpu, __LC_MCCK_CODE, inti->mchk.mcic); > > if (rc) > > exception = 1; > > > > In fact, with the current kvm gaccess code it's even broken, since on error > > the guest access functions may return also -ENOMEM instead of -EFAULT, which > > would be ignored by your code. > > I addressed that with a patch when trying to clean up the guest access > > functions. Maybe the patch below should be merged anyway. Christian? > > The whole guest memory access of KVM needs to be reworked to work properly > in those corner cases. I have this on my todo list as one of things for next > year with lots of open questions that I dont want to answer before xmas. > what about in kernel intercepts? (shall we then return EFAULT for the KVM_RUN > ioctl, shall we kill the guest?.....) > > We actually need to test the address for validity via the memslots (and not > via return value of copy_from/to_user) all across the s390 code. > > I really want to avoid mixing this effort with the virtio-ccw patches. > So my proposal is to apply your patch below and keep Conny's patch as is. > Ok? Sure. :) -- To unsubscribe from this list: send the line "unsubscribe kvm" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
