Page MenuHomeFreeBSD
Differential D51744

time: Handle kern_clock_gettime() failures in kern_clock_nanosleep()
ClosedPublic
Actions

Authored by markj on Tue, Aug 5, 4:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Aug 17, 1:00 PM
Unknown Object (File)
Sat, Aug 16, 7:08 AM
Unknown Object (File)
Tue, Aug 12, 1:50 AM
Unknown Object (File)
Tue, Aug 12, 12:06 AM
Unknown Object (File)
Sun, Aug 10, 10:44 AM
Unknown Object (File)
Sat, Aug 9, 7:28 PM
Unknown Object (File)
Fri, Aug 8, 5:29 PM
Unknown Object (File)
Wed, Aug 6, 11:01 PM
Subscribers
olce

Details

Reviewers
kib
imp
nwhitehorn
Commits
rG7556b55f0d67: time: Handle kern_clock_gettime() failures in kern_clock_nanosleep()
Summary

kern_clock_gettime(CLOCK_TAI) can fail with EINVAL.

Reported by: syzbot+e17e46b1f0b65027b005@syzkaller.appspotmail.com
Fixes: 7b7ba7857ce8 ("Implement CLOCK_TAI")

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Skipped
Unit
Tests Skipped
Build Status
Buildable 65996
Build 62879: arc lint + arc unit

Event Timeline

markj created this revision.Tue, Aug 5, 4:16 PM
Herald added a subscriber: olce. · View Herald TranscriptTue, Aug 5, 4:16 PM
markj requested review of this revision.Tue, Aug 5, 4:16 PM
Harbormaster completed remote builds in B65981: Diff 159790.Tue, Aug 5, 4:16 PM
imp accepted this revision.Tue, Aug 5, 4:18 PM

That looks good to me. This routine can return errors for other conditions. Thanks for catching this dropped ball.

This revision is now accepted and ready to land.Tue, Aug 5, 4:18 PM
kib added inline comments.Wed, Aug 6, 12:04 AM
sys/kern/kern_time.c
574–575

I do not think it is neccessary. td_rtcgen is cleared by other threads to make sure that sleeping thread notices the wall clock change. There we are no longer intend sleeping, so I do not see point in maintaining rtcgen.

markj updated this revision to Diff 159827.Wed, Aug 6, 12:09 AM
markj marked an inline comment as done.

Don't bother resetting td_rtcgen

This revision now requires review to proceed.Wed, Aug 6, 12:09 AM
Harbormaster completed remote builds in B65996: Diff 159827.Wed, Aug 6, 12:09 AM
kib accepted this revision.Wed, Aug 6, 12:12 AM
This revision is now accepted and ready to land.Wed, Aug 6, 12:12 AM
kib added a comment.Wed, Aug 6, 12:20 AM

Sorry, I figured it out. I think that the initial version was right, and the reset needs to be restored. It is not assumed to leak from the wrappers of sleepers that sleep using wall time values. See the sleeping_on_old_rtc() code and its use in tc_setclock(). Not resetting td_rtcgen would cause spurious wakeups for later sleeps on non-wall clock timeouts if the wall clock changes.

From this principle, I believe timerfd_read() is buggy.

markj updated this revision to Diff 159851.Wed, Aug 6, 1:06 PM

Restore clearing td_rtcgen, otherwise we may spuriously abort sleeps

This revision now requires review to proceed.Wed, Aug 6, 1:06 PM
Harbormaster completed remote builds in B66012: Diff 159851.Wed, Aug 6, 1:06 PM
kib accepted this revision.Wed, Aug 6, 9:04 PM
This revision is now accepted and ready to land.Wed, Aug 6, 9:04 PM
markj added a comment.Wed, Aug 6, 9:26 PM

This change fixes the panic, but there are some other places where kern_clock_gettime()'s return value is not checked. I will fix those up.

Closed by commit rG7556b55f0d67: time: Handle kern_clock_gettime() failures in kern_clock_nanosleep() (authored by markj). · Explain WhyWed, Aug 6, 9:27 PM
This revision was automatically updated to reflect the committed changes.
markj added a commit: rG7556b55f0d67: time: Handle kern_clock_gettime() failures in kern_clock_nanosleep().

Revision Contents
Changeset List

PathSize
sys/
kern/
3 lines

Diff 159827

View Options

sys/kern/kern_time.c

Loading...