From 28004bc9e610aa735f469d59d7079f077a061a65 Mon Sep 17 00:00:00 2001 From: Christian Helmuth Date: Fri, 29 Sep 2017 14:12:10 +0200 Subject: [PATCH] timer: limit rate of handling timeouts Ensure that the timer does not handle timeouts again within 1000 microseconds after the last handling of timeouts. This makes denial of service attacks harder. This commit does not limit the rate of timeout signals handled inside the timer but it causes the timer to do it less often. If a client continuously installs a very small timeout at the timer it still causes a signal to be submitted to the timer each time and some extra CPU time to be spent in the internal handling method. But only every 1000 microseconds this internal handling causes user timeouts to trigger. If we would want to limit also the call of the internal handling method to ensure that CPU time is spent beside the RPCs only every 1000 microseconds, things would get more complex. For instance, on NOVA Time_source::schedule_timeout(0) must be called each time a new timeout gets installed and becomes head of the scheduling queue. We cannot simply overwrite the already running timeout with the new one. Ref #2490 --- repos/base/lib/symbols/ld | 4 +- repos/os/include/os/alarm.h | 40 ++++++++++----- repos/os/include/timer/timeout.h | 3 +- .../drivers/timer/include/root_component.h | 5 +- repos/os/src/lib/alarm/alarm.cc | 51 +++++++++++-------- repos/os/src/lib/timeout/timeout.cc | 6 ++- 6 files changed, 70 insertions(+), 39 deletions(-) diff --git a/repos/base/lib/symbols/ld b/repos/base/lib/symbols/ld index a0b9f40cd..01cde99d0 100644 --- a/repos/base/lib/symbols/ld +++ b/repos/base/lib/symbols/ld @@ -200,8 +200,8 @@ _ZN6Genode23Alarm_timeout_scheduler14handle_timeoutENS_8DurationE T _ZN6Genode23Alarm_timeout_scheduler18_schedule_one_shotERNS_7TimeoutENS_12MicrosecondsE T _ZN6Genode23Alarm_timeout_scheduler18_schedule_periodicERNS_7TimeoutENS_12MicrosecondsE T _ZN6Genode23Alarm_timeout_scheduler7_enableEv T -_ZN6Genode23Alarm_timeout_schedulerC1ERNS_11Time_sourceE T -_ZN6Genode23Alarm_timeout_schedulerC2ERNS_11Time_sourceE T +_ZN6Genode23Alarm_timeout_schedulerC1ERNS_11Time_sourceENS_12MicrosecondsE T +_ZN6Genode23Alarm_timeout_schedulerC2ERNS_11Time_sourceENS_12MicrosecondsE T _ZN6Genode25env_stack_area_region_mapE B 8 _ZN6Genode28env_stack_area_ram_allocatorE B 8 _ZN6Genode29upgrade_pd_quota_non_blockingENS_9Ram_quotaENS_9Cap_quotaE T diff --git a/repos/os/include/os/alarm.h b/repos/os/include/os/alarm.h index d009c93bf..9967493cd 100644 --- a/repos/os/include/os/alarm.h +++ b/repos/os/include/os/alarm.h @@ -32,10 +32,17 @@ class Genode::Alarm friend class Alarm_scheduler; + struct Raw + { + Time deadline; /* next deadline */ + bool deadline_period; + Time period; /* duration between alarms */ + + bool is_pending_at(unsigned long time, bool time_period) const; + }; + Lock _dispatch_lock; /* taken during handle method */ - Time _deadline; /* next deadline */ - bool _deadline_period; - Time _period; /* duration between alarms */ + Raw _raw; int _active; /* set to one when active */ Alarm *_next; /* next alarm in alarm list */ Alarm_scheduler *_scheduler; /* currently assigned scheduler */ @@ -45,17 +52,15 @@ class Genode::Alarm bool deadline_period, Alarm_scheduler *scheduler) { - _period = period; - _deadline_period = deadline_period; - _deadline = deadline; - _scheduler = scheduler; + _raw.period = period; + _raw.deadline_period = deadline_period; + _raw.deadline = deadline; + _scheduler = scheduler; } void _reset() { _assign(0, 0, false, 0), _active = 0, _next = 0; } - bool _is_pending_at(unsigned long time, bool time_period) const; - protected: /** @@ -79,10 +84,11 @@ class Genode::Alarm_scheduler { private: - Lock _lock; /* protect alarm list */ - Alarm *_head; /* head of alarm list */ - Alarm::Time _now; /* recent time (updated by handle method) */ + Lock _lock; /* protect alarm list */ + Alarm *_head { nullptr }; /* head of alarm list */ + Alarm::Time _now { 0UL }; /* recent time (updated by handle method) */ bool _now_period { false }; + Alarm::Raw _min_handle_period; /** * Enqueue alarm into alarm queue @@ -111,7 +117,15 @@ class Genode::Alarm_scheduler public: - Alarm_scheduler() : _head(0), _now(0) { } + Alarm_scheduler(Alarm::Time min_handle_period = 1) + { + Alarm::Time const deadline = _now + min_handle_period; + _min_handle_period.period = min_handle_period; + _min_handle_period.deadline = deadline; + _min_handle_period.deadline_period = _now > deadline ? + !_now_period : _now_period; + } + ~Alarm_scheduler(); /** diff --git a/repos/os/include/timer/timeout.h b/repos/os/include/timer/timeout.h index ac75445cd..d1dd38bb8 100644 --- a/repos/os/include/timer/timeout.h +++ b/repos/os/include/timer/timeout.h @@ -224,7 +224,8 @@ class Genode::Alarm_timeout_scheduler : private Noncopyable, public: - Alarm_timeout_scheduler(Time_source &time_source); + Alarm_timeout_scheduler(Time_source &time_source, + Microseconds min_handle_period = Microseconds(1)); /*********************** diff --git a/repos/os/src/drivers/timer/include/root_component.h b/repos/os/src/drivers/timer/include/root_component.h index 41045b0b7..c9f85e881 100644 --- a/repos/os/src/drivers/timer/include/root_component.h +++ b/repos/os/src/drivers/timer/include/root_component.h @@ -29,6 +29,8 @@ class Timer::Root_component : public Genode::Root_component { private: + enum { MIN_TIMEOUT_US = 1000 }; + Time_source _time_source; Genode::Alarm_timeout_scheduler _timeout_scheduler; @@ -55,7 +57,8 @@ class Timer::Root_component : public Genode::Root_component Root_component(Genode::Env &env, Genode::Allocator &md_alloc) : Genode::Root_component(&env.ep().rpc_ep(), &md_alloc), - _time_source(env), _timeout_scheduler(_time_source) + _time_source(env), + _timeout_scheduler(_time_source, Microseconds(MIN_TIMEOUT_US)) { _timeout_scheduler._enable(); } diff --git a/repos/os/src/lib/alarm/alarm.cc b/repos/os/src/lib/alarm/alarm.cc index c50cc440c..a38e49520 100644 --- a/repos/os/src/lib/alarm/alarm.cc +++ b/repos/os/src/lib/alarm/alarm.cc @@ -34,7 +34,7 @@ void Alarm_scheduler::_unsynchronized_enqueue(Alarm *alarm) } /* if deadline is smaller than any other deadline, put it on the head */ - if (alarm->_is_pending_at(_head->_deadline, _head->_deadline_period)) { + if (alarm->_raw.is_pending_at(_head->_raw.deadline, _head->_raw.deadline_period)) { alarm->_next = _head; _head = alarm; return; @@ -43,7 +43,7 @@ void Alarm_scheduler::_unsynchronized_enqueue(Alarm *alarm) /* find list element with a higher deadline */ Alarm *curr = _head; while (curr->_next && - curr->_next->_is_pending_at(alarm->_deadline, alarm->_deadline_period)) + curr->_next->_raw.is_pending_at(alarm->_raw.deadline, alarm->_raw.deadline_period)) { curr = curr->_next; } @@ -83,12 +83,12 @@ void Alarm_scheduler::_unsynchronized_dequeue(Alarm *alarm) } -bool Alarm::_is_pending_at(unsigned long time, bool time_period) const +bool Alarm::Raw::is_pending_at(unsigned long time, bool time_period) const { - return (time_period == _deadline_period && - time >= _deadline) || - (time_period != _deadline_period && - time < _deadline); + return (time_period == deadline_period && + time >= deadline) || + (time_period != deadline_period && + time < deadline); } @@ -96,7 +96,7 @@ Alarm *Alarm_scheduler::_get_pending_alarm() { Lock::Guard lock_guard(_lock); - if (!_head || !_head->_is_pending_at(_now, _now_period)) { + if (!_head || !_head->_raw.is_pending_at(_now, _now_period)) { return nullptr; } /* remove alarm from head of the list */ @@ -128,19 +128,27 @@ void Alarm_scheduler::handle(Alarm::Time curr_time) } _now = curr_time; + if (!_min_handle_period.is_pending_at(_now, _now_period)) { + return; + } + Alarm::Time const deadline = _now + _min_handle_period.period; + _min_handle_period.deadline = deadline; + _min_handle_period.deadline_period = _now > deadline ? + !_now_period : _now_period; + Alarm *curr; while ((curr = _get_pending_alarm())) { unsigned long triggered = 1; - if (curr->_period) { - Alarm::Time deadline = curr->_deadline; + if (curr->_raw.period) { + Alarm::Time deadline = curr->_raw.deadline; /* schedule next event */ if (deadline == 0) deadline = curr_time; - triggered += (curr_time - deadline) / curr->_period; + triggered += (curr_time - deadline) / curr->_raw.period; } /* do not reschedule if alarm function returns 0 */ @@ -153,21 +161,21 @@ void Alarm_scheduler::handle(Alarm::Time curr_time) * the current time but If the alarm had no deadline by now, * initialize it with the current time. */ - if (curr->_deadline == 0) { - curr->_deadline = _now; - curr->_deadline_period = _now_period; + if (curr->_raw.deadline == 0) { + curr->_raw.deadline = _now; + curr->_raw.deadline_period = _now_period; } /* * Raise the deadline value by one period of the alarm and * if the deadline value wraps thereby, update also in which * period it is located. */ - Alarm::Time const deadline = curr->_deadline + - triggered * curr->_period; - if (curr->_deadline > deadline) { - curr->_deadline_period = !curr->_deadline_period; + Alarm::Time const deadline = curr->_raw.deadline + + triggered * curr->_raw.period; + if (curr->_raw.deadline > deadline) { + curr->_raw.deadline_period = !curr->_raw.deadline_period; } - curr->_deadline = deadline; + curr->_raw.deadline = deadline; /* synchronize enqueue operation */ Lock::Guard lock_guard(_lock); @@ -251,8 +259,11 @@ bool Alarm_scheduler::next_deadline(Alarm::Time *deadline) if (!_head) return false; if (deadline) - *deadline = _head->_deadline; + *deadline = _head->_raw.deadline; + if (*deadline < _min_handle_period.deadline) { + *deadline = _min_handle_period.deadline; + } return true; } diff --git a/repos/os/src/lib/timeout/timeout.cc b/repos/os/src/lib/timeout/timeout.cc index f3a44a5e9..fef207ca7 100644 --- a/repos/os/src/lib/timeout/timeout.cc +++ b/repos/os/src/lib/timeout/timeout.cc @@ -72,6 +72,7 @@ void Alarm_timeout_scheduler::handle_timeout(Duration) _alarm_scheduler.handle(curr_time_us); + /* sleep time is either until the next deadline or the maximum timout */ unsigned long sleep_time_us; Alarm::Time deadline_us; if (_alarm_scheduler.next_deadline(&deadline_us)) { @@ -89,9 +90,10 @@ void Alarm_timeout_scheduler::handle_timeout(Duration) } -Alarm_timeout_scheduler::Alarm_timeout_scheduler(Time_source &time_source) +Alarm_timeout_scheduler::Alarm_timeout_scheduler(Time_source &time_source, + Microseconds min_handle_period) : - _time_source(time_source) + _time_source(time_source), _alarm_scheduler(min_handle_period.value) { }