Skip to content

Commit 11306d7

Browse files
authored
Kernel: Modify TimeManagement::current_time(..) API so it can't fail. (#6869)
The fact that current_time can "fail" makes its use a bit awkward. All callers in the Kernel are trusted besides syscalls, so assert that they never get there, and make sure all current callers perform validation of the clock_id with TimeManagement::is_valid_clock_id(). I have fuzzed this change locally for a bit to make sure I didn't miss any obvious regression.
1 parent 64b4e3f commit 11306d7

File tree

6 files changed

+14
-13
lines changed

6 files changed

+14
-13
lines changed

Kernel/Syscalls/alarm.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ KResultOr<unsigned> Process::sys$alarm(unsigned seconds)
2525
}
2626

2727
if (seconds > 0) {
28-
auto deadline = TimeManagement::the().current_time(CLOCK_REALTIME_COARSE).value();
28+
auto deadline = TimeManagement::the().current_time(CLOCK_REALTIME_COARSE);
2929
deadline = deadline + Time::from_seconds(seconds);
3030
m_alarm_timer = TimerQueue::the().add_timer_without_id(CLOCK_REALTIME_COARSE, deadline, [this]() {
3131
[[maybe_unused]] auto rc = send_signal(SIGALRM, nullptr);

Kernel/Syscalls/clock.cpp

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,13 @@ KResultOr<int> Process::sys$clock_gettime(clockid_t clock_id, Userspace<timespec
1414
{
1515
REQUIRE_PROMISE(stdio);
1616

17-
auto time = TimeManagement::the().current_time(clock_id);
18-
if (time.is_error())
19-
return time.error();
17+
if (!TimeManagement::is_valid_clock_id(clock_id))
18+
return EINVAL;
2019

21-
auto ts = time.value().to_timespec();
20+
auto ts = TimeManagement::the().current_time(clock_id).to_timespec();
2221
if (!copy_to_user(user_ts, &ts))
2322
return EFAULT;
23+
2424
return 0;
2525
}
2626

Kernel/ThreadBlockers.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ Thread::BlockTimeout::BlockTimeout(bool is_absolute, const Time* time, const Tim
2323
m_time = *time;
2424
m_should_block = true;
2525
}
26-
m_start_time = start_time ? *start_time : TimeManagement::the().current_time(clock_id).value();
26+
m_start_time = start_time ? *start_time : TimeManagement::the().current_time(clock_id);
2727
if (!is_absolute)
2828
m_time += m_start_time;
2929
}
@@ -326,7 +326,7 @@ void Thread::SleepBlocker::calculate_remaining()
326326
{
327327
if (!m_remaining)
328328
return;
329-
auto time_now = TimeManagement::the().current_time(m_deadline.clock_id()).value();
329+
auto time_now = TimeManagement::the().current_time(m_deadline.clock_id());
330330
if (time_now < m_deadline.absolute_time())
331331
*m_remaining = m_deadline.absolute_time() - time_now;
332332
else

Kernel/Time/TimeManagement.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ bool TimeManagement::is_valid_clock_id(clockid_t clock_id)
4444
};
4545
}
4646

47-
KResultOr<Time> TimeManagement::current_time(clockid_t clock_id) const
47+
Time TimeManagement::current_time(clockid_t clock_id) const
4848
{
4949
switch (clock_id) {
5050
case CLOCK_MONOTONIC:
@@ -58,7 +58,8 @@ KResultOr<Time> TimeManagement::current_time(clockid_t clock_id) const
5858
case CLOCK_REALTIME_COARSE:
5959
return epoch_time(TimePrecision::Coarse);
6060
default:
61-
return KResult(EINVAL);
61+
// Syscall entrypoint is missing a is_valid_clock_id(..) check?
62+
VERIFY_NOT_REACHED();
6263
}
6364
}
6465

Kernel/Time/TimeManagement.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ class TimeManagement {
3434
static TimeManagement& the();
3535

3636
static bool is_valid_clock_id(clockid_t);
37-
KResultOr<Time> current_time(clockid_t) const;
37+
Time current_time(clockid_t) const;
3838
Time monotonic_time(TimePrecision = TimePrecision::Coarse) const;
3939
Time monotonic_time_raw() const
4040
{

Kernel/TimerQueue.cpp

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ Time Timer::now(bool is_firing) const
4444
break;
4545
}
4646
}
47-
return TimeManagement::the().current_time(clock_id).value();
47+
return TimeManagement::the().current_time(clock_id);
4848
}
4949

5050
TimerQueue& TimerQueue::the()
@@ -59,7 +59,7 @@ UNMAP_AFTER_INIT TimerQueue::TimerQueue()
5959

6060
RefPtr<Timer> TimerQueue::add_timer_without_id(clockid_t clock_id, const Time& deadline, Function<void()>&& callback)
6161
{
62-
if (deadline <= TimeManagement::the().current_time(clock_id).value())
62+
if (deadline <= TimeManagement::the().current_time(clock_id))
6363
return {};
6464

6565
// Because timer handlers can execute on any processor and there is
@@ -117,7 +117,7 @@ void TimerQueue::add_timer_locked(NonnullRefPtr<Timer> timer)
117117

118118
TimerId TimerQueue::add_timer(clockid_t clock_id, const Time& deadline, Function<void()>&& callback)
119119
{
120-
auto expires = TimeManagement::the().current_time(clock_id).value();
120+
auto expires = TimeManagement::the().current_time(clock_id);
121121
expires = expires + deadline;
122122
return add_timer(adopt_ref(*new Timer(clock_id, expires, move(callback))));
123123
}

0 commit comments

Comments
 (0)