linux-kernel - Re: [PATCH v5 10/10] Drivers: hv: Introduce mshv

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: <Z9msXAClr-vGn3GR@liuwe-devbox-ubuntu-v2.lamzopl0uupeniq2etz1fddiyg.xx.internal.cloudapp.net>
Date: Tue, 18 Mar 2025 17:24:44 +0000
From: Wei Liu <wei.liu@...nel.org>
To: Michael Kelley <mhklinux@...look.com>
Cc: Nuno Das Neves <nunodasneves@...ux.microsoft.com>,
	"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-arch@...r.kernel.org" <linux-arch@...r.kernel.org>,
	"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>,
	"kys@...rosoft.com" <kys@...rosoft.com>,
	"haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
	"wei.liu@...nel.org" <wei.liu@...nel.org>,
	"decui@...rosoft.com" <decui@...rosoft.com>,
	"catalin.marinas@....com" <catalin.marinas@....com>,
	"will@...nel.org" <will@...nel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>,
	"mingo@...hat.com" <mingo@...hat.com>,
	"bp@...en8.de" <bp@...en8.de>,
	"dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
	"hpa@...or.com" <hpa@...or.com>,
	"daniel.lezcano@...aro.org" <daniel.lezcano@...aro.org>,
	"joro@...tes.org" <joro@...tes.org>,
	"robin.murphy@....com" <robin.murphy@....com>,
	"arnd@...db.de" <arnd@...db.de>,
	"jinankjain@...ux.microsoft.com" <jinankjain@...ux.microsoft.com>,
	"muminulrussell@...il.com" <muminulrussell@...il.com>,
	"skinsburskii@...ux.microsoft.com" <skinsburskii@...ux.microsoft.com>,
	"mrathor@...ux.microsoft.com" <mrathor@...ux.microsoft.com>,
	"ssengar@...ux.microsoft.com" <ssengar@...ux.microsoft.com>,
	"apais@...ux.microsoft.com" <apais@...ux.microsoft.com>,
	"Tianyu.Lan@...rosoft.com" <Tianyu.Lan@...rosoft.com>,
	"stanislav.kinsburskiy@...il.com" <stanislav.kinsburskiy@...il.com>,
	"gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
	"vkuznets@...hat.com" <vkuznets@...hat.com>,
	"prapal@...ux.microsoft.com" <prapal@...ux.microsoft.com>,
	"muislam@...rosoft.com" <muislam@...rosoft.com>,
	"anrayabh@...ux.microsoft.com" <anrayabh@...ux.microsoft.com>,
	"rafael@...nel.org" <rafael@...nel.org>,
	"lenb@...nel.org" <lenb@...nel.org>,
	"corbet@....net" <corbet@....net>
Subject: Re: [PATCH v5 10/10] Drivers: hv: Introduce mshv_root module to
 expose /dev/mshv to VMMs

On Mon, Mar 17, 2025 at 11:51:52PM +0000, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@...ux.microsoft.com> Sent: Wednesday, February 26, 2025 3:08 PM
[...]
> > +static long
> > +mshv_vp_ioctl_get_set_state(struct mshv_vp *vp,
> > +			    struct mshv_get_set_vp_state __user *user_args,
> > +			    bool is_set)
> > +{
> > +	struct mshv_get_set_vp_state args;
> > +	long ret = 0;
> > +	union hv_output_get_vp_state vp_state;
> > +	u32 data_sz;
> > +	struct hv_vp_state_data state_data = {};
> > +
> > +	if (copy_from_user(&args, user_args, sizeof(args)))
> > +		return -EFAULT;
> > +
> > +	if (args.type >= MSHV_VP_STATE_COUNT || mshv_field_nonzero(args, rsvd) ||
> > +	    !args.buf_sz || !PAGE_ALIGNED(args.buf_sz) ||
> > +	    !PAGE_ALIGNED(args.buf_ptr))
> > +		return -EINVAL;
> > +
> > +	if (!access_ok((void __user *)args.buf_ptr, args.buf_sz))
> > +		return -EFAULT;
> > +
> > +	switch (args.type) {
> > +	case MSHV_VP_STATE_LAPIC:
> > +		state_data.type = HV_GET_SET_VP_STATE_LAPIC_STATE;
> > +		data_sz = HV_HYP_PAGE_SIZE;
> > +		break;
> > +	case MSHV_VP_STATE_XSAVE:
> 
> Just FYI, you can put a semicolon after the colon on the above line, which
> adds a null statement, and then the C compiler will accept the definition
> of local variable data_sz_64 without needing the odd-looking braces. 
> 
> See https://stackoverflow.com/questions/92396/why-cant-variables-be-declared-in-a-switch-statement/19830820
> 

This is a rarely seen pattern in the kernel, so I would prefer to keep
the braces for clarity.

 $ git grep -A5 -P 'case\s+\w+:;$'

This shows a few places are using this pattern. But they are not
declaring variables afterwards.

> I learn something new every day! :-)
> 

Yep, me too.

Thanks for reviewing the code. Nuno will address the comments. I can fix
them up.

Thanks,
Wei.