* [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
@ 2026-05-29 16:21 Calvin Owens
2026-05-30 9:50 ` Rodolfo Giometti
0 siblings, 1 reply; 6+ messages in thread
From: Calvin Owens @ 2026-05-29 16:21 UTC (permalink / raw)
To: linux-kernel
Cc: linux-rt-devel, Rodolfo Giometti, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Andrew Morton
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
2026-05-29 16:21 [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH Calvin Owens
@ 2026-05-30 9:50 ` Rodolfo Giometti
2026-05-30 14:54 ` Calvin Owens
0 siblings, 1 reply; 6+ messages in thread
From: Rodolfo Giometti @ 2026-05-30 9:50 UTC (permalink / raw)
To: Calvin Owens, linux-kernel
Cc: linux-rt-devel, Sebastian Andrzej Siewior, Clark Williams,
Steven Rostedt, Andrew Morton
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
2026-05-30 9:50 ` Rodolfo Giometti
@ 2026-05-30 14:54 ` Calvin Owens
2026-06-01 7:04 ` Rodolfo Giometti
0 siblings, 1 reply; 6+ messages in thread
From: Calvin Owens @ 2026-05-30 14:54 UTC (permalink / raw)
To: Rodolfo Giometti
Cc: linux-kernel, linux-rt-devel, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Andrew Morton
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
2026-05-30 14:54 ` Calvin Owens
@ 2026-06-01 7:04 ` Rodolfo Giometti
2026-06-10 15:48 ` Calvin Owens
0 siblings, 1 reply; 6+ messages in thread
From: Rodolfo Giometti @ 2026-06-01 7:04 UTC (permalink / raw)
To: Calvin Owens
Cc: linux-kernel, linux-rt-devel, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Andrew Morton
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
2026-06-01 7:04 ` Rodolfo Giometti
@ 2026-06-10 15:48 ` Calvin Owens
2026-06-11 11:09 ` Rodolfo Giometti
0 siblings, 1 reply; 6+ messages in thread
From: Calvin Owens @ 2026-06-10 15:48 UTC (permalink / raw)
To: Rodolfo Giometti
Cc: linux-kernel, linux-rt-devel, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Andrew Morton
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH
2026-06-10 15:48 ` Calvin Owens
@ 2026-06-11 11:09 ` Rodolfo Giometti
0 siblings, 0 replies; 6+ messages in thread
From: Rodolfo Giometti @ 2026-06-11 11:09 UTC (permalink / raw)
To: Calvin Owens
Cc: linux-kernel, linux-rt-devel, Sebastian Andrzej Siewior,
Clark Williams, Steven Rostedt, Andrew Morton
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-06-11 11:12 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-05-29 16:21 [PATCH] pps: Don't try to wait for negative timeouts in PPS_FETCH Calvin Owens
2026-05-30 9:50 ` Rodolfo Giometti
2026-05-30 14:54 ` Calvin Owens
2026-06-01 7:04 ` Rodolfo Giometti
2026-06-10 15:48 ` Calvin Owens
2026-06-11 11:09 ` Rodolfo Giometti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®