[PATCH] posix-clock: bind the device before the cdev is published

[fangsz]

posix_clock_register() adds the character device and only afterwards
stores the device pointer:

	cdev_init(&clk->cdev, &posix_clock_file_operations);
	err = cdev_device_add(&clk->cdev, dev);
	if (err)
		...
	clk->dev = dev;

device_add() publishes the /dev/ptpN node through devtmpfs_create_node()
before it returns, so a concurrent open() can reach posix_clock_open()
while clk->dev is still NULL. get_device(NULL) is a no-op, so such a file
does not hold the reference it is supposed to hold, while
posix_clock_release() unconditionally calls put_device(clk->dev) and thus
drops a reference that was never taken.

That stray put frees the parent device early. When the character device
is released later, cdev_default_release() calls kobject_put() on the
already freed device and the refcount saturates:

  refcount_t: underflow; use-after-free.
  WARNING: CPU: 6 PID: 4270 at lib/refcount.c:28 refcount_warn_saturate+0xec/0x140
  ...
  Call trace:
   refcount_warn_saturate+0xec/0x140
   kobject_put+0x160/0x208
   cdev_default_release+0x78/0x90
   kobject_put+0xa8/0x208
   cdev_put+0x20/0x40
   __fput+0x2cc/0x2e8
   __arm64_sys_close+0x3c/0x88

The window is small, but it is hit in practice once a driver registers
and unregisters the PTP clock repeatedly, for example on every link up
and down, while ptp4l keeps reopening the node:

  [   50.048138] eth0: registered PTP clock
  [   50.767426] ptp4l: refcount_t: underflow; use-after-free.
  [   55.735110] eth0: Link is Down
  [   55.816736] eth0: registered PTP clock

The ordering was introduced by a33121e5487b, which added the
get_device()/put_device() pair around clk->dev but left the assignment
at the end of posix_clock_register(). Bind clk->dev before the character
device is published and clear it again when cdev_device_add() fails, so
that an open() racing with the registration always finds a valid device
to take a reference on.

Fixes: a33121e5487b ("ptp: fix the race between the release of ptp_clock and cdev")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: fangsz <315539811@xxxxxx>
---
 kernel/time/posix-clock.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/kernel/time/posix-clock.c b/kernel/time/posix-clock.c
index dab3729..1c61382 100644
--- a/kernel/time/posix-clock.c
+++ b/kernel/time/posix-clock.c
@@ -163,15 +163,23 @@ int posix_clock_register(struct posix_clock *clk, struct device *dev)
 
 	init_rwsem(&clk->rwsem);
 
+	/*
+	 * cdev_device_add() publishes the device node before it returns, so
+	 * clk->dev must be set up front.  Otherwise an open() racing with the
+	 * registration skips the get_device() in posix_clock_open() while
+	 * posix_clock_release() still drops a reference for it.
+	 */
+	clk->dev = dev;
+
 	cdev_init(&clk->cdev, &posix_clock_file_operations);
 	err = cdev_device_add(&clk->cdev, dev);
 	if (err) {
+		clk->dev = NULL;
 		pr_err("%s unable to add device %d:%d\n",
 			dev_name(dev), MAJOR(dev->devt), MINOR(dev->devt));
 		return err;
 	}
 	clk->cdev.owner = clk->ops.owner;
-	clk->dev = dev;
 
 	return 0;
 }