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

[Danish Khateeb]

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@xxxxxxxxxxxxxxx
Suggested-by: Rodolfo Giometti <giometti@xxxxxxxxxxxx>
Assisted-by: LLM
Signed-off-by: Danish Khateeb <danishkhateeb03@xxxxxxxxx>
---

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);
 }
-- 
2.55.0