[PATCH v2] pps: Don't allow PPS_KC_BIND on removed devices

Calvin Owens posted 1 patch 4 days, 14 hours ago
drivers/pps/kc.c           | 10 ++++++++++
include/linux/pps_kernel.h |  1 +
2 files changed, 11 insertions(+)
[PATCH v2] pps: Don't allow PPS_KC_BIND on removed devices
Posted by Calvin Owens 4 days, 14 hours ago
If userspace holds its file descriptor open, it can call PPS_KC_BIND on
a device which has been unplugged, leaving pps_kc_hardpps_dev as a
dangling pointer after close().

After that sequence, PPS_KC_BIND is broken until the system is rebooted,
because the pointer comparison in pps_kc_bind() can never be true.

    calling  pps_ktimer_init+0x0/0x1000 [pps_ktimer] @ 1081
    initcall pps_ktimer_init+0x0/0x1000 [pps_ktimer] returned 0 after 811 usecs
    pps pps0: bound kernel consumer: edge=0x1
    pps pps0: unbound kernel consumer on device removal
    pps pps0: bound kernel consumer: edge=0x1
    calling  pps_ktimer_init+0x0/0x1000 [pps_ktimer] @ 1085
    initcall pps_ktimer_init+0x0/0x1000 [pps_ktimer] returned 0 after 340 usecs
    pps pps0: another kernel consumer is already bound

Here is a short reproducer, which uses rmmod of the pps-ktimer testcase
to simulate a device being unplugged:

    #include <stdlib.h>
    #include <stdio.h>
    #include <unistd.h>
    #include <fcntl.h>
    #include <sys/ioctl.h>
    #include <linux/pps.h>
    #include <errno.h>
    #include <err.h>

    int main(void)
    {
        while (1) {
            int fd;

            if (system("insmod ./pps-ktimer.ko"))
                err(1, "insmod failed");

            fd = open("/dev/pps0", O_RDWR);
            if (fd == -1)
                err(1, "open failed");

            struct pps_bind_args args = {
                .tsformat = PPS_TSFMT_TSPEC,
                .edge = PPS_CAPTUREASSERT,
                .consumer = PPS_KC_HARDPPS,
            };

            if (ioctl(fd, PPS_KC_BIND, &args))
                err(1, "first PPS_KC_BIND failed");

            if (system("rmmod pps-ktimer"))
                err(1, "rmmod failed");

            if (ioctl(fd, PPS_KC_BIND, &args)) {
                if (errno != ENODEV)
                    err(1, "second PPS_KC_BIND failed");
                else
                    puts("Got ENODEV, kernel is patched");
            }

            close(fd);
        }
    }

Fix this by setting a flag when the device is unplugged, returning
-ENODEV from PPS_KC_BIND if the flag is set.

For userspace to encounter this new behavior, it must do something which
breaks the interface today, so this fix shouldn't cause any observable
behavior change for working programs.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/cover.1779733602.git.calvin%40wbinvd.org?part=1
Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
Changes in v2:
* Drop guard() changes so irqs are unmasked for printk() calls [Rodolfo]

v1: https://lore.kernel.org/lkml/c9bfee54cbabbbfc8da7aa3d3893cf61d3ebf3b2.1780108479.git.calvin@wbinvd.org/

 drivers/pps/kc.c           | 10 ++++++++++
 include/linux/pps_kernel.h |  1 +
 2 files changed, 11 insertions(+)

diff --git a/drivers/pps/kc.c b/drivers/pps/kc.c
index fbd23295afd7..4f8fffa7edd6 100644
--- a/drivers/pps/kc.c
+++ b/drivers/pps/kc.c
@@ -38,6 +38,14 @@ int pps_kc_bind(struct pps_device *pps, struct pps_bind_args *bind_args)
 	/* Check if another consumer is already bound */
 	spin_lock_irq(&pps_kc_hardpps_lock);
 
+	/*
+	 * Don't allow PPS_KC_BIND on a removed device.
+	 */
+	if (pps->kc_removed) {
+		spin_unlock_irq(&pps_kc_hardpps_lock);
+		return -ENODEV;
+	}
+
 	if (bind_args->edge == 0)
 		if (pps_kc_hardpps_dev == pps) {
 			pps_kc_hardpps_mode = 0;
@@ -79,6 +87,8 @@ int pps_kc_bind(struct pps_device *pps, struct pps_bind_args *bind_args)
 void pps_kc_remove(struct pps_device *pps)
 {
 	spin_lock_irq(&pps_kc_hardpps_lock);
+
+	pps->kc_removed = true;
 	if (pps == pps_kc_hardpps_dev) {
 		pps_kc_hardpps_mode = 0;
 		pps_kc_hardpps_dev = NULL;
diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
index aab0aebb529e..3c2979f8a5ac 100644
--- a/include/linux/pps_kernel.h
+++ b/include/linux/pps_kernel.h
@@ -60,6 +60,7 @@ struct pps_device {
 	struct device dev;
 	struct fasync_struct *async_queue;	/* fasync method */
 	spinlock_t lock;
+	bool kc_removed;
 };
 
 /*
-- 
2.47.3
Re: [PATCH v2] pps: Don't allow PPS_KC_BIND on removed devices
Posted by Rodolfo Giometti 3 days, 23 hours ago
On 03/06/2026 19:25, Calvin Owens wrote:
> If userspace holds its file descriptor open, it can call PPS_KC_BIND on
> a device which has been unplugged, leaving pps_kc_hardpps_dev as a
> dangling pointer after close().
> 
> After that sequence, PPS_KC_BIND is broken until the system is rebooted,
> because the pointer comparison in pps_kc_bind() can never be true.
> 
>      calling  pps_ktimer_init+0x0/0x1000 [pps_ktimer] @ 1081
>      initcall pps_ktimer_init+0x0/0x1000 [pps_ktimer] returned 0 after 811 usecs
>      pps pps0: bound kernel consumer: edge=0x1
>      pps pps0: unbound kernel consumer on device removal
>      pps pps0: bound kernel consumer: edge=0x1
>      calling  pps_ktimer_init+0x0/0x1000 [pps_ktimer] @ 1085
>      initcall pps_ktimer_init+0x0/0x1000 [pps_ktimer] returned 0 after 340 usecs
>      pps pps0: another kernel consumer is already bound
> 
> Here is a short reproducer, which uses rmmod of the pps-ktimer testcase
> to simulate a device being unplugged:
> 
>      #include <stdlib.h>
>      #include <stdio.h>
>      #include <unistd.h>
>      #include <fcntl.h>
>      #include <sys/ioctl.h>
>      #include <linux/pps.h>
>      #include <errno.h>
>      #include <err.h>
> 
>      int main(void)
>      {
>          while (1) {
>              int fd;
> 
>              if (system("insmod ./pps-ktimer.ko"))
>                  err(1, "insmod failed");
> 
>              fd = open("/dev/pps0", O_RDWR);
>              if (fd == -1)
>                  err(1, "open failed");
> 
>              struct pps_bind_args args = {
>                  .tsformat = PPS_TSFMT_TSPEC,
>                  .edge = PPS_CAPTUREASSERT,
>                  .consumer = PPS_KC_HARDPPS,
>              };
> 
>              if (ioctl(fd, PPS_KC_BIND, &args))
>                  err(1, "first PPS_KC_BIND failed");
> 
>              if (system("rmmod pps-ktimer"))
>                  err(1, "rmmod failed");
> 
>              if (ioctl(fd, PPS_KC_BIND, &args)) {
>                  if (errno != ENODEV)
>                      err(1, "second PPS_KC_BIND failed");
>                  else
>                      puts("Got ENODEV, kernel is patched");
>              }
> 
>              close(fd);
>          }
>      }
> 
> Fix this by setting a flag when the device is unplugged, returning
> -ENODEV from PPS_KC_BIND if the flag is set.
> 
> For userspace to encounter this new behavior, it must do something which
> breaks the interface today, so this fix shouldn't cause any observable
> behavior change for working programs.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/cover.1779733602.git.calvin%40wbinvd.org?part=1
> Signed-off-by: Calvin Owens <calvin@wbinvd.org>

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

> ---
> Changes in v2:
> * Drop guard() changes so irqs are unmasked for printk() calls [Rodolfo]
> 
> v1: https://lore.kernel.org/lkml/c9bfee54cbabbbfc8da7aa3d3893cf61d3ebf3b2.1780108479.git.calvin@wbinvd.org/
> 
>   drivers/pps/kc.c           | 10 ++++++++++
>   include/linux/pps_kernel.h |  1 +
>   2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/pps/kc.c b/drivers/pps/kc.c
> index fbd23295afd7..4f8fffa7edd6 100644
> --- a/drivers/pps/kc.c
> +++ b/drivers/pps/kc.c
> @@ -38,6 +38,14 @@ int pps_kc_bind(struct pps_device *pps, struct pps_bind_args *bind_args)
>   	/* Check if another consumer is already bound */
>   	spin_lock_irq(&pps_kc_hardpps_lock);
>   
> +	/*
> +	 * Don't allow PPS_KC_BIND on a removed device.
> +	 */
> +	if (pps->kc_removed) {
> +		spin_unlock_irq(&pps_kc_hardpps_lock);
> +		return -ENODEV;
> +	}
> +
>   	if (bind_args->edge == 0)
>   		if (pps_kc_hardpps_dev == pps) {
>   			pps_kc_hardpps_mode = 0;
> @@ -79,6 +87,8 @@ int pps_kc_bind(struct pps_device *pps, struct pps_bind_args *bind_args)
>   void pps_kc_remove(struct pps_device *pps)
>   {
>   	spin_lock_irq(&pps_kc_hardpps_lock);
> +
> +	pps->kc_removed = true;
>   	if (pps == pps_kc_hardpps_dev) {
>   		pps_kc_hardpps_mode = 0;
>   		pps_kc_hardpps_dev = NULL;
> diff --git a/include/linux/pps_kernel.h b/include/linux/pps_kernel.h
> index aab0aebb529e..3c2979f8a5ac 100644
> --- a/include/linux/pps_kernel.h
> +++ b/include/linux/pps_kernel.h
> @@ -60,6 +60,7 @@ struct pps_device {
>   	struct device dev;
>   	struct fasync_struct *async_queue;	/* fasync method */
>   	spinlock_t lock;
> +	bool kc_removed;
>   };
>   
>   /*


-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming