Re: [PATCH net v3 1/5] ptp: drain virtual clock sysfs operations before unregistering children

[Vadim Fedorenko <vadim.fedorenko@linux.dev>]

On 13/10/2026 20:19, James Hilliard wrote:
> Parent clock removal walks its virtual clocks before removing the
> n_vclocks sysfs attribute. The mutex taken by ptp_vclock_in_use() is
> released before that walk, so a concurrent sysfs deletion can pick the
> same child and unregister and free its ptp_vclock a second time. A
> reference held by the device iterator does not protect that separately
> allocated virtual clock.
> 
> Remove n_vclocks before walking the children. Removing the attribute
> prevents new stores and drains stores already running, without holding
> n_vclocks_mux across a callback that needs that mutex. Skip virtual
> clocks, which do not have this attribute.
> 
> This race was found by code inspection of virtual-clock registration
> failure cleanup and parent removal.
> 
> Fixes: bfcbb76b0f59 ("ptp: unregister virtual clocks when unregistering physical clock.")
> Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
> ---
>   drivers/ptp/ptp_clock.c   | 5 +++++
>   drivers/ptp/ptp_private.h | 1 +
>   drivers/ptp/ptp_sysfs.c   | 6 ++++++
>   3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/ptp/ptp_clock.c b/drivers/ptp/ptp_clock.c
> index 4111342d64f0..47ffc773065a 100644
> --- a/drivers/ptp/ptp_clock.c
> +++ b/drivers/ptp/ptp_clock.c
> @@ -508,6 +508,11 @@ static int unregister_vclock(struct device *dev, void *data)
>   
>   int ptp_clock_unregister(struct ptp_clock *ptp)
>   {
> +	/*
> +	 * Stop and drain virtual-clock creation and deletion before walking the
> +	 * children. Do not hold n_vclocks_mux while waiting for sysfs callbacks.
> +	 */
> +	ptp_vclock_remove_sysfs(ptp);
>   	if (ptp_vclock_in_use(ptp)) {
>   		device_for_each_child(&ptp->dev, NULL, unregister_vclock);
>   	}
> diff --git a/drivers/ptp/ptp_private.h b/drivers/ptp/ptp_private.h
> index db4039d642b4..ec8633126d6b 100644
> --- a/drivers/ptp/ptp_private.h
> +++ b/drivers/ptp/ptp_private.h
> @@ -169,6 +169,7 @@ extern const struct attribute_group *ptp_groups[];
>   
>   int ptp_populate_pin_groups(struct ptp_clock *ptp);
>   void ptp_cleanup_pin_groups(struct ptp_clock *ptp);
> +void ptp_vclock_remove_sysfs(struct ptp_clock *ptp);
>   
>   struct ptp_vclock *ptp_vclock_register(struct ptp_clock *pclock);
>   void ptp_vclock_unregister(struct ptp_vclock *vclock);
> diff --git a/drivers/ptp/ptp_sysfs.c b/drivers/ptp/ptp_sysfs.c
> index dc398c6b7528..53388b123198 100644
> --- a/drivers/ptp/ptp_sysfs.c
> +++ b/drivers/ptp/ptp_sysfs.c
> @@ -263,6 +263,12 @@ static ssize_t n_vclocks_store(struct device *dev,
>   }
>   static DEVICE_ATTR_RW(n_vclocks);
>   
> +void ptp_vclock_remove_sysfs(struct ptp_clock *ptp)
> +{
> +	if (!ptp->is_virtual_clock)
> +		device_remove_file(&ptp->dev, &dev_attr_n_vclocks);
> +}
> +
>   static ssize_t max_vclocks_show(struct device *dev,
>   				struct device_attribute *attr, char *page)
>   {
> 

Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>