diff --git a/pkg/sentry/kernel/BUILD b/pkg/sentry/kernel/BUILD index b40bd63c5..32e146ce8 100644 --- a/pkg/sentry/kernel/BUILD +++ b/pkg/sentry/kernel/BUILD @@ -105,6 +105,13 @@ declare_mutex( prefix = "threadGroupTimer", ) +declare_mutex( + name = "timekeeper_tcpip_timer_mutex", + out = "timekeeper_tcpip_timer_mutex.go", + package = "kernel", + prefix = "timekeeperTcpipTimer", +) + declare_mutex( name = "cgroup_mounts_mutex", out = "cgroup_mounts_mutex.go", @@ -290,6 +297,7 @@ go_library( "threads_impl.go", "timekeeper.go", "timekeeper_state.go", + "timekeeper_tcpip_timer_mutex.go", "tty.go", "user_counters_mutex.go", "uts_namespace.go", diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index b4ba2ca0c..2db357098 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -20,6 +20,7 @@ // // Kernel.extMu // TTY.mu +// timekeeperTcpipTimer.mu // ThreadGroup.timerMu // Locks acquired by ktime.Timer methods // TaskSet.mu diff --git a/pkg/sentry/kernel/timekeeper.go b/pkg/sentry/kernel/timekeeper.go index 339efc33a..75b2986d1 100644 --- a/pkg/sentry/kernel/timekeeper.go +++ b/pkg/sentry/kernel/timekeeper.go @@ -170,32 +170,6 @@ func (t *Timekeeper) SetClocks(c sentrytime.Clocks, params *VDSOParamPage) { } } -var _ tcpip.Clock = (*Timekeeper)(nil) - -// Now implements tcpip.Clock. -func (t *Timekeeper) Now() time.Time { - nsec, err := t.GetTime(sentrytime.Realtime) - if err != nil { - panic("timekeeper.GetTime(sentrytime.Realtime): " + err.Error()) - } - return time.Unix(0, nsec) -} - -// NowMonotonic implements tcpip.Clock. -func (t *Timekeeper) NowMonotonic() tcpip.MonotonicTime { - nsec, err := t.GetTime(sentrytime.Monotonic) - if err != nil { - panic("timekeeper.GetTime(sentrytime.Monotonic): " + err.Error()) - } - var mt tcpip.MonotonicTime - return mt.Add(time.Duration(nsec) * time.Nanosecond) -} - -// AfterFunc implements tcpip.Clock. -func (t *Timekeeper) AfterFunc(d time.Duration, f func()) tcpip.Timer { - return ktime.AfterFunc(t.realtimeClock, d, f) -} - // startUpdater starts an update goroutine that keeps the clocks updated. // // mu must be held. @@ -354,3 +328,116 @@ func (tc *timekeeperClock) Now() ktime.Time { func (tc *timekeeperClock) NewTimer(l ktime.Listener) ktime.Timer { return ktime.NewSampledTimer(tc, l) } + +var _ tcpip.Clock = (*Timekeeper)(nil) + +// Now implements tcpip.Clock. +func (t *Timekeeper) Now() time.Time { + nsec, err := t.GetTime(sentrytime.Realtime) + if err != nil { + panic("timekeeper.GetTime(sentrytime.Realtime): " + err.Error()) + } + return time.Unix(0, nsec) +} + +// NowMonotonic implements tcpip.Clock. +func (t *Timekeeper) NowMonotonic() tcpip.MonotonicTime { + nsec, err := t.GetTime(sentrytime.Monotonic) + if err != nil { + panic("timekeeper.GetTime(sentrytime.Monotonic): " + err.Error()) + } + var mt tcpip.MonotonicTime + return mt.Add(time.Duration(nsec) * time.Nanosecond) +} + +// AfterFunc implements tcpip.Clock. +func (t *Timekeeper) AfterFunc(d time.Duration, f func()) tcpip.Timer { + timer := &timekeeperTcpipTimer{ + clock: t.monotonicClock, + fn: f, + } + timer.Reset(d) + return timer +} + +// timekeeperTcpipTimer implements tcpip.Timer by wrapping a ktime.SampledTimer. +// tcpip.Timer does not define a Destroy method, so each timer expiration and +// each call to Timer.Stop() must release all resources by calling +// ktime.SampledTimer.Destroy(). +type timekeeperTcpipTimer struct { + // immutable + clock *timekeeperClock + fn func() + + // mu protects t. + mu timekeeperTcpipTimerMutex + + // t stores the latest running Timer. This is replaced whenever Reset is + // called since Timer cannot be restarted once it has been Destroyed by Stop. + // + // This field is nil iff Stop has been called. + t *ktime.SampledTimer + + // resets is the number of times Reset has been called. resets is written + // with both mu and ktime.SampledTimer locks held, so it may be read with + // either or both locks held. + resets int +} + +// Stop implements tcpip.Timer.Stop. +func (r *timekeeperTcpipTimer) Stop() bool { + r.mu.Lock() + defer r.mu.Unlock() + + if r.t == nil { + return false + } + _, lastSetting := r.t.Set(ktime.Setting{}, nil) + r.t.Destroy() + r.t = nil + return lastSetting.Enabled +} + +// stopExpired is equivalent to Stop, but is called when the timer expires. +func (r *timekeeperTcpipTimer) stopExpired(reset int) { + r.mu.Lock() + defer r.mu.Unlock() + + if r.t == nil || r.resets != reset { + return + } + r.t.Destroy() + r.t = nil +} + +// Reset implements tcpip.Timer.Reset. +func (r *timekeeperTcpipTimer) Reset(d time.Duration) { + r.mu.Lock() + defer r.mu.Unlock() + + if r.t == nil { + r.t = ktime.NewSampledTimer(r.clock, r) + } + r.t.Set(ktime.Setting{ + Enabled: true, + Next: r.clock.Now().Add(d), + }, r.incResets) +} + +func (r *timekeeperTcpipTimer) incResets() { + r.resets++ +} + +// NotifyTimer implements ktime.Listener.NotifyTimer. +func (r *timekeeperTcpipTimer) NotifyTimer(exp uint64) { + // Implementations of ktime.Listener.NotifyTimer() can't call Timer methods + // due to lock ordering, so we must call r.t.Destroy() from another + // goroutine. We also must call r.stopExpired() rather than r.Stop(), since + // the latter might cancel an unrelated call to r.Reset() that happens + // between now and when this goroutine runs. + thisReset := r.resets + go func() { + r.stopExpired(thisReset) + r.fn() + }() +} diff --git a/pkg/sentry/ktime/BUILD b/pkg/sentry/ktime/BUILD index d16dbe02a..9401f98c7 100644 --- a/pkg/sentry/ktime/BUILD +++ b/pkg/sentry/ktime/BUILD @@ -65,7 +65,6 @@ go_library( "synthetic_timer_list.go", "synthetic_timer_set.go", "uint64_range.go", - "util.go", ], visibility = ["//pkg/sentry:internal"], deps = [ diff --git a/pkg/sentry/ktime/util.go b/pkg/sentry/ktime/util.go deleted file mode 100644 index 1bddb027d..000000000 --- a/pkg/sentry/ktime/util.go +++ /dev/null @@ -1,127 +0,0 @@ -// Copyright 2020 The gVisor Authors. -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package ktime - -import ( - "sync" - "time" -) - -// AfterFunc waits for duration to elapse according to clock then runs fn. -// The timer is started immediately and will fire exactly once. -func AfterFunc(clock Clock, duration time.Duration, fn func()) *VariableTimer { - timer := &VariableTimer{ - clock: clock, - } - timer.notifier = functionNotifier{ - fn: func() { - // tcpip.Timer.Stop() explicitly states that the function is called in a - // separate goroutine that Stop() does not synchronize with. - // Timer.Destroy() synchronizes with calls to Listener.NotifyTimer(). - // This is semantically meaningful because, in the former case, it's - // legal to call tcpip.Timer.Stop() while holding locks that may also be - // taken by the function, but this isn't so in the latter case. Most - // immediately, Timer calls Listener.NotifyTimer() while holding - // Timer.mu. A deadlock occurs without spawning a goroutine: - // T1: (Timer expires) - // => Timer.Tick() <- Timer.mu.Lock() called - // => Listener.NotifyTimer() - // => Timer.Stop() - // => Timer.Destroy() <- Timer.mu.Lock() called, deadlock! - // - // Spawning a goroutine avoids the deadlock: - // T1: (Timer expires) - // => Timer.Tick() <- Timer.mu.Lock() called - // => Listener.NotifyTimer() <- Launches T2 - // T2: - // => Timer.Stop() - // => Timer.Destroy() <- Timer.mu.Lock() called, blocks - // T1: - // => (returns) <- Timer.mu.Unlock() called - // T2: - // => (continues) <- No deadlock! - go func() { - timer.Stop() - fn() - }() - }, - } - timer.Reset(duration) - return timer -} - -// VariableTimer is a resettable timer with variable duration expirations. -// Implements tcpip.Timer, which does not define a Destroy method; instead, all -// resources are released after timer expiration and calls to Timer.Stop. -// -// Must be created by AfterFunc. -type VariableTimer struct { - // clock is the time source. clock is immutable. - clock Clock - - // notifier is called when the Timer expires. notifier is immutable. - notifier functionNotifier - - // mu protects t. - mu sync.Mutex - - // t stores the latest running Timer. This is replaced whenever Reset is - // called since Timer cannot be restarted once it has been Destroyed by Stop. - // - // This field is nil iff Stop has been called. - t Timer -} - -// Stop implements tcpip.Timer.Stop. -func (r *VariableTimer) Stop() bool { - r.mu.Lock() - defer r.mu.Unlock() - - if r.t == nil { - return false - } - _, lastSetting := r.t.Set(Setting{}, nil) - r.t.Destroy() - r.t = nil - return lastSetting.Enabled -} - -// Reset implements tcpip.Timer.Reset. -func (r *VariableTimer) Reset(d time.Duration) { - r.mu.Lock() - defer r.mu.Unlock() - - if r.t == nil { - r.t = r.clock.NewTimer(&r.notifier) - } - - r.t.Set(Setting{ - Enabled: true, - Period: 0, - Next: r.clock.Now().Add(d), - }, nil) -} - -// functionNotifier is a TimerListener that runs a function. -// -// functionNotifier cannot be saved or loaded. -type functionNotifier struct { - fn func() -} - -// NotifyTimer implements ktime.TimerListener.NotifyTimer. -func (f *functionNotifier) NotifyTimer(uint64) { - f.fn() -}