On 7/25/2025 8:05 PM, Michael Kelley wrote:
From: Naman Jain <namj...@linux.microsoft.com> Sent: Thursday, July 24, 2025
10:54 PM
On 7/25/2025 8:52 AM, Michael Kelley wrote:
From: Naman Jain <namj...@linux.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;
+}
+
