Gregory Haskins wrote:
Avi Kivity wrote:
Gregory Haskins wrote:
+int
+kvm_irqfd(struct kvm *kvm, int gsi, int flags)
+{
+ struct _irqfd *irqfd;
+ struct file *file = NULL;
+ int fd = -1;
+ int ret;
+
+ irqfd = kzalloc(sizeof(*irqfd), GFP_KERNEL);
+ if (!irqfd)
+ return -ENOMEM;
+
+ irqfd->kvm = kvm;
You need to increase the refcount on struct kvm here. Otherwise evil
userspace will create an irqfd, close the vm and vcpu fds, and inject
an interrupt.
I just reviewed the code in prep for v5, and now I remember why I didnt
take a reference: I designed it the opposite direction: the vm-fd owns
a reference to the irqfd, and will decouple the kvm context from the
eventfd on shutdown (see kvm_irqfd_release()). I still need to spin a
v5 regardless in order to add the padding as previously discussed. But
let me know if you still see any holes in light of this alternate object
lifetime approach I am using.
Right, irqfd_release works. But I think refcounting is simpler, since
we already kvm_get_kvm() and kvm_put_kvm(), and you wouldn't need the
irqfd list. On the other hand, I'm not sure you get a callback from
eventfd on close(), so refcounting may not be implementable.
Drat, irqfd_release doesn't work. You reference kvm->lock in
irqfd_inject without taking any locks.
btw, there's still your original idea of creating the eventfd in
userspace and passing it down. That would be workable if we can see a
way to both signal the eventfd and get called back in irq context.
Maybe that's preferable to what we're doing here, but we need to see how
it would work.
--
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
