From e51d31c454fbd64e5de8d85c94bb519228f4c78a Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Sun, 23 Jun 2024 11:26:50 +0200 Subject: xen/manage: Constify struct shutdown_handler 'struct shutdown_handler' is not modified in this driver. Constifying this structure moves some data to a read-only section, so increase overall security. On a x86_64, with allmodconfig: Before: ====== text data bss dec hex filename 7043 788 8 7839 1e9f drivers/xen/manage.o After: ===== text data bss dec hex filename 7164 676 8 7848 1ea8 drivers/xen/manage.o Signed-off-by: Christophe JAILLET Reviewed-by: Juergen Gross Link: https://lore.kernel.org/r/ca1e75f66aed43191cf608de6593c7d6db9148f1.1719134768.git.christophe.jaillet@wanadoo.fr Signed-off-by: Juergen Gross --- drivers/xen/manage.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/xen/manage.c b/drivers/xen/manage.c index c16df629907e..b4b4ebed68da 100644 --- a/drivers/xen/manage.c +++ b/drivers/xen/manage.c @@ -208,7 +208,7 @@ static void do_reboot(void) orderly_reboot(); } -static struct shutdown_handler shutdown_handlers[] = { +static const struct shutdown_handler shutdown_handlers[] = { { "poweroff", true, do_poweroff }, { "halt", false, do_poweroff }, { "reboot", true, do_reboot }, -- cgit v1.2.3 From 7cd23c1817b8f9df61dac67848d9593b1ca8882f Mon Sep 17 00:00:00 2001 From: Jeff Johnson Date: Tue, 11 Jun 2024 16:54:05 -0700 Subject: xen: add missing MODULE_DESCRIPTION() macros With ARCH=x86, make allmodconfig && make W=1 C=1 reports: WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/xen/xen-pciback/xen-pciback.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/xen/xen-evtchn.o WARNING: modpost: missing MODULE_DESCRIPTION() in drivers/xen/xen-privcmd.o Add the missing invocations of the MODULE_DESCRIPTION() macro. Signed-off-by: Jeff Johnson Reviewed-by: Juergen Gross Link: https://lore.kernel.org/r/20240611-md-drivers-xen-v1-1-1eb677364ca6@quicinc.com Signed-off-by: Juergen Gross --- drivers/xen/evtchn.c | 1 + drivers/xen/privcmd-buf.c | 1 + drivers/xen/privcmd.c | 1 + drivers/xen/xen-pciback/pci_stub.c | 1 + 4 files changed, 4 insertions(+) (limited to 'drivers') diff --git a/drivers/xen/evtchn.c b/drivers/xen/evtchn.c index f6a2216c2c87..9b7fcc7dbb38 100644 --- a/drivers/xen/evtchn.c +++ b/drivers/xen/evtchn.c @@ -729,4 +729,5 @@ static void __exit evtchn_cleanup(void) module_init(evtchn_init); module_exit(evtchn_cleanup); +MODULE_DESCRIPTION("Xen /dev/xen/evtchn device driver"); MODULE_LICENSE("GPL"); diff --git a/drivers/xen/privcmd-buf.c b/drivers/xen/privcmd-buf.c index 2fa10ca5be14..0f0dad427d7e 100644 --- a/drivers/xen/privcmd-buf.c +++ b/drivers/xen/privcmd-buf.c @@ -19,6 +19,7 @@ #include "privcmd.h" +MODULE_DESCRIPTION("Xen Mmap of hypercall buffers"); MODULE_LICENSE("GPL"); struct privcmd_buf_private { diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index 67dfa4778864..b9b784633c01 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -48,6 +48,7 @@ #include "privcmd.h" +MODULE_DESCRIPTION("Xen hypercall passthrough driver"); MODULE_LICENSE("GPL"); #define PRIV_VMA_LOCKED ((void *)1) diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c index e34b623e4b41..4faebbb84999 100644 --- a/drivers/xen/xen-pciback/pci_stub.c +++ b/drivers/xen/xen-pciback/pci_stub.c @@ -1708,5 +1708,6 @@ static void __exit xen_pcibk_cleanup(void) module_init(xen_pcibk_init); module_exit(xen_pcibk_cleanup); +MODULE_DESCRIPTION("Xen PCI-device stub driver"); MODULE_LICENSE("Dual BSD/GPL"); MODULE_ALIAS("xen-backend:pci"); -- cgit v1.2.3 From 1c682593096a487fd9aebc079a307ff7a6d054a3 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 18 Jun 2024 15:12:28 +0530 Subject: xen: privcmd: Switch from mutex to spinlock for irqfds irqfd_wakeup() gets EPOLLHUP, when it is called by eventfd_release() by way of wake_up_poll(&ctx->wqh, EPOLLHUP), which gets called under spin_lock_irqsave(). We can't use a mutex here as it will lead to a deadlock. Fix it by switching over to a spin lock. Reported-by: Al Viro Signed-off-by: Viresh Kumar Reviewed-by: Juergen Gross Link: https://lore.kernel.org/r/a66d7a7a9001424d432f52a9fc3931a1f345464f.1718703669.git.viresh.kumar@linaro.org Signed-off-by: Juergen Gross --- drivers/xen/privcmd.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) (limited to 'drivers') diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index b9b784633c01..f4ba962de62b 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -846,7 +846,7 @@ out: #ifdef CONFIG_XEN_PRIVCMD_EVENTFD /* Irqfd support */ static struct workqueue_struct *irqfd_cleanup_wq; -static DEFINE_MUTEX(irqfds_lock); +static DEFINE_SPINLOCK(irqfds_lock); static LIST_HEAD(irqfds_list); struct privcmd_kernel_irqfd { @@ -910,9 +910,11 @@ irqfd_wakeup(wait_queue_entry_t *wait, unsigned int mode, int sync, void *key) irqfd_inject(kirqfd); if (flags & EPOLLHUP) { - mutex_lock(&irqfds_lock); + unsigned long flags; + + spin_lock_irqsave(&irqfds_lock, flags); irqfd_deactivate(kirqfd); - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); } return 0; @@ -930,6 +932,7 @@ irqfd_poll_func(struct file *file, wait_queue_head_t *wqh, poll_table *pt) static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) { struct privcmd_kernel_irqfd *kirqfd, *tmp; + unsigned long flags; __poll_t events; struct fd f; void *dm_op; @@ -969,18 +972,18 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) init_waitqueue_func_entry(&kirqfd->wait, irqfd_wakeup); init_poll_funcptr(&kirqfd->pt, irqfd_poll_func); - mutex_lock(&irqfds_lock); + spin_lock_irqsave(&irqfds_lock, flags); list_for_each_entry(tmp, &irqfds_list, list) { if (kirqfd->eventfd == tmp->eventfd) { ret = -EBUSY; - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); goto error_eventfd; } } list_add_tail(&kirqfd->list, &irqfds_list); - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); /* * Check if there was an event already pending on the eventfd before we @@ -1012,12 +1015,13 @@ static int privcmd_irqfd_deassign(struct privcmd_irqfd *irqfd) { struct privcmd_kernel_irqfd *kirqfd; struct eventfd_ctx *eventfd; + unsigned long flags; eventfd = eventfd_ctx_fdget(irqfd->fd); if (IS_ERR(eventfd)) return PTR_ERR(eventfd); - mutex_lock(&irqfds_lock); + spin_lock_irqsave(&irqfds_lock, flags); list_for_each_entry(kirqfd, &irqfds_list, list) { if (kirqfd->eventfd == eventfd) { @@ -1026,7 +1030,7 @@ static int privcmd_irqfd_deassign(struct privcmd_irqfd *irqfd) } } - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); eventfd_ctx_put(eventfd); @@ -1074,13 +1078,14 @@ static int privcmd_irqfd_init(void) static void privcmd_irqfd_exit(void) { struct privcmd_kernel_irqfd *kirqfd, *tmp; + unsigned long flags; - mutex_lock(&irqfds_lock); + spin_lock_irqsave(&irqfds_lock, flags); list_for_each_entry_safe(kirqfd, tmp, &irqfds_list, list) irqfd_deactivate(kirqfd); - mutex_unlock(&irqfds_lock); + spin_unlock_irqrestore(&irqfds_lock, flags); destroy_workqueue(irqfd_cleanup_wq); } -- cgit v1.2.3 From 611ff1b1ae989a7bcce3e2a8e132ee30e968c557 Mon Sep 17 00:00:00 2001 From: Viresh Kumar Date: Tue, 18 Jun 2024 15:12:29 +0530 Subject: xen: privcmd: Fix possible access to a freed kirqfd instance Nothing prevents simultaneous ioctl calls to privcmd_irqfd_assign() and privcmd_irqfd_deassign(). If that happens, it is possible that a kirqfd created and added to the irqfds_list by privcmd_irqfd_assign() may get removed by another thread executing privcmd_irqfd_deassign(), while the former is still using it after dropping the locks. This can lead to a situation where an already freed kirqfd instance may be accessed and cause kernel oops. Use SRCU locking to prevent the same, as is done for the KVM implementation for irqfds. Reported-by: Al Viro Suggested-by: Paolo Bonzini Signed-off-by: Viresh Kumar Reviewed-by: Juergen Gross Link: https://lore.kernel.org/r/9e884af1f1f842eacbb7afc5672c8feb4dea7f3f.1718703669.git.viresh.kumar@linaro.org Signed-off-by: Juergen Gross --- drivers/xen/privcmd.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) (limited to 'drivers') diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c index f4ba962de62b..9563650dfbaf 100644 --- a/drivers/xen/privcmd.c +++ b/drivers/xen/privcmd.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -847,6 +848,7 @@ out: /* Irqfd support */ static struct workqueue_struct *irqfd_cleanup_wq; static DEFINE_SPINLOCK(irqfds_lock); +DEFINE_STATIC_SRCU(irqfds_srcu); static LIST_HEAD(irqfds_list); struct privcmd_kernel_irqfd { @@ -874,6 +876,9 @@ static void irqfd_shutdown(struct work_struct *work) container_of(work, struct privcmd_kernel_irqfd, shutdown); u64 cnt; + /* Make sure irqfd has been initialized in assign path */ + synchronize_srcu(&irqfds_srcu); + eventfd_ctx_remove_wait_queue(kirqfd->eventfd, &kirqfd->wait, &cnt); eventfd_ctx_put(kirqfd->eventfd); kfree(kirqfd); @@ -936,7 +941,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) __poll_t events; struct fd f; void *dm_op; - int ret; + int ret, idx; kirqfd = kzalloc(sizeof(*kirqfd) + irqfd->size, GFP_KERNEL); if (!kirqfd) @@ -982,6 +987,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) } } + idx = srcu_read_lock(&irqfds_srcu); list_add_tail(&kirqfd->list, &irqfds_list); spin_unlock_irqrestore(&irqfds_lock, flags); @@ -993,6 +999,8 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd) if (events & EPOLLIN) irqfd_inject(kirqfd); + srcu_read_unlock(&irqfds_srcu, idx); + /* * Do not drop the file until the kirqfd is fully initialized, otherwise * we might race against the EPOLLHUP. -- cgit v1.2.3