* [PATCH] kvm-kmod: fix kvm_request_irq race @ 2009-05-21 6:21 Chris Wright 2009-05-21 6:29 ` Avi Kivity 0 siblings, 1 reply; 5+ messages in thread From: Chris Wright @ 2009-05-21 6:21 UTC (permalink / raw) To: Avi Kivity; +Cc: kvm Commit "32658734: Fix request_irq() for < 2.6.19" is racy between multiple guests since ioctl is only serialized per guest. Add mutex and serialize kvm_request_irq/kvm_free_irq to avoid race. Signed-off-by: Chris Wright <chrisw@redhat.com> --- external-module-compat-comm.h | 14 +++++++++++--- 1 files changed, 11 insertions(+), 3 deletions(-) diff --git a/external-module-compat-comm.h b/external-module-compat-comm.h index 8cb5440..eaad986 100644 --- a/external-module-compat-comm.h +++ b/external-module-compat-comm.h @@ -645,6 +645,7 @@ static inline int pci_reset_function(struct pci_dev *dev) typedef irqreturn_t (*kvm_irq_handler_t)(int, void *); static kvm_irq_handler_t kvm_irq_handlers[NR_IRQS]; +static DEFINE_MUTEX(kvm_irq_handlers_mutex); static irqreturn_t kvm_irq_thunk(int irq, void *dev_id, struct pt_regs *regs) { @@ -655,21 +656,28 @@ static irqreturn_t kvm_irq_thunk(int irq, void *dev_id, struct pt_regs *regs) static inline int kvm_request_irq(unsigned int a, kvm_irq_handler_t handler, unsigned long c, const char *d, void *e) { - int rc; - kvm_irq_handler_t old = kvm_irq_handlers[a]; + int rc = -EBUSY; + kvm_irq_handler_t old; + + mutex_lock(&kvm_irq_handlers_mutex); + old = kvm_irq_handlers[a]; if (old) - return -EBUSY; + goto out; kvm_irq_handlers[a] = handler; rc = request_irq(a, kvm_irq_thunk, c, d, e); if (rc) kvm_irq_handlers[a] = NULL; +out: + mutex_unlock(&kvm_irq_handlers_mutex); return rc; } static inline void kvm_free_irq(unsigned int irq, void *dev_id) { + mutex_lock(&kvm_irq_handlers_mutex); free_irq(irq, dev_id); kvm_irq_handlers[irq] = NULL; + mutex_unlock(&kvm_irq_handlers_mutex); } #else ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm-kmod: fix kvm_request_irq race 2009-05-21 6:21 [PATCH] kvm-kmod: fix kvm_request_irq race Chris Wright @ 2009-05-21 6:29 ` Avi Kivity 2009-05-21 6:38 ` Chris Wright 2009-05-21 7:38 ` [PATCH kvm-kmod v2] " Chris Wright 0 siblings, 2 replies; 5+ messages in thread From: Avi Kivity @ 2009-05-21 6:29 UTC (permalink / raw) To: Chris Wright; +Cc: kvm Chris Wright wrote: > Commit "32658734: Fix request_irq() for < 2.6.19" is racy between multiple > guests since ioctl is only serialized per guest. Add mutex and serialize > kvm_request_irq/kvm_free_irq to avoid race. > > Signed-off-by: Chris Wright <chrisw@redhat.com> > --- > external-module-compat-comm.h | 14 +++++++++++--- > 1 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/external-module-compat-comm.h b/external-module-compat-comm.h > index 8cb5440..eaad986 100644 > --- a/external-module-compat-comm.h > +++ b/external-module-compat-comm.h > @@ -645,6 +645,7 @@ static inline int pci_reset_function(struct pci_dev *dev) > > typedef irqreturn_t (*kvm_irq_handler_t)(int, void *); > static kvm_irq_handler_t kvm_irq_handlers[NR_IRQS]; > +static DEFINE_MUTEX(kvm_irq_handlers_mutex); > > Since this mutex is in a header file, it can be instantiated multiple times. It will only serialize callers within a translation unit. Please define it in a C file. Would be best to move the the code as well. -- Do not meddle in the internals of kernels, for they are subtle and quick to panic. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kvm-kmod: fix kvm_request_irq race 2009-05-21 6:29 ` Avi Kivity @ 2009-05-21 6:38 ` Chris Wright 2009-05-21 7:38 ` [PATCH kvm-kmod v2] " Chris Wright 1 sibling, 0 replies; 5+ messages in thread From: Chris Wright @ 2009-05-21 6:38 UTC (permalink / raw) To: Avi Kivity; +Cc: Chris Wright, kvm * Avi Kivity (avi@redhat.com) wrote: > Since this mutex is in a header file, it can be instantiated multiple > times. It will only serialize callers within a translation unit. > Please define it in a C file. > > Would be best to move the the code as well. Yeah it seems to get linked multiple times (when I tried that I found duplicates). I'll look again. thanks, -chris ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH kvm-kmod v2] fix kvm_request_irq race 2009-05-21 6:29 ` Avi Kivity 2009-05-21 6:38 ` Chris Wright @ 2009-05-21 7:38 ` Chris Wright 2009-05-21 7:45 ` Avi Kivity 1 sibling, 1 reply; 5+ messages in thread From: Chris Wright @ 2009-05-21 7:38 UTC (permalink / raw) To: Avi Kivity; +Cc: Chris Wright, kvm Commit "32658734: Fix request_irq() for < 2.6.19" is racy between multiple guests since ioctl is only serialized per guest. Add mutex and serialize kvm_request_irq/kvm_free_irq to avoid race. Also move all this to its own C file to keep from creating per ojbect file private mutexes. Signed-off-by: Chris Wright <chrisw@redhat.com> --- external-module-compat-comm.h | 29 +------------------------- ia64/Kbuild | 3 +- request-irq-compat.c | 44 +++++++++++++++++++++++++++++++++++++++++ x86/Kbuild | 2 +- 4 files changed, 49 insertions(+), 29 deletions(-) diff --git a/external-module-compat-comm.h b/external-module-compat-comm.h index f6975e0..c014b7f 100644 --- a/external-module-compat-comm.h +++ b/external-module-compat-comm.h @@ -656,33 +656,8 @@ static inline int pci_reset_function(struct pci_dev *dev) #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,19) typedef irqreturn_t (*kvm_irq_handler_t)(int, void *); -static kvm_irq_handler_t kvm_irq_handlers[NR_IRQS]; - -static irqreturn_t kvm_irq_thunk(int irq, void *dev_id, struct pt_regs *regs) -{ - kvm_irq_handler_t handler = kvm_irq_handlers[irq]; - return handler(irq, dev_id); -} - -static inline int kvm_request_irq(unsigned int a, kvm_irq_handler_t handler, - unsigned long c, const char *d, void *e) -{ - int rc; - kvm_irq_handler_t old = kvm_irq_handlers[a]; - if (old) - return -EBUSY; - kvm_irq_handlers[a] = handler; - rc = request_irq(a, kvm_irq_thunk, c, d, e); - if (rc) - kvm_irq_handlers[a] = NULL; - return rc; -} - -static inline void kvm_free_irq(unsigned int irq, void *dev_id) -{ - free_irq(irq, dev_id); - kvm_irq_handlers[irq] = NULL; -} +int kvm_request_irq(unsigned int a, kvm_irq_handler_t handler, unsigned long c, const char *d, void *e); +void kvm_free_irq(unsigned int irq, void *dev_id); #else diff --git a/ia64/Kbuild b/ia64/Kbuild index 5bc6098..e2c8acc 100644 --- a/ia64/Kbuild +++ b/ia64/Kbuild @@ -1,7 +1,8 @@ obj-m := kvm.o kvm-intel.o kvm-objs := kvm_main.o ioapic.o coalesced_mmio.o kvm-ia64.o kvm_fw.o \ - irq_comm.o ../anon_inodes.o ../external-module-compat.o + irq_comm.o ../anon_inodes.o ../external-module-compat.o \ + ../request-irq-compat.o ifeq ($(CONFIG_IOMMU_API),y) kvm-objs += iommu.o diff --git a/request-irq-compat.c b/request-irq-compat.c new file mode 100644 index 0000000..51193cb --- /dev/null +++ b/request-irq-compat.c @@ -0,0 +1,44 @@ +/* + * compat for request_irq + */ + +#include <linux/interrupt.h> +#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,19) + +static kvm_irq_handler_t kvm_irq_handlers[NR_IRQS]; +static DEFINE_MUTEX(kvm_irq_handlers_mutex); + +static irqreturn_t kvm_irq_thunk(int irq, void *dev_id, struct pt_regs *regs) +{ + kvm_irq_handler_t handler = kvm_irq_handlers[irq]; + return handler(irq, dev_id); +} + +int kvm_request_irq(unsigned int a, kvm_irq_handler_t handler, + unsigned long c, const char *d, void *e) +{ + int rc = -EBUSY; + kvm_irq_handler_t old; + + mutex_lock(&kvm_irq_handlers_mutex); + old = kvm_irq_handlers[a]; + if (old) + goto out; + kvm_irq_handlers[a] = handler; + rc = request_irq(a, kvm_irq_thunk, c, d, e); + if (rc) + kvm_irq_handlers[a] = NULL; +out: + mutex_unlock(&kvm_irq_handlers_mutex); + return rc; +} + +void kvm_free_irq(unsigned int irq, void *dev_id) +{ + mutex_lock(&kvm_irq_handlers_mutex); + free_irq(irq, dev_id); + kvm_irq_handlers[irq] = NULL; + mutex_unlock(&kvm_irq_handlers_mutex); +} + +#endif diff --git a/x86/Kbuild b/x86/Kbuild index d3aca00..c5b84e9 100644 --- a/x86/Kbuild +++ b/x86/Kbuild @@ -6,7 +6,7 @@ obj-m := kvm.o kvm-intel.o kvm-amd.o kvm-objs := kvm_main.o x86.o mmu.o x86_emulate.o ../anon_inodes.o irq.o i8259.o \ lapic.o ioapic.o preempt.o i8254.o coalesced_mmio.o irq_comm.o \ timer.o \ - ../external-module-compat.o + ../external-module-compat.o ../request-irq-compat.o ifeq ($(EXT_CONFIG_KVM_TRACE),y) kvm-objs += kvm_trace.o endif ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH kvm-kmod v2] fix kvm_request_irq race 2009-05-21 7:38 ` [PATCH kvm-kmod v2] " Chris Wright @ 2009-05-21 7:45 ` Avi Kivity 0 siblings, 0 replies; 5+ messages in thread From: Avi Kivity @ 2009-05-21 7:45 UTC (permalink / raw) To: Chris Wright; +Cc: kvm Chris Wright wrote: > Commit "32658734: Fix request_irq() for < 2.6.19" is racy between multiple > guests since ioctl is only serialized per guest. Add mutex and serialize > kvm_request_irq/kvm_free_irq to avoid race. Also move all this to its > own C file to keep from creating per ojbect file private mutexes. > > Applied, thanks. -- error compiling committee.c: too many arguments to function ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2009-05-21 7:46 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-21 6:21 [PATCH] kvm-kmod: fix kvm_request_irq race Chris Wright 2009-05-21 6:29 ` Avi Kivity 2009-05-21 6:38 ` Chris Wright 2009-05-21 7:38 ` [PATCH kvm-kmod v2] " Chris Wright 2009-05-21 7:45 ` Avi Kivity
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox