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

[Frederic Weisbecker]

Le Tue, Oct 27, 2026 at 01:18:32AM +0800, fangsz a écrit :
> 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>

Reviewed-by: Frederic Weisbecker <frederic@xxxxxxxxxx>

-- 
Frederic Weisbecker
SUSE Labs