[PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH

Calvin Owens posted 1 patch 1 week, 2 days ago
drivers/pps/pps.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
[PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
Posted by Calvin Owens 1 week, 2 days ago
If userspace passes a negative timeout to PPS_FETCH, it triggers a
kernel splat from schedule_timeout():

    schedule_timeout: wrong timeout value fffffffffff0bfb4
    CPU: 17 UID: 0 PID: 4720 Comm: a.out Not tainted 7.1.0-rc5-x86-kvm-00150-g331d97e36b37 #1 PREEMPT_RT
    Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-20240910_120124-localhost 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x4b/0x70
     schedule_timeout+0xb7/0xe0
     pps_cdev_pps_fetch.isra.0+0x93/0x150
     pps_cdev_ioctl+0x70/0x310
     __x64_sys_ioctl+0x7b/0xc0
     do_syscall_64+0xb6/0xfc0
     entry_SYSCALL_64_after_hwframe+0x4b/0x53

Sashiko imagines this to be some sort of security problem, which is
obviously really silly. But I think it is still worth fixing, so buggy
userspace code can't trigger the splat.

Silence the splat by skipping the wait if the ticks count is negative.
The current behavior is to return -ETIMEDOUT in that case, so keep that
return value in case any userspace code might rely on it.

Fixes: eae9d2ba0cfc ("LinuxPPS: core support")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://sashiko.dev/#/patchset/cover.1779733602.git.calvin%40wbinvd.org?part=3
Signed-off-by: Calvin Owens <calvin@wbinvd.org>
---
 drivers/pps/pps.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
index de1122bb69ea..6755901fbdae 100644
--- a/drivers/pps/pps.c
+++ b/drivers/pps/pps.c
@@ -65,17 +65,19 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct pps_fdata *fdata)
 	if (fdata->timeout.flags & PPS_TIME_INVALID)
 		err = wait_event_interruptible(pps->queue,
 				ev != pps->last_ev);
 	else {
-		unsigned long ticks;
+		long ticks;
 
 		dev_dbg(&pps->dev, "timeout %lld.%09d\n",
 				(long long) fdata->timeout.sec,
 				fdata->timeout.nsec);
 		ticks = fdata->timeout.sec * HZ;
 		ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
 
-		if (ticks != 0) {
+		if (ticks < 0) {
+			return -ETIMEDOUT;
+		} else if (ticks > 0) {
 			err = wait_event_interruptible_timeout(
 					pps->queue,
 					ev != pps->last_ev,
 					ticks);
-- 
2.47.3
Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
Posted by Rodolfo Giometti 1 week, 1 day ago
On 29/05/2026 18:21, Calvin Owens wrote:
> If userspace passes a negative timeout to PPS_FETCH, it triggers a
> kernel splat from schedule_timeout():
> 
>      schedule_timeout: wrong timeout value fffffffffff0bfb4
>      CPU: 17 UID: 0 PID: 4720 Comm: a.out Not tainted 7.1.0-rc5-x86-kvm-00150-g331d97e36b37 #1 PREEMPT_RT
>      Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-20240910_120124-localhost 04/01/2014
>      Call Trace:
>       <TASK>
>       dump_stack_lvl+0x4b/0x70
>       schedule_timeout+0xb7/0xe0
>       pps_cdev_pps_fetch.isra.0+0x93/0x150
>       pps_cdev_ioctl+0x70/0x310
>       __x64_sys_ioctl+0x7b/0xc0
>       do_syscall_64+0xb6/0xfc0
>       entry_SYSCALL_64_after_hwframe+0x4b/0x53
> 
> Sashiko imagines this to be some sort of security problem, which is
> obviously really silly. But I think it is still worth fixing, so buggy
> userspace code can't trigger the splat.
> 
> Silence the splat by skipping the wait if the ticks count is negative.
> The current behavior is to return -ETIMEDOUT in that case, so keep that
> return value in case any userspace code might rely on it.
> 
> Fixes: eae9d2ba0cfc ("LinuxPPS: core support")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://sashiko.dev/#/patchset/cover.1779733602.git.calvin%40wbinvd.org?part=3
> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> ---
>   drivers/pps/pps.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
> index de1122bb69ea..6755901fbdae 100644
> --- a/drivers/pps/pps.c
> +++ b/drivers/pps/pps.c
> @@ -65,17 +65,19 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct pps_fdata *fdata)
>   	if (fdata->timeout.flags & PPS_TIME_INVALID)
>   		err = wait_event_interruptible(pps->queue,
>   				ev != pps->last_ev);
>   	else {
> -		unsigned long ticks;
> +		long ticks;
>   
>   		dev_dbg(&pps->dev, "timeout %lld.%09d\n",
>   				(long long) fdata->timeout.sec,
>   				fdata->timeout.nsec);
>   		ticks = fdata->timeout.sec * HZ;
>   		ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
>   
> -		if (ticks != 0) {
> +		if (ticks < 0) {
> +			return -ETIMEDOUT;
> +		} else if (ticks > 0) {
>   			err = wait_event_interruptible_timeout(
>   					pps->queue,
>   					ev != pps->last_ev,
>   					ticks);

Should the problem originate from user-space data, I deem it more appropriate to 
verify them directly, rather than relying on computed data.

	unsigned long ticks;

	if (fdata->timeout.sec < 0 || fdata->timeout.nsec < 0)
		return -ETIMEDOUT;

	dev_dbg(&pps->dev, "timeout %lld.%09d\n",
			(long long) fdata->timeout.sec,
			fdata->timeout.nsec);
		
	ticks = fdata->timeout.sec * HZ;
	ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);

	if (ticks > 0) {

Ciao,

Rodolfo

-- 
GNU/Linux Solutions                  e-mail: giometti@enneenne.com
Linux Device Driver                          giometti@linux.it
Embedded Systems                     phone:  +39 349 2432127
UNIX programming
Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
Posted by Calvin Owens 1 week, 1 day ago
On Saturday 05/30 at 11:50 +0200, Rodolfo Giometti wrote:
> On 29/05/2026 18:21, Calvin Owens wrote:
> > If userspace passes a negative timeout to PPS_FETCH, it triggers a
> > kernel splat from schedule_timeout():
> > 
> >      schedule_timeout: wrong timeout value fffffffffff0bfb4
> >      CPU: 17 UID: 0 PID: 4720 Comm: a.out Not tainted 7.1.0-rc5-x86-kvm-00150-g331d97e36b37 #1 PREEMPT_RT
> >      Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-20240910_120124-localhost 04/01/2014
> >      Call Trace:
> >       <TASK>
> >       dump_stack_lvl+0x4b/0x70
> >       schedule_timeout+0xb7/0xe0
> >       pps_cdev_pps_fetch.isra.0+0x93/0x150
> >       pps_cdev_ioctl+0x70/0x310
> >       __x64_sys_ioctl+0x7b/0xc0
> >       do_syscall_64+0xb6/0xfc0
> >       entry_SYSCALL_64_after_hwframe+0x4b/0x53
> > 
> > Sashiko imagines this to be some sort of security problem, which is
> > obviously really silly. But I think it is still worth fixing, so buggy
> > userspace code can't trigger the splat.
> > 
> > Silence the splat by skipping the wait if the ticks count is negative.
> > The current behavior is to return -ETIMEDOUT in that case, so keep that
> > return value in case any userspace code might rely on it.
> > 
> > Fixes: eae9d2ba0cfc ("LinuxPPS: core support")
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://sashiko.dev/#/patchset/cover.1779733602.git.calvin%40wbinvd.org?part=3
> > Signed-off-by: Calvin Owens <calvin@wbinvd.org>

Hi Rodolfo,

Thanks for taking a look at this and the others.

> > ---
> >   drivers/pps/pps.c | 6 ++++--
> >   1 file changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
> > index de1122bb69ea..6755901fbdae 100644
> > --- a/drivers/pps/pps.c
> > +++ b/drivers/pps/pps.c
> > @@ -65,17 +65,19 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct pps_fdata *fdata)
> >   	if (fdata->timeout.flags & PPS_TIME_INVALID)
> >   		err = wait_event_interruptible(pps->queue,
> >   				ev != pps->last_ev);
> >   	else {
> > -		unsigned long ticks;
> > +		long ticks;
> >   		dev_dbg(&pps->dev, "timeout %lld.%09d\n",
> >   				(long long) fdata->timeout.sec,
> >   				fdata->timeout.nsec);
> >   		ticks = fdata->timeout.sec * HZ;
> >   		ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
> > -		if (ticks != 0) {
> > +		if (ticks < 0) {
> > +			return -ETIMEDOUT;
> > +		} else if (ticks > 0) {
> >   			err = wait_event_interruptible_timeout(
> >   					pps->queue,
> >   					ev != pps->last_ev,
> >   					ticks);
> 
> Should the problem originate from user-space data, I deem it more
> appropriate to verify them directly, rather than relying on computed data.
> 
> 	unsigned long ticks;
> 
> 	if (fdata->timeout.sec < 0 || fdata->timeout.nsec < 0)
> 		return -ETIMEDOUT;

I agree your way is nicer to read.

Pedantically, it's a user visable behavior change: today, the user
can pass absurd values for sec and nsec, and everything works so long as
the math works out to positive ticks (e.g. sec=ULONG_MAX/HZ,
nsec=2*HZ*NSEC_PER_SEC).

If it was just that, it wouldn't really matter IMO, but...

> 	dev_dbg(&pps->dev, "timeout %lld.%09d\n",
> 			(long long) fdata->timeout.sec,
> 			fdata->timeout.nsec);
> 		
> 	ticks = fdata->timeout.sec * HZ;

...the multiplication by HZ could also overflow even if sec is positive,
so I think userspace would still be able to trigger the splat this way.

> 	ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
> 
> 	if (ticks > 0) {
> 
> Ciao,
> 
> Rodolfo
> 
> -- 
> GNU/Linux Solutions                  e-mail: giometti@enneenne.com
> Linux Device Driver                          giometti@linux.it
> Embedded Systems                     phone:  +39 349 2432127
> UNIX programming
Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
Posted by Rodolfo Giometti 1 week ago
On 30/05/2026 16:54, Calvin Owens wrote:
> On Saturday 05/30 at 11:50 +0200, Rodolfo Giometti wrote:
>> On 29/05/2026 18:21, Calvin Owens wrote:
>>> If userspace passes a negative timeout to PPS_FETCH, it triggers a
>>> kernel splat from schedule_timeout():
>>>
>>>       schedule_timeout: wrong timeout value fffffffffff0bfb4
>>>       CPU: 17 UID: 0 PID: 4720 Comm: a.out Not tainted 7.1.0-rc5-x86-kvm-00150-g331d97e36b37 #1 PREEMPT_RT
>>>       Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.16.3-20240910_120124-localhost 04/01/2014
>>>       Call Trace:
>>>        <TASK>
>>>        dump_stack_lvl+0x4b/0x70
>>>        schedule_timeout+0xb7/0xe0
>>>        pps_cdev_pps_fetch.isra.0+0x93/0x150
>>>        pps_cdev_ioctl+0x70/0x310
>>>        __x64_sys_ioctl+0x7b/0xc0
>>>        do_syscall_64+0xb6/0xfc0
>>>        entry_SYSCALL_64_after_hwframe+0x4b/0x53
>>>
>>> Sashiko imagines this to be some sort of security problem, which is
>>> obviously really silly. But I think it is still worth fixing, so buggy
>>> userspace code can't trigger the splat.
>>>
>>> Silence the splat by skipping the wait if the ticks count is negative.
>>> The current behavior is to return -ETIMEDOUT in that case, so keep that
>>> return value in case any userspace code might rely on it.
>>>
>>> Fixes: eae9d2ba0cfc ("LinuxPPS: core support")
>>> Reported-by: Sashiko <sashiko-bot@kernel.org>
>>> Closes: https://sashiko.dev/#/patchset/cover.1779733602.git.calvin%40wbinvd.org?part=3
>>> Signed-off-by: Calvin Owens <calvin@wbinvd.org>
> 
> Hi Rodolfo,
> 
> Thanks for taking a look at this and the others.
> 
>>> ---
>>>    drivers/pps/pps.c | 6 ++++--
>>>    1 file changed, 4 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/pps/pps.c b/drivers/pps/pps.c
>>> index de1122bb69ea..6755901fbdae 100644
>>> --- a/drivers/pps/pps.c
>>> +++ b/drivers/pps/pps.c
>>> @@ -65,17 +65,19 @@ static int pps_cdev_pps_fetch(struct pps_device *pps, struct pps_fdata *fdata)
>>>    	if (fdata->timeout.flags & PPS_TIME_INVALID)
>>>    		err = wait_event_interruptible(pps->queue,
>>>    				ev != pps->last_ev);
>>>    	else {
>>> -		unsigned long ticks;
>>> +		long ticks;
>>>    		dev_dbg(&pps->dev, "timeout %lld.%09d\n",
>>>    				(long long) fdata->timeout.sec,
>>>    				fdata->timeout.nsec);
>>>    		ticks = fdata->timeout.sec * HZ;
>>>    		ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
>>> -		if (ticks != 0) {
>>> +		if (ticks < 0) {
>>> +			return -ETIMEDOUT;
>>> +		} else if (ticks > 0) {
>>>    			err = wait_event_interruptible_timeout(
>>>    					pps->queue,
>>>    					ev != pps->last_ev,
>>>    					ticks);
>>
>> Should the problem originate from user-space data, I deem it more
>> appropriate to verify them directly, rather than relying on computed data.
>>
>> 	unsigned long ticks;
>>
>> 	if (fdata->timeout.sec < 0 || fdata->timeout.nsec < 0)
>> 		return -ETIMEDOUT;
> 
> I agree your way is nicer to read.

We can also do as follow:

	/* Canonical validation of nanoseconds */
	if (fdata->timeout.nsec < 0 || fdata->timeout.nsec >= NSEC_PER_SEC)
		return -EINVAL;

	/* Preserve historical API behavior for negative seconds */
	if (fdata->timeout.sec < 0)
		return -ETIMEDOUT;

> Pedantically, it's a user visable behavior change: today, the user
> can pass absurd values for sec and nsec, and everything works so long as
> the math works out to positive ticks (e.g. sec=ULONG_MAX/HZ,
> nsec=2*HZ*NSEC_PER_SEC).
> 
> If it was just that, it wouldn't really matter IMO, but...
> 
>> 	dev_dbg(&pps->dev, "timeout %lld.%09d\n",
>> 			(long long) fdata->timeout.sec,
>> 			fdata->timeout.nsec);
>> 		
>> 	ticks = fdata->timeout.sec * HZ;
> 
> ...the multiplication by HZ could also overflow even if sec is positive,
> so I think userspace would still be able to trigger the splat this way.
> 
>> 	ticks += fdata->timeout.nsec / (NSEC_PER_SEC / HZ);
>>
>> 	if (ticks > 0) {

We can do as follow:

	/* Safe conversion using standard kernel API */
	ts.tv_sec = fdata->timeout.sec;
	ts.tv_nsec = fdata->timeout.nsec;
	ticks = timespec64_to_jiffies(&ts);

	if (ticks > 0) {
		err = wait_event_interruptible_timeout(
				pps->queue,
				ev != pps->last_ev,
				ticks);
	}

Ciao,

Rodolfo

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