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: <9965dc77-eab9-4252-8c93-01c27e417bdc@linux.microsoft.com>
Date: Tue, 29 Jul 2025 10:38:51 +0530
From: Naman Jain <namjain@...ux.microsoft.com>
To: Michael Kelley <mhklinux@...look.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

On 7/25/2025 8:05 PM, Michael Kelley wrote:
> 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

Acked.

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