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

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

[Calvin Owens]

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

[Rodolfo Giometti]

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

[Calvin Owens]

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

[Rodolfo Giometti]

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

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

[Calvin Owens]

On Monday 06/01 at 09:04 +0200, Rodolfo Giometti wrote:
> 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;

Thinking about this a little more... code in the kernel generally doesn't
try to enforce tv_nsec < NSEC_PER_SEC from userspace today.

I think nobody cares: let's just not worry about tv_nsec unless you
really think a real user will care.

If it's worth fixing, it ought to be done more systematically throughout
the kernel, I feel like adding the check in random places makes the
situation more confusing overall.

But again... I seriously doubt any real user cares. I'll look into the
history around this a bit more, but I'm very disinclined to think it's
worth the reviewer time to try and push it more broadly...

This KC code also depends on CONFIG_HZ_PERIODIC, which no distro ships,
so any user must necessarily be building their own kernel.

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

Ah that helper is nicer, thanks.

This plus the (tv_sec < 0) change should be sufficient, I'll send that
along soon unless you reply that you want the tv_nsec check too.

> 	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

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

[Rodolfo Giometti]

On 10/06/2026 17:48, Calvin Owens wrote:
> On Monday 06/01 at 09:04 +0200, Rodolfo Giometti wrote:
>> 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;
> 
> Thinking about this a little more... code in the kernel generally doesn't
> try to enforce tv_nsec < NSEC_PER_SEC from userspace today.
> 
> I think nobody cares: let's just not worry about tv_nsec unless you
> really think a real user will care.
> 
> If it's worth fixing, it ought to be done more systematically throughout
> the kernel, I feel like adding the check in random places makes the
> situation more confusing overall.
> 
> But again... I seriously doubt any real user cares. I'll look into the
> history around this a bit more, but I'm very disinclined to think it's
> worth the reviewer time to try and push it more broadly...
> 
> This KC code also depends on CONFIG_HZ_PERIODIC, which no distro ships,
> so any user must necessarily be building their own kernel.
> 
>>> 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);
> 
> Ah that helper is nicer, thanks.
> 
> This plus the (tv_sec < 0) change should be sufficient, I'll send that
> along soon unless you reply that you want the tv_nsec check too.

I agree with you.

>> 	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

Ciao,

Rodolfo

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