Re: [PATCH v2 3/4] pps: generators: stop the generator on unregister

[Rodolfo Giometti <giometti@enneenne.com>]

On Tue, 29 Sep 2026 07:49:36 -0500, Danish Khateeb wrote:
> Both generator drivers stop their timer and then call
> pps_gen_unregister_source(): pps_gen_tio_remove() cancels its hrtimer
> and disables the TIO, and pps_gen_dummy_exit() deletes its timer. But
> /dev/pps-genN and the sysfs "enable" attribute are still there until
> the unregister, and a PPS_GEN_SETENABLE or a write to "enable" in
> between starts the timer again. Nothing stops it after that: TIO's
> hrtimer keeps running in the devm memory freed by the unbind, and the
> dummy's timer is left in the unloaded module.
>
> The drivers can't avoid this by unregistering first, as TIO's timer
> callback uses pps_gen, which the unregister may free.
>
> Stop the generator in pps_gen_unregister_cdev() instead, under
> info_lock after the device and its sysfs files are gone, when nothing
> can enable it again. pps_gen_unregister_source() then also does what
> its kernel-doc says: "it disables the generator so no pulses are
> generated anymore".
>
> Fixes: 86b525bed275 ("drivers pps: add PPS generators support")
> Cc: stable@vger.kernel.org
> Suggested-by: Rodolfo Giometti <giometti@enneenne.com>
> Assisted-by: LLM
> Signed-off-by: Danish Khateeb <danishkhateeb03@gmail.com>
> ---
>
> Notes:
>     Tested in the same setup, with patches 1/4 and 2/4 applied. The test
>     driver got a periodic hrtimer in its devm memory, like TIO's, and a
>     remove() that cancels it and then enables the generator again before
>     unregistering, as a PPS_GEN_SETENABLE landing there would. Without this
>     patch the timer keeps running after the unbind:
>     
>       BUG: KASAN: slab-use-after-free in rb_erase+0x174d/0x1a70
>        __remove_hrtimer+0x138/0x450
>        __hrtimer_run_queues+0x2c5/0x7f0
>        hrtimer_interrupt+0x3db/0x910
>       ...
>       Freed by task 175:
>        kfree+0x25a/0x6d0
>        release_nodes+0xd1/0x140
>        devres_release_all+0x10e/0x1a0
>        device_unbind_cleanup+0x71/0x250
>        device_release_driver_internal+0x41b/0x570
>        unbind_store+0xd9/0x100
>     
>     With it, unregister calls enable(false), the timer is stopped, and
>     there are no reports. Unbinding the test driver while enabled, and
>     unloading pps_gen-dummy while its file is open and enabled, are clean
>     too.
>
>  drivers/pps/generators/pps_gen.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/pps/generators/pps_gen.c b/drivers/pps/generators/pps_gen.c
> index d80e28dc31dc..452cc12a96f2 100644
> --- a/drivers/pps/generators/pps_gen.c
> +++ b/drivers/pps/generators/pps_gen.c
> @@ -228,9 +228,18 @@ static void pps_gen_unregister_cdev(struct pps_gen_device *pps_gen)
>  	 * An open file keeps pps_gen around, but the driver may free info as
>  	 * soon as we return. The sysfs files are gone now, so wait for the
>  	 * ioctls using info and make later ones fail.
> +	 *
> +	 * The driver may have stopped the generator before calling us, but
> +	 * userspace could have enabled it again since. Nothing can enable it
> +	 * after this point, so stop it here for good.
>  	 */
> -	scoped_guard(mutex, &pps_gen->info_lock)
> +	scoped_guard(mutex, &pps_gen->info_lock) {
> +		if (pps_gen->enabled) {
> +			pps_gen->info->enable(pps_gen, false);
> +			pps_gen->enabled = false;
> +		}
>  		pps_gen->info = NULL;
> +	}
>  
>  	put_device(&pps_gen->dev);
>  }

Acked-by: Rodolfo Giometti <giometti@enneenne.com>