ptp_clock_unregister() marks the clock defunct and wakes timestamp
waiters before posix_clock_unregister() marks it a zombie. A poll
waiter can consume the removal wakeup while the event queue is empty,
return no events, and remain blocked until timeout after device
removal.
Using netdevsim's mock PTP clock in an x86_64 QEMU/KVM guest,
the unpatched kernel missed all eight epoll removal notifications.
The four poll waits returned POLLERR only at timeout. With this
patch, all 12 poll and epoll waits reported clock removal promptly.
Check the defunct state after registering the poll wait queue and
return EPOLLERR when the clock is being removed. Use READ_ONCE() for
the lockless defunct checks and WRITE_ONCE() for the unregister store.
Fixes: d94ba80ebbea ("ptp: Added a brand new class driver for ptp clocks.")
Signed-off-by: Avi Weiss <redacted>
---
v2:
- ddress Sashiko's v1 review, forwarded by Simon Horman, by pairing
the new READ_ONCE() with WRITE_ONCE() in ptp_clock_unregister() and
using READ_ONCE() for both ptp_read() checks
- the v1 poll fix already addressed the observed removal scenario,
these changes are really just to make the lockless accesses consistent
v1: https://lore.kernel.org/r/20261102122340.1160733-1-thnkslprpt@gmail.com/ (local)
drivers/ptp/ptp_chardev.c | 8 ++++++--
drivers/ptp/ptp_clock.c | 2 +-
2 files changed, 7 insertions(+), 3 deletions(-)
diff --git a/drivers/ptp/ptp_chardev.c b/drivers/ptp/ptp_chardev.c
index f63b45f5ee6a..dc5865e95f77 100644
--- a/drivers/ptp/ptp_chardev.c
+++ b/drivers/ptp/ptp_chardev.c
@@ -593,6 +593,9 @@ __poll_t ptp_poll(struct posix_clock_context *pccontext, struct file *fp,
poll_wait(fp, &ptp->tsev_wq, wait);
+ if (READ_ONCE(ptp->defunct))
+ return EPOLLERR;
+
return queue_cnt(queue) ? EPOLLIN : 0;
}
@@ -616,10 +619,11 @@ ssize_t ptp_read(struct posix_clock_context *pccontext, uint rdflags,
if (cnt > EXTTS_BUFSIZE)
cnt = EXTTS_BUFSIZE;
- if (wait_event_interruptible(ptp->tsev_wq, ptp->defunct || queue_cnt(queue)))
+ if (wait_event_interruptible(ptp->tsev_wq,
+ READ_ONCE(ptp->defunct) || queue_cnt(queue)))
return -ERESTARTSYS;
- if (ptp->defunct)
+ if (READ_ONCE(ptp->defunct))
return -ENODEV;
event = kmalloc(EXTTS_BUFSIZE, GFP_KERNEL);
diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
index 4111342d64f0..d2640325abf7 100644
--- a/drivers/ptp/ptp_clock.c
+++ b/drivers/ptp/ptp_clock.c
@@ -518,7 +518,7 @@ int ptp_clock_unregister(struct ptp_clock *ptp)
get_device(&ptp->dev);
/* Wake up any userspace waiting for an event. */
- ptp->defunct = 1;
+ WRITE_ONCE(ptp->defunct, 1);
wake_up_interruptible(&ptp->tsev_wq);
/* Tear down the POSIX clock, which removes the user interface. */
--
2.43.0