Skip to content
This repository was archived by the owner on Nov 8, 2023. It is now read-only.

Commit 611ff1b

Browse files
vireshkjgross1
authored andcommitted
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 <viro@zeniv.linux.org.uk> Suggested-by: Paolo Bonzini <pbonzini@redhat.com> Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> Reviewed-by: Juergen Gross <jgross@suse.com> Link: https://lore.kernel.org/r/9e884af1f1f842eacbb7afc5672c8feb4dea7f3f.1718703669.git.viresh.kumar@linaro.org Signed-off-by: Juergen Gross <jgross@suse.com>
1 parent 1c68259 commit 611ff1b

File tree

1 file changed

+9
-1
lines changed

1 file changed

+9
-1
lines changed

drivers/xen/privcmd.c

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <linux/poll.h>
1818
#include <linux/sched.h>
1919
#include <linux/slab.h>
20+
#include <linux/srcu.h>
2021
#include <linux/string.h>
2122
#include <linux/workqueue.h>
2223
#include <linux/errno.h>
@@ -847,6 +848,7 @@ static long privcmd_ioctl_mmap_resource(struct file *file,
847848
/* Irqfd support */
848849
static struct workqueue_struct *irqfd_cleanup_wq;
849850
static DEFINE_SPINLOCK(irqfds_lock);
851+
DEFINE_STATIC_SRCU(irqfds_srcu);
850852
static LIST_HEAD(irqfds_list);
851853

852854
struct privcmd_kernel_irqfd {
@@ -874,6 +876,9 @@ static void irqfd_shutdown(struct work_struct *work)
874876
container_of(work, struct privcmd_kernel_irqfd, shutdown);
875877
u64 cnt;
876878

879+
/* Make sure irqfd has been initialized in assign path */
880+
synchronize_srcu(&irqfds_srcu);
881+
877882
eventfd_ctx_remove_wait_queue(kirqfd->eventfd, &kirqfd->wait, &cnt);
878883
eventfd_ctx_put(kirqfd->eventfd);
879884
kfree(kirqfd);
@@ -936,7 +941,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd)
936941
__poll_t events;
937942
struct fd f;
938943
void *dm_op;
939-
int ret;
944+
int ret, idx;
940945

941946
kirqfd = kzalloc(sizeof(*kirqfd) + irqfd->size, GFP_KERNEL);
942947
if (!kirqfd)
@@ -982,6 +987,7 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd)
982987
}
983988
}
984989

990+
idx = srcu_read_lock(&irqfds_srcu);
985991
list_add_tail(&kirqfd->list, &irqfds_list);
986992
spin_unlock_irqrestore(&irqfds_lock, flags);
987993

@@ -993,6 +999,8 @@ static int privcmd_irqfd_assign(struct privcmd_irqfd *irqfd)
993999
if (events & EPOLLIN)
9941000
irqfd_inject(kirqfd);
9951001

1002+
srcu_read_unlock(&irqfds_srcu, idx);
1003+
9961004
/*
9971005
* Do not drop the file until the kirqfd is fully initialized, otherwise
9981006
* we might race against the EPOLLHUP.

0 commit comments

Comments
 (0)