Re: [PATCH 2/2] pps: generators: don't use the driver's info after unregister

[Rodolfo Giometti <giometti@enneenne.com>]

On Mon, 28 Sep 2026 21:22:19 -0500, Danish Khateeb wrote:
> @@ -212,6 +223,15 @@ static void pps_gen_unregister_cdev(struct pps_gen_device *pps_gen)
>  {
>  	pr_debug("unregistering pps-gen%d\n", pps_gen->id);
>  	cdev_device_del(&pps_gen->cdev, &pps_gen->dev);
> +
> +	/*
> +	 * 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.
> +	 */
> +	scoped_guard(mutex, &pps_gen->info_lock)
> +		pps_gen->info = NULL;
> +
>  	put_device(&pps_gen->dev);
>  }

This closes the window after the unregister, but what about the one
just before it? The two generators in the tree stop their timer before
calling pps_gen_unregister_source(): pps_gen_tio_remove() does
hrtimer_cancel() and pps_tio_disable(), pps_gen_dummy_exit() does
timer_delete_sync(). At that point the file and the sysfs "enable"
attribute are still there, so AFAICS a PPS_GEN_SETENABLE landing in
between re-arms the timer, and nothing stops it again before tio is
freed (or the dummy module goes away).

I don't think the drivers can fix this on their own: if they unregister
first, the TIO timer may call pps_gen_event() on a pps_gen that is
already gone. So I suspect the core has to disable the generator itself
on unregister, once the file and the sysfs are gone and before clearing
info. On top of this patch, something like (untested):

--- a/drivers/pps/generators/pps_gen.c
+++ b/drivers/pps/generators/pps_gen.c
@@ static void pps_gen_unregister_cdev(struct pps_gen_device *pps_gen)
-	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;
+	}

Could you add that as its own patch in this series, and test it with
your unbind/rmmod setup? As a bonus, pps_gen_unregister_source() would
then do what its kernel-doc promises.

Also, a reader sleeping in PPS_GEN_FETCHEVENT is never woken up by the
unregister: after 1/2 it no longer touches freed memory, but it now
sleeps until a signal arrives. Worth a wake-up and an -ENODEV while you
are there?

1/2 looks good to me; I'll ack the whole series once this is sorted
out.

Ciao,

Rodolfo