From 7a92412c08fb1e32c68ebf036831b4175fad9d16 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Thu, 8 Jun 2023 10:43:49 -0700 Subject: [PATCH] fasync: Unlock the FileAsync mutex before sending signal This cl reverts cl/537942317 and cl/538645255. We need to lock the mutex once and read all required fields of the FileAsync structure and then release it right before calling SendSignal. Right now, getPermAndSignal() and recipient() can return inconsistent results, because FileAsync can be changed between them. PiperOrigin-RevId: 538829847 --- pkg/sentry/kernel/fasync/fasync.go | 46 ++++++++++++------------------ 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/pkg/sentry/kernel/fasync/fasync.go b/pkg/sentry/kernel/fasync/fasync.go index ff2095eda..0e6f4dc38 100644 --- a/pkg/sentry/kernel/fasync/fasync.go +++ b/pkg/sentry/kernel/fasync/fasync.go @@ -85,12 +85,17 @@ type FileAsync struct { recipientT *kernel.Task } -func (a *FileAsync) recipient() *kernel.Task { +// NotifyEvent implements waiter.EventListener.NotifyEvent. +func (a *FileAsync) NotifyEvent(mask waiter.EventMask) { a.mu.Lock() - defer a.mu.Unlock() + locked := true + defer func() { + if locked { + a.mu.Unlock() + } + }() if !a.registered { - // No recipient has been registered. - return nil + return } t := a.recipientT tg := a.recipientTG @@ -100,34 +105,17 @@ func (a *FileAsync) recipient() *kernel.Task { if tg != nil { t = tg.Leader() } - return t -} - -// getPermAndSignal checks the credentials and returns the result of the -// credentials check and signal. -// -//go:nosplit -func (a *FileAsync) getPermAndSignal(c *auth.Credentials) (int32, bool) { - a.mu.Lock() - defer a.mu.Unlock() - sig := int32(a.signal) + if t == nil { + // No recipient has been registered. + return + } + c := t.Credentials() // Logic from sigio_perm in fs/fcntl.c. permCheck := (a.requester.EffectiveKUID == 0 || a.requester.EffectiveKUID == c.SavedKUID || a.requester.EffectiveKUID == c.RealKUID || a.requester.RealKUID == c.SavedKUID || a.requester.RealKUID == c.RealKUID) - return sig, permCheck -} - -// NotifyEvent implements waiter.EventListener.NotifyEvent. -func (a *FileAsync) NotifyEvent(mask waiter.EventMask) { - t := a.recipient() - if t == nil { - return - } - c := t.Credentials() - sig, permCheck := a.getPermAndSignal(c) if !permCheck { return } @@ -135,8 +123,8 @@ func (a *FileAsync) NotifyEvent(mask waiter.EventMask) { Signo: int32(linux.SIGIO), Code: linux.SI_KERNEL, } - if sig != 0 { - signalInfo.Signo = sig + if a.signal != 0 { + signalInfo.Signo = int32(a.signal) signalInfo.SetFD(uint32(a.fd)) var band int64 for m, bandCode := range bandTable { @@ -146,6 +134,8 @@ func (a *FileAsync) NotifyEvent(mask waiter.EventMask) { } signalInfo.SetBand(band) } + a.mu.Unlock() + locked = false t.SendSignal(signalInfo) }