On Tue, May 02, 2023, Anish Moorthy wrote: > Thanks for nailing this down for me! One more question: should we be > concerned about any guest memory accesses occurring in the preamble to > that vcpu_run() call in kvm_arch_vcpu_ioctl_run()? > > I only see two spots from which an EFAULT could make it to userspace, > those being the sync_regs() and cui() calls. The former looks clean Ya, sync_regs() is a non-issue, that doesn't touch guest memory unless userspace is doing something truly bizarre. > but I'm not sure about the latter. As written it's not an issue per se > if the cui() call tries a vCPU memory access- the > kvm_populate_efault_info() helper will just not populate the run > struct and WARN_ON_ONCE(). But it would be good to know about. If KVM triggers a WARN_ON_ONCE(), then that's an issue. Though looking at the code, the cui() aspect is a moot point. As I stated in the previous discussion, the WARN_ON_ONCE() in question needs to be off-by-default. : Hmm, one idea would be to have the initial -EFAULT detection fill kvm_run.memory_fault, : but set kvm_run.exit_reason to some magic number, e.g. zero it out. Then KVM could : WARN if something tries to overwrite kvm_run.exit_reason. The WARN would need to : be buried by a Kconfig or something since kvm_run can be modified by userspace, : but other than that I think it would work.
