On 06/07/2017 14:51, Christian Borntraeger wrote:
> We do use rcu to protect the pid pointer. Mark it as such and
> adopt all code to use the proper access methods.
>
> This was detected by sparse.
> "virt/kvm/kvm_main.c:2248:15: error: incompatible types in comparison
> expression (different address spaces)"
>
> Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
> ---
> include/linux/kvm_host.h | 2 +-
> virt/kvm/kvm_main.c | 15 +++++++++++----
> 2 files changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
> index 0b50e7b..bcd37b8 100644
> --- a/include/linux/kvm_host.h
> +++ b/include/linux/kvm_host.h
> @@ -234,7 +234,7 @@ struct kvm_vcpu {
>
> int guest_fpu_loaded, guest_xcr0_loaded;
> struct swait_queue_head wq;
> - struct pid *pid;
> + struct pid __rcu *pid;
> int sigset_active;
> sigset_t sigset;
> struct kvm_vcpu_stat stat;
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 19f0ecb..7bd5509 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -293,7 +293,12 @@ EXPORT_SYMBOL_GPL(kvm_vcpu_init);
>
> void kvm_vcpu_uninit(struct kvm_vcpu *vcpu)
> {
> - put_pid(vcpu->pid);
> + /*
> + * no need for rcu_read_lock as VCPU_RUN is the only place that
> + * will change the vcpu->pid pointer and on uninit all file
> + * descriptors are already gone.
> + */
> + put_pid(rcu_dereference(vcpu->pid));
> kvm_arch_vcpu_uninit(vcpu);
> free_page((unsigned long)vcpu->run);
> }
> @@ -2551,13 +2556,14 @@ static long kvm_vcpu_ioctl(struct file *filp,
> if (r)
> return r;
> switch (ioctl) {
> - case KVM_RUN:
> + case KVM_RUN: {
> + struct pid *oldpid;
> r = -EINVAL;
> if (arg)
> goto out;
> - if (unlikely(vcpu->pid != current->pids[PIDTYPE_PID].pid)) {
> + oldpid = rcu_dereference(vcpu->pid);
> + if (unlikely(oldpid != current->pids[PIDTYPE_PID].pid)) {
Since we never dereference oldpid, the rcu_dereference should be
rcu_access_pointer instead.
Otherwise
Reviewed-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> /* The thread running this VCPU changed. */
> - struct pid *oldpid = vcpu->pid;
> struct pid *newpid = get_task_pid(current, PIDTYPE_PID);
>
> rcu_assign_pointer(vcpu->pid, newpid);
> @@ -2568,6 +2574,7 @@ static long kvm_vcpu_ioctl(struct file *filp,
> r = kvm_arch_vcpu_ioctl_run(vcpu, vcpu->run);
> trace_kvm_userspace_exit(vcpu->run->exit_reason, r);
> break;
> + }
> case KVM_GET_REGS: {
> struct kvm_regs *kvm_regs;
>
>
