linux-kernel - Re: [PATCH v14 01/22] KVM: x86: Introduce KVM_{G,S}ET_ONE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aMG28ahWdS-2gHk6@google.com>
Date: Wed, 10 Sep 2025 10:35:45 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Chao Gao <chao.gao@...el.com>
Cc: kvm@...r.kernel.org, linux-kernel@...r.kernel.org, acme@...hat.com, 
	bp@...en8.de, dave.hansen@...ux.intel.com, hpa@...or.com, john.allen@....com, 
	mingo@...nel.org, mingo@...hat.com, minipli@...ecurity.net, 
	mlevitsk@...hat.com, namhyung@...nel.org, pbonzini@...hat.com, 
	prsampat@....com, rick.p.edgecombe@...el.com, shuah@...nel.org, 
	tglx@...utronix.de, weijiang.yang@...el.com, x86@...nel.org, xin@...or.com, 
	xiaoyao.li@...el.com
Subject: Re: [PATCH v14 01/22] KVM: x86: Introduce KVM_{G,S}ET_ONE_REG uAPIs support

On Tue, Sep 09, 2025, Chao Gao wrote:
> @@ -6031,6 +6071,44 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  		srcu_read_unlock(&vcpu->kvm->srcu, idx);
>  		break;
>  	}
> +	case KVM_GET_ONE_REG:
> +	case KVM_SET_ONE_REG: {
> +		struct kvm_x86_reg_id *id;
> +		struct kvm_one_reg reg;
> +		u64 __user *value;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&reg, argp, sizeof(reg)))
> +			break;
> +
> +		r = -EINVAL;
> +		if ((reg.id & KVM_REG_ARCH_MASK) != KVM_REG_X86)
> +			break;
> +
> +		id = (struct kvm_x86_reg_id *)&reg.id;
> +		if (id->rsvd1 || id->rsvd2)
> +			break;
> +
> +		if (id->type == KVM_X86_REG_TYPE_KVM) {
> +			r = kvm_translate_kvm_reg(id);
> +			if (r)
> +				break;
> +		}
> +
> +		r = -EINVAL;
> +		if (id->type != KVM_X86_REG_TYPE_MSR)
> +			break;
> +
> +		if ((reg.id & KVM_REG_SIZE_MASK) != KVM_REG_SIZE_U64)
> +			break;
> +

Almost forgot.  I think it makes sense to grab kvm->srcu here.  I'm not entirely
positive that's necessary these days, e.g. after commit 3617c0ee7dec ("KVM: x86/xen:
Only write Xen hypercall page for guest writes to MSR")but there are path, but
_proving_ that there are no memory or MSR/PMU filter accesses is practically
impossible given how much code is reachable via MSR emulation.

If someone wants to put in the effort to prove SRCU isn't needed, then we should
also drop SRCU protection from KVM_{G,S}ET_MSRS.

> +		value = u64_to_user_ptr(reg.addr);
> +		if (ioctl == KVM_GET_ONE_REG)
> +			r = kvm_get_one_msr(vcpu, id->index, value);
> +		else
> +			r = kvm_set_one_msr(vcpu, id->index, value);
> +		break;
> +	}
>  	case KVM_TPR_ACCESS_REPORTING: {
>  		struct kvm_tpr_access_ctl tac;
>  
> -- 
> 2.47.3
>