linux-kernel - RE: [PATCH v6 2/2] 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:
 <SN6PR02MB41579F474B6FC43D4E5754CDD459A@SN6PR02MB4157.namprd02.prod.outlook.com>
Date: Fri, 25 Jul 2025 14:35:06 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Naman Jain <namjain@...ux.microsoft.com>, "K . Y . Srinivasan"
	<kys@...rosoft.com>, Haiyang Zhang <haiyangz@...rosoft.com>, Wei Liu
	<wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>
CC: Roman Kisel <romank@...ux.microsoft.com>, Anirudh Rayabharam
	<anrayabh@...ux.microsoft.com>, Saurabh Sengar <ssengar@...ux.microsoft.com>,
	Stanislav Kinsburskii <skinsburskii@...ux.microsoft.com>, Nuno Das Neves
	<nunodasneves@...ux.microsoft.com>, ALOK TIWARI <alok.a.tiwari@...cle.com>,
	Markus Elfring <Markus.Elfring@....de>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>
Subject: RE: [PATCH v6 2/2] Drivers: hv: Introduce mshv_vtl driver

From: Naman Jain <namjain@...ux.microsoft.com> Sent: Thursday, July 24, 2025 10:54 PM
> 
> On 7/25/2025 8:52 AM, Michael Kelley wrote:
> > From: Naman Jain <namjain@...ux.microsoft.com> Sent: Thursday, July 24, 2025 1:26 AM
> >>

[snip]

> >> +
> >> +static int mshv_vtl_sint_ioctl_set_eventfd(struct mshv_vtl_set_eventfd __user *arg)
> >> +{
> >> +	struct mshv_vtl_set_eventfd set_eventfd;
> >> +	struct eventfd_ctx *eventfd, *old_eventfd;
> >> +
> >> +	if (copy_from_user(&set_eventfd, arg, sizeof(set_eventfd)))
> >> +		return -EFAULT;
> >> +	if (set_eventfd.flag >= HV_EVENT_FLAGS_COUNT)
> >> +		return -EINVAL;
> >> +
> >> +	eventfd = NULL;
> >> +	if (set_eventfd.fd >= 0) {
> >> +		eventfd = eventfd_ctx_fdget(set_eventfd.fd);
> >> +		if (IS_ERR(eventfd))
> >> +			return PTR_ERR(eventfd);
> >> +	}
> >> +
> >> +	guard(mutex)(&flag_lock);
> >> +	old_eventfd = READ_ONCE(flag_eventfds[set_eventfd.flag]);
> >> +	WRITE_ONCE(flag_eventfds[set_eventfd.flag], eventfd);
> >> +
> >> +	if (old_eventfd) {
> >> +		synchronize_rcu();
> >> +		eventfd_ctx_put(old_eventfd);
> >
> > Again, I wonder if is OK to do eventfd_ctx_put() while holding
> > flag_lock, since the use of guard() changes the scope of the lock
> > compared with the previous version of this patch.
> >
> 
> I didn't find eventfd_ctx_put() to be a blocking operation, so I thought
> of keeping guard() here. Although, synchronize_rcu() is a blocking
> operation. Please advise, I am Ok with removing the guard, as the lock
> is just being used here, and automatic cleanup should not be an issue
> here.

Yes, I think you are right. I saw the kref_put() and was unsure what
would be called if the object was freed. But the "free" function is
right there staring at me. :-) All it does is ida_free() and kfree(),
both of which would be safe.

You should be good keeping the guard().

Michael

> 
> 
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +