[PATCH net v2] ptp: Report clock removal to poll waiters

[PATCH net v2] ptp: Report clock removal to poll waiters

[Avi Weiss]

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 <thnkslprpt@gmail.com>
---

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/
 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

Re: [PATCH net v2] ptp: Report clock removal to poll waiters

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.