* [PATCH] xen/privcmd: fix double free when an irqfd is shut down twice
@ 2026-11-01 16:32 Qihang
0 siblings, 0 replies; 3+ messages in thread
From: Qihang @ 2026-11-01 16:32 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Stefano Stabellini, Oleksandr Tyshchenko, Viresh Kumar,
linux-kernel, Qihang, stable
irqfd_deactivate() takes the irqfd off irqfds_list and queues
irqfd_shutdown(). queue_work() is not a once-only gate: it refuses to
enqueue a work item only while WORK_STRUCT_PENDING is set, and
process_one_work() clears that bit before it calls the work function. So
nothing records that the shutdown work has already been queued, and
nothing protects the window between the start of irqfd_shutdown() and the
kfree() at its end.
Two paths call irqfd_deactivate() for the same irqfd:
- privcmd_irqfd_deassign(), from the PRIVCMD_IRQFD_FLAG_DEASSIGN ioctl;
- irqfd_wakeup(), with EPOLLHUP set, when the last reference to the
eventfd file goes away, which runs wake_up_poll(&ctx->wqh, EPOLLHUP)
from eventfd_release().
irqfds_lock serialises the two calls, but it does not make the second one
a no-op, and eventfd_ctx_fdget() takes a reference to the eventfd_ctx,
not to the struct file, so the last fd can still be closed while the
ioctl is running:
CPU0 CPU1
eventfd_ctx_fdget()
spin_lock(irqfds_lock)
list_del_init(&kirqfd->list)
queue_work(&kirqfd->shutdown) close(fd)
eventfd_release()
wake_up_poll(EPOLLHUP)
irqfd_wakeup()
spin_lock(irqfds_lock)
irqfd_deactivate() again
If the work item has already started by then, its pending bit has been
cleared and the second queue_work() succeeds. irqfd_shutdown() then runs
twice, so eventfd_ctx_remove_wait_queue(), eventfd_ctx_put() and kfree()
each hit the same object a second time; and when the second queue_work()
lands after the irqfd has been freed, the workqueue list operations write
to freed memory.
virt/kvm/eventfd.c has the same two callers and has had this guard since
the original irqfd support (721eecbf4fe99): irqfd_wakeup() calls
irqfd_deactivate() only if irqfd_is_active(). The comment there explains
why that is enough: the other side deactivates the irqfd before it takes
it off the waitqueue, and irqfd_shutdown() needs the waitqueue lock,
which this callback holds, so the irqfd cannot be freed under it.
Do the same here. list_del_init() leaves the list head empty and the
irqfd stays off the list until kfree(), so it serves as a one-way
"active" flag. The test is in irqfd_deactivate() rather than in
irqfd_wakeup() because privcmd_irqfd_deassign() walks irqfds_list and can
therefore never reach an inactive irqfd; only the wakeup path can, and a
single test covers both callers.
Fixes: f8941e6c4c712 ("xen: privcmd: Add support for irqfd")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
drivers/xen/privcmd.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5e56fbc934478..221b31611343e 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -901,10 +901,25 @@ struct privcmd_kernel_irqfd {
poll_table pt;
};
+static bool irqfd_is_active(struct privcmd_kernel_irqfd *kirqfd)
+{
+ lockdep_assert_held(&irqfds_lock);
+
+ return !list_empty(&kirqfd->list);
+}
+
static void irqfd_deactivate(struct privcmd_kernel_irqfd *kirqfd)
{
lockdep_assert_held(&irqfds_lock);
+ /*
+ * Both the deassign ioctl and the eventfd wakeup path can reach
+ * this function. list_del_init() marks the irqfd inactive, so
+ * only the first caller may queue the shutdown work.
+ */
+ if (!irqfd_is_active(kirqfd))
+ return;
+
list_del_init(&kirqfd->list);
queue_work(irqfd_cleanup_wq, &kirqfd->shutdown);
}
--
2.54.0 (Apple Git-157)
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH] xen/privcmd: fix double free when an irqfd is shut down twice
@ 2026-11-01 16:31 Qihang
2026-11-01 16:31 ` Qihang
0 siblings, 1 reply; 3+ messages in thread
From: Qihang @ 2026-11-01 16:31 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Stefano Stabellini, Oleksandr Tyshchenko, Viresh Kumar,
linux-kernel, Qihang, stable
irqfd_deactivate() takes the irqfd off irqfds_list and queues
irqfd_shutdown(). queue_work() is not a once-only gate: it refuses to
enqueue a work item only while WORK_STRUCT_PENDING is set, and
process_one_work() clears that bit before it calls the work function. So
nothing records that the shutdown work has already been queued, and
nothing protects the window between the start of irqfd_shutdown() and the
kfree() at its end.
Two paths call irqfd_deactivate() for the same irqfd:
- privcmd_irqfd_deassign(), from the PRIVCMD_IRQFD_FLAG_DEASSIGN ioctl;
- irqfd_wakeup(), with EPOLLHUP set, when the last reference to the
eventfd file goes away, which runs wake_up_poll(&ctx->wqh, EPOLLHUP)
from eventfd_release().
irqfds_lock serialises the two calls, but it does not make the second one
a no-op, and eventfd_ctx_fdget() takes a reference to the eventfd_ctx,
not to the struct file, so the last fd can still be closed while the
ioctl is running:
CPU0 CPU1
eventfd_ctx_fdget()
spin_lock(irqfds_lock)
list_del_init(&kirqfd->list)
queue_work(&kirqfd->shutdown) close(fd)
eventfd_release()
wake_up_poll(EPOLLHUP)
irqfd_wakeup()
spin_lock(irqfds_lock)
irqfd_deactivate() again
If the work item has already started by then, its pending bit has been
cleared and the second queue_work() succeeds. irqfd_shutdown() then runs
twice, so eventfd_ctx_remove_wait_queue(), eventfd_ctx_put() and kfree()
each hit the same object a second time; and when the second queue_work()
lands after the irqfd has been freed, the workqueue list operations write
to freed memory.
virt/kvm/eventfd.c has the same two callers and has had this guard since
the original irqfd support (721eecbf4fe99): irqfd_wakeup() calls
irqfd_deactivate() only if irqfd_is_active(). The comment there explains
why that is enough: the other side deactivates the irqfd before it takes
it off the waitqueue, and irqfd_shutdown() needs the waitqueue lock,
which this callback holds, so the irqfd cannot be freed under it.
Do the same here. list_del_init() leaves the list head empty and the
irqfd stays off the list until kfree(), so it serves as a one-way
"active" flag. The test is in irqfd_deactivate() rather than in
irqfd_wakeup() because privcmd_irqfd_deassign() walks irqfds_list and can
therefore never reach an inactive irqfd; only the wakeup path can, and a
single test covers both callers.
Fixes: f8941e6c4c712 ("xen: privcmd: Add support for irqfd")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
drivers/xen/privcmd.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5e56fbc934478..221b31611343e 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -901,10 +901,25 @@ struct privcmd_kernel_irqfd {
poll_table pt;
};
+static bool irqfd_is_active(struct privcmd_kernel_irqfd *kirqfd)
+{
+ lockdep_assert_held(&irqfds_lock);
+
+ return !list_empty(&kirqfd->list);
+}
+
static void irqfd_deactivate(struct privcmd_kernel_irqfd *kirqfd)
{
lockdep_assert_held(&irqfds_lock);
+ /*
+ * Both the deassign ioctl and the eventfd wakeup path can reach
+ * this function. list_del_init() marks the irqfd inactive, so
+ * only the first caller may queue the shutdown work.
+ */
+ if (!irqfd_is_active(kirqfd))
+ return;
+
list_del_init(&kirqfd->list);
queue_work(irqfd_cleanup_wq, &kirqfd->shutdown);
}
--
2.54.0 (Apple Git-157)
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH] xen/privcmd: fix double free when an irqfd is shut down twice
2026-11-01 16:31 Qihang
@ 2026-11-01 16:31 ` Qihang
0 siblings, 0 replies; 3+ messages in thread
From: Qihang @ 2026-11-01 16:31 UTC (permalink / raw)
To: Juergen Gross, xen-devel
Cc: Stefano Stabellini, Oleksandr Tyshchenko, Viresh Kumar,
linux-kernel, Qihang, stable
irqfd_deactivate() takes the irqfd off irqfds_list and queues
irqfd_shutdown(). queue_work() is not a once-only gate: it refuses to
enqueue a work item only while WORK_STRUCT_PENDING is set, and
process_one_work() clears that bit before it calls the work function. So
nothing records that the shutdown work has already been queued, and
nothing protects the window between the start of irqfd_shutdown() and the
kfree() at its end.
Two paths call irqfd_deactivate() for the same irqfd:
- privcmd_irqfd_deassign(), from the PRIVCMD_IRQFD_FLAG_DEASSIGN ioctl;
- irqfd_wakeup(), with EPOLLHUP set, when the last reference to the
eventfd file goes away, which runs wake_up_poll(&ctx->wqh, EPOLLHUP)
from eventfd_release().
irqfds_lock serialises the two calls, but it does not make the second one
a no-op, and eventfd_ctx_fdget() takes a reference to the eventfd_ctx,
not to the struct file, so the last fd can still be closed while the
ioctl is running:
CPU0 CPU1
eventfd_ctx_fdget()
spin_lock(irqfds_lock)
list_del_init(&kirqfd->list)
queue_work(&kirqfd->shutdown) close(fd)
eventfd_release()
wake_up_poll(EPOLLHUP)
irqfd_wakeup()
spin_lock(irqfds_lock)
irqfd_deactivate() again
If the work item has already started by then, its pending bit has been
cleared and the second queue_work() succeeds. irqfd_shutdown() then runs
twice, so eventfd_ctx_remove_wait_queue(), eventfd_ctx_put() and kfree()
each hit the same object a second time; and when the second queue_work()
lands after the irqfd has been freed, the workqueue list operations write
to freed memory.
virt/kvm/eventfd.c has the same two callers and has had this guard since
the original irqfd support (721eecbf4fe99): irqfd_wakeup() calls
irqfd_deactivate() only if irqfd_is_active(). The comment there explains
why that is enough: the other side deactivates the irqfd before it takes
it off the waitqueue, and irqfd_shutdown() needs the waitqueue lock,
which this callback holds, so the irqfd cannot be freed under it.
Do the same here. list_del_init() leaves the list head empty and the
irqfd stays off the list until kfree(), so it serves as a one-way
"active" flag. The test is in irqfd_deactivate() rather than in
irqfd_wakeup() because privcmd_irqfd_deassign() walks irqfds_list and can
therefore never reach an inactive irqfd; only the wakeup path can, and a
single test covers both callers.
Fixes: f8941e6c4c712 ("xen: privcmd: Add support for irqfd")
Cc: stable@vger.kernel.org
Signed-off-by: Qihang <q.h.hack.winter@gmail.com>
---
drivers/xen/privcmd.c | 15 +++++++++++++++
1 file changed, 15 insertions(+)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index 5e56fbc934478..221b31611343e 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -901,10 +901,25 @@ struct privcmd_kernel_irqfd {
poll_table pt;
};
+static bool irqfd_is_active(struct privcmd_kernel_irqfd *kirqfd)
+{
+ lockdep_assert_held(&irqfds_lock);
+
+ return !list_empty(&kirqfd->list);
+}
+
static void irqfd_deactivate(struct privcmd_kernel_irqfd *kirqfd)
{
lockdep_assert_held(&irqfds_lock);
+ /*
+ * Both the deassign ioctl and the eventfd wakeup path can reach
+ * this function. list_del_init() marks the irqfd inactive, so
+ * only the first caller may queue the shutdown work.
+ */
+ if (!irqfd_is_active(kirqfd))
+ return;
+
list_del_init(&kirqfd->list);
queue_work(irqfd_cleanup_wq, &kirqfd->shutdown);
}
--
2.54.0 (Apple Git-157)
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-01 16:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-01 16:32 [PATCH] xen/privcmd: fix double free when an irqfd is shut down twice Qihang
-- strict thread matches above, loose matches on Subject: below --
2026-11-01 16:31 Qihang
2026-11-01 16:31 ` Qihang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®