diff options
author | Etienne Perot <eperot@google.com> | 2020-12-03 06:18:03 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2020-12-03 06:20:29 -0800 |
commit | 6f60a2b0a27a742690aa6acd5df1912ccb5fc8d3 (patch) | |
tree | b706a5e534592d9aaa7fb4dc6237b4ab3686af13 | |
parent | 80552b936d06e43ea77df09a6b6c5ce2600a6f6a (diff) |
Implement `fcntl` options `F_GETSIG` and `F_SETSIG`.
These options allow overriding the signal that gets sent to the process when
I/O operations are available on the file descriptor, rather than the default
`SIGIO` signal. Doing so also populates `siginfo` to contain extra information
about which file descriptor caused the event (`si_fd`) and what events happened
on it (`si_band`). The logic around which FD is populated within `si_fd`
matches Linux's, which means it has some weird edge cases where that value may
not actually refer to a file descriptor that is still valid.
This CL also ports extra S/R logic regarding async handler in VFS2.
Without this, async I/O handlers aren't properly re-registered after S/R.
PiperOrigin-RevId: 345436598
-rw-r--r-- | pkg/abi/linux/fcntl.go | 2 | ||||
-rw-r--r-- | pkg/sentry/arch/signal.go | 23 | ||||
-rw-r--r-- | pkg/sentry/fsimpl/overlay/regular_file.go | 4 | ||||
-rw-r--r-- | pkg/sentry/kernel/epoll/epoll.go | 7 | ||||
-rw-r--r-- | pkg/sentry/kernel/fasync/BUILD | 2 | ||||
-rw-r--r-- | pkg/sentry/kernel/fasync/fasync.go | 96 | ||||
-rw-r--r-- | pkg/sentry/syscalls/linux/sys_file.go | 16 | ||||
-rw-r--r-- | pkg/sentry/syscalls/linux/vfs2/fd.go | 18 | ||||
-rw-r--r-- | pkg/sentry/syscalls/linux/vfs2/ioctl.go | 2 | ||||
-rw-r--r-- | pkg/sentry/vfs/epoll.go | 10 | ||||
-rw-r--r-- | pkg/sentry/vfs/file_description.go | 9 | ||||
-rw-r--r-- | pkg/sentry/vfs/save_restore.go | 19 | ||||
-rw-r--r-- | pkg/waiter/waiter.go | 11 | ||||
-rw-r--r-- | pkg/waiter/waiter_test.go | 20 | ||||
-rw-r--r-- | test/syscalls/linux/BUILD | 3 | ||||
-rw-r--r-- | test/syscalls/linux/fcntl.cc | 486 |
16 files changed, 610 insertions, 118 deletions
diff --git a/pkg/abi/linux/fcntl.go b/pkg/abi/linux/fcntl.go index cc3571fad..d1ca56370 100644 --- a/pkg/abi/linux/fcntl.go +++ b/pkg/abi/linux/fcntl.go @@ -25,6 +25,8 @@ const ( F_SETLKW = 7 F_SETOWN = 8 F_GETOWN = 9 + F_SETSIG = 10 + F_GETSIG = 11 F_SETOWN_EX = 15 F_GETOWN_EX = 16 F_DUPFD_CLOEXEC = 1024 + 6 diff --git a/pkg/sentry/arch/signal.go b/pkg/sentry/arch/signal.go index c9fb55d00..5138f3bf5 100644 --- a/pkg/sentry/arch/signal.go +++ b/pkg/sentry/arch/signal.go @@ -251,3 +251,26 @@ func (s *SignalInfo) Arch() uint32 { func (s *SignalInfo) SetArch(val uint32) { usermem.ByteOrder.PutUint32(s.Fields[12:16], val) } + +// Band returns the si_band field. +func (s *SignalInfo) Band() int64 { + return int64(usermem.ByteOrder.Uint64(s.Fields[0:8])) +} + +// SetBand mutates the si_band field. +func (s *SignalInfo) SetBand(val int64) { + // Note: this assumes the platform uses `long` as `__ARCH_SI_BAND_T`. + // On some platforms, which gVisor doesn't support, `__ARCH_SI_BAND_T` is + // `int`. See siginfo.h. + usermem.ByteOrder.PutUint64(s.Fields[0:8], uint64(val)) +} + +// FD returns the si_fd field. +func (s *SignalInfo) FD() uint32 { + return usermem.ByteOrder.Uint32(s.Fields[8:12]) +} + +// SetFD mutates the si_fd field. +func (s *SignalInfo) SetFD(val uint32) { + usermem.ByteOrder.PutUint32(s.Fields[8:12], val) +} diff --git a/pkg/sentry/fsimpl/overlay/regular_file.go b/pkg/sentry/fsimpl/overlay/regular_file.go index 2b89a7a6d..25c785fd4 100644 --- a/pkg/sentry/fsimpl/overlay/regular_file.go +++ b/pkg/sentry/fsimpl/overlay/regular_file.go @@ -103,8 +103,8 @@ func (fd *regularFileFD) currentFDLocked(ctx context.Context) (*vfs.FileDescript for e, mask := range fd.lowerWaiters { fd.cachedFD.EventUnregister(e) upperFD.EventRegister(e, mask) - if ready&mask != 0 { - e.Callback.Callback(e) + if m := ready & mask; m != 0 { + e.Callback.Callback(e, m) } } } diff --git a/pkg/sentry/kernel/epoll/epoll.go b/pkg/sentry/kernel/epoll/epoll.go index 15519f0df..61aeca044 100644 --- a/pkg/sentry/kernel/epoll/epoll.go +++ b/pkg/sentry/kernel/epoll/epoll.go @@ -273,7 +273,7 @@ func (e *EventPoll) ReadEvents(max int) []linux.EpollEvent { // // Callback is called when one of the files we're polling becomes ready. It // moves said file to the readyList if it's currently in the waiting list. -func (p *pollEntry) Callback(*waiter.Entry) { +func (p *pollEntry) Callback(*waiter.Entry, waiter.EventMask) { e := p.epoll e.listsMu.Lock() @@ -306,9 +306,8 @@ func (e *EventPoll) initEntryReadiness(entry *pollEntry) { f.EventRegister(&entry.waiter, entry.mask) // Check if the file happens to already be in a ready state. - ready := f.Readiness(entry.mask) & entry.mask - if ready != 0 { - entry.Callback(&entry.waiter) + if ready := f.Readiness(entry.mask) & entry.mask; ready != 0 { + entry.Callback(&entry.waiter, ready) } } diff --git a/pkg/sentry/kernel/fasync/BUILD b/pkg/sentry/kernel/fasync/BUILD index 2b3955598..f855f038b 100644 --- a/pkg/sentry/kernel/fasync/BUILD +++ b/pkg/sentry/kernel/fasync/BUILD @@ -8,11 +8,13 @@ go_library( visibility = ["//:sandbox"], deps = [ "//pkg/abi/linux", + "//pkg/sentry/arch", "//pkg/sentry/fs", "//pkg/sentry/kernel", "//pkg/sentry/kernel/auth", "//pkg/sentry/vfs", "//pkg/sync", + "//pkg/syserror", "//pkg/waiter", ], ) diff --git a/pkg/sentry/kernel/fasync/fasync.go b/pkg/sentry/kernel/fasync/fasync.go index 153d2cd9b..b66d61c6f 100644 --- a/pkg/sentry/kernel/fasync/fasync.go +++ b/pkg/sentry/kernel/fasync/fasync.go @@ -17,22 +17,45 @@ package fasync import ( "gvisor.dev/gvisor/pkg/abi/linux" + "gvisor.dev/gvisor/pkg/sentry/arch" "gvisor.dev/gvisor/pkg/sentry/fs" "gvisor.dev/gvisor/pkg/sentry/kernel" "gvisor.dev/gvisor/pkg/sentry/kernel/auth" "gvisor.dev/gvisor/pkg/sentry/vfs" "gvisor.dev/gvisor/pkg/sync" + "gvisor.dev/gvisor/pkg/syserror" "gvisor.dev/gvisor/pkg/waiter" ) -// New creates a new fs.FileAsync. -func New() fs.FileAsync { - return &FileAsync{} +// Table to convert waiter event masks into si_band siginfo codes. +// Taken from fs/fcntl.c:band_table. +var bandTable = map[waiter.EventMask]int64{ + // POLL_IN + waiter.EventIn: linux.EPOLLIN | linux.EPOLLRDNORM, + // POLL_OUT + waiter.EventOut: linux.EPOLLOUT | linux.EPOLLWRNORM | linux.EPOLLWRBAND, + // POLL_ERR + waiter.EventErr: linux.EPOLLERR, + // POLL_PRI + waiter.EventPri: linux.EPOLLPRI | linux.EPOLLRDBAND, + // POLL_HUP + waiter.EventHUp: linux.EPOLLHUP | linux.EPOLLERR, } -// NewVFS2 creates a new vfs.FileAsync. -func NewVFS2() vfs.FileAsync { - return &FileAsync{} +// New returns a function that creates a new fs.FileAsync with the given file +// descriptor. +func New(fd int) func() fs.FileAsync { + return func() fs.FileAsync { + return &FileAsync{fd: fd} + } +} + +// NewVFS2 returns a function that creates a new vfs.FileAsync with the given +// file descriptor. +func NewVFS2(fd int) func() vfs.FileAsync { + return func() vfs.FileAsync { + return &FileAsync{fd: fd} + } } // FileAsync sends signals when the registered file is ready for IO. @@ -42,6 +65,12 @@ type FileAsync struct { // e is immutable after first use (which is protected by mu below). e waiter.Entry + // fd is the file descriptor to notify about. + // It is immutable, set at allocation time. This matches Linux semantics in + // fs/fcntl.c:fasync_helper. + // The fd value is passed to the signal recipient in siginfo.si_fd. + fd int + // regMu protects registeration and unregistration actions on e. // // regMu must be held while registration decisions are being made @@ -56,6 +85,10 @@ type FileAsync struct { mu sync.Mutex `state:"nosave"` requester *auth.Credentials registered bool + // signal is the signal to deliver upon I/O being available. + // The default value ("zero signal") means the default SIGIO signal will be + // delivered. + signal linux.Signal // Only one of the following is allowed to be non-nil. recipientPG *kernel.ProcessGroup @@ -64,10 +97,10 @@ type FileAsync struct { } // Callback sends a signal. -func (a *FileAsync) Callback(e *waiter.Entry) { +func (a *FileAsync) Callback(e *waiter.Entry, mask waiter.EventMask) { a.mu.Lock() + defer a.mu.Unlock() if !a.registered { - a.mu.Unlock() return } t := a.recipientT @@ -80,19 +113,34 @@ func (a *FileAsync) Callback(e *waiter.Entry) { } if t == nil { // No recipient has been registered. - a.mu.Unlock() return } c := t.Credentials() // Logic from sigio_perm in fs/fcntl.c. - if a.requester.EffectiveKUID == 0 || + 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 { - t.SendSignal(kernel.SignalInfoPriv(linux.SIGIO)) + a.requester.RealKUID == c.RealKUID) + if !permCheck { + return } - a.mu.Unlock() + signalInfo := &arch.SignalInfo{ + Signo: int32(linux.SIGIO), + Code: arch.SignalInfoKernel, + } + if a.signal != 0 { + signalInfo.Signo = int32(a.signal) + signalInfo.SetFD(uint32(a.fd)) + var band int64 + for m, bandCode := range bandTable { + if m&mask != 0 { + band |= bandCode + } + } + signalInfo.SetBand(band) + } + t.SendSignal(signalInfo) } // Register sets the file which will be monitored for IO events. @@ -186,3 +234,25 @@ func (a *FileAsync) ClearOwner() { a.recipientTG = nil a.recipientPG = nil } + +// Signal returns which signal will be sent to the signal recipient. +// A value of zero means the signal to deliver wasn't customized, which means +// the default signal (SIGIO) will be delivered. +func (a *FileAsync) Signal() linux.Signal { + a.mu.Lock() + defer a.mu.Unlock() + return a.signal +} + +// SetSignal overrides which signal to send when I/O is available. +// The default behavior can be reset by specifying signal zero, which means +// to send SIGIO. +func (a *FileAsync) SetSignal(signal linux.Signal) error { + if signal != 0 && !signal.IsValid() { + return syserror.EINVAL + } + a.mu.Lock() + defer a.mu.Unlock() + a.signal = signal + return nil +} diff --git a/pkg/sentry/syscalls/linux/sys_file.go b/pkg/sentry/syscalls/linux/sys_file.go index 519066a47..8db587401 100644 --- a/pkg/sentry/syscalls/linux/sys_file.go +++ b/pkg/sentry/syscalls/linux/sys_file.go @@ -646,7 +646,7 @@ func Ioctl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall if _, err := primitive.CopyInt32In(t, args[2].Pointer(), &set); err != nil { return 0, nil, err } - fSetOwn(t, file, set) + fSetOwn(t, int(fd), file, set) return 0, nil, nil case linux.FIOGETOWN, linux.SIOCGPGRP: @@ -901,8 +901,8 @@ func fGetOwn(t *kernel.Task, file *fs.File) int32 { // // If who is positive, it represents a PID. If negative, it represents a PGID. // If the PID or PGID is invalid, the owner is silently unset. -func fSetOwn(t *kernel.Task, file *fs.File, who int32) error { - a := file.Async(fasync.New).(*fasync.FileAsync) +func fSetOwn(t *kernel.Task, fd int, file *fs.File, who int32) error { + a := file.Async(fasync.New(fd)).(*fasync.FileAsync) if who < 0 { // Check for overflow before flipping the sign. if who-1 > who { @@ -1049,7 +1049,7 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall case linux.F_GETOWN: return uintptr(fGetOwn(t, file)), nil, nil case linux.F_SETOWN: - return 0, nil, fSetOwn(t, file, args[2].Int()) + return 0, nil, fSetOwn(t, int(fd), file, args[2].Int()) case linux.F_GETOWN_EX: addr := args[2].Pointer() owner := fGetOwnEx(t, file) @@ -1062,7 +1062,7 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall if err != nil { return 0, nil, err } - a := file.Async(fasync.New).(*fasync.FileAsync) + a := file.Async(fasync.New(int(fd))).(*fasync.FileAsync) switch owner.Type { case linux.F_OWNER_TID: task := t.PIDNamespace().TaskWithID(kernel.ThreadID(owner.PID)) @@ -1111,6 +1111,12 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall } n, err := sz.SetFifoSize(int64(args[2].Int())) return uintptr(n), nil, err + case linux.F_GETSIG: + a := file.Async(fasync.New(int(fd))).(*fasync.FileAsync) + return uintptr(a.Signal()), nil, nil + case linux.F_SETSIG: + a := file.Async(fasync.New(int(fd))).(*fasync.FileAsync) + return 0, nil, a.SetSignal(linux.Signal(args[2].Int())) default: // Everything else is not yet supported. return 0, nil, syserror.EINVAL diff --git a/pkg/sentry/syscalls/linux/vfs2/fd.go b/pkg/sentry/syscalls/linux/vfs2/fd.go index 36e89700e..7dd9ef857 100644 --- a/pkg/sentry/syscalls/linux/vfs2/fd.go +++ b/pkg/sentry/syscalls/linux/vfs2/fd.go @@ -165,7 +165,7 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall ownerType = linux.F_OWNER_PGRP who = -who } - return 0, nil, setAsyncOwner(t, file, ownerType, who) + return 0, nil, setAsyncOwner(t, int(fd), file, ownerType, who) case linux.F_GETOWN_EX: owner, hasOwner := getAsyncOwner(t, file) if !hasOwner { @@ -179,7 +179,7 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall if err != nil { return 0, nil, err } - return 0, nil, setAsyncOwner(t, file, owner.Type, owner.PID) + return 0, nil, setAsyncOwner(t, int(fd), file, owner.Type, owner.PID) case linux.F_SETPIPE_SZ: pipefile, ok := file.Impl().(*pipe.VFSPipeFD) if !ok { @@ -207,6 +207,16 @@ func Fcntl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall return 0, nil, err case linux.F_SETLK, linux.F_SETLKW: return 0, nil, posixLock(t, args, file, cmd) + case linux.F_GETSIG: + a := file.AsyncHandler() + if a == nil { + // Default behavior aka SIGIO. + return 0, nil, nil + } + return uintptr(a.(*fasync.FileAsync).Signal()), nil, nil + case linux.F_SETSIG: + a := file.SetAsyncHandler(fasync.NewVFS2(int(fd))).(*fasync.FileAsync) + return 0, nil, a.SetSignal(linux.Signal(args[2].Int())) default: // Everything else is not yet supported. return 0, nil, syserror.EINVAL @@ -241,7 +251,7 @@ func getAsyncOwner(t *kernel.Task, fd *vfs.FileDescription) (ownerEx linux.FOwne } } -func setAsyncOwner(t *kernel.Task, fd *vfs.FileDescription, ownerType, pid int32) error { +func setAsyncOwner(t *kernel.Task, fd int, file *vfs.FileDescription, ownerType, pid int32) error { switch ownerType { case linux.F_OWNER_TID, linux.F_OWNER_PID, linux.F_OWNER_PGRP: // Acceptable type. @@ -249,7 +259,7 @@ func setAsyncOwner(t *kernel.Task, fd *vfs.FileDescription, ownerType, pid int32 return syserror.EINVAL } - a := fd.SetAsyncHandler(fasync.NewVFS2).(*fasync.FileAsync) + a := file.SetAsyncHandler(fasync.NewVFS2(fd)).(*fasync.FileAsync) if pid == 0 { a.ClearOwner() return nil diff --git a/pkg/sentry/syscalls/linux/vfs2/ioctl.go b/pkg/sentry/syscalls/linux/vfs2/ioctl.go index 2806c3f6f..20c264fef 100644 --- a/pkg/sentry/syscalls/linux/vfs2/ioctl.go +++ b/pkg/sentry/syscalls/linux/vfs2/ioctl.go @@ -100,7 +100,7 @@ func Ioctl(t *kernel.Task, args arch.SyscallArguments) (uintptr, *kernel.Syscall ownerType = linux.F_OWNER_PGRP who = -who } - return 0, nil, setAsyncOwner(t, file, ownerType, who) + return 0, nil, setAsyncOwner(t, int(fd), file, ownerType, who) } ret, err := file.Ioctl(t, t.MemoryManager(), args) diff --git a/pkg/sentry/vfs/epoll.go b/pkg/sentry/vfs/epoll.go index a98aac52b..072655fe8 100644 --- a/pkg/sentry/vfs/epoll.go +++ b/pkg/sentry/vfs/epoll.go @@ -204,8 +204,8 @@ func (ep *EpollInstance) AddInterest(file *FileDescription, num int32, event lin file.EventRegister(&epi.waiter, wmask) // Check if the file is already ready. - if file.Readiness(wmask)&wmask != 0 { - epi.Callback(nil) + if m := file.Readiness(wmask) & wmask; m != 0 { + epi.Callback(nil, m) } // Add epi to file.epolls so that it is removed when the last @@ -274,8 +274,8 @@ func (ep *EpollInstance) ModifyInterest(file *FileDescription, num int32, event file.EventRegister(&epi.waiter, wmask) // Check if the file is already ready with the new mask. - if file.Readiness(wmask)&wmask != 0 { - epi.Callback(nil) + if m := file.Readiness(wmask) & wmask; m != 0 { + epi.Callback(nil, m) } return nil @@ -311,7 +311,7 @@ func (ep *EpollInstance) DeleteInterest(file *FileDescription, num int32) error } // Callback implements waiter.EntryCallback.Callback. -func (epi *epollInterest) Callback(*waiter.Entry) { +func (epi *epollInterest) Callback(*waiter.Entry, waiter.EventMask) { newReady := false epi.epoll.mu.Lock() if !epi.ready { diff --git a/pkg/sentry/vfs/file_description.go b/pkg/sentry/vfs/file_description.go index 51fddbaff..2153382e9 100644 --- a/pkg/sentry/vfs/file_description.go +++ b/pkg/sentry/vfs/file_description.go @@ -43,7 +43,7 @@ import ( type FileDescription struct { FileDescriptionRefs - // flagsMu protects statusFlags and asyncHandler below. + // flagsMu protects `statusFlags`, `saved`, and `asyncHandler` below. flagsMu sync.Mutex `state:"nosave"` // statusFlags contains status flags, "initialized by open(2) and possibly @@ -52,6 +52,11 @@ type FileDescription struct { // access to asyncHandler. statusFlags uint32 + // saved is true after beforeSave is called. This is used to prevent + // double-unregistration of asyncHandler. This does not work properly for + // save-resume, which is not currently supported in gVisor (see b/26588733). + saved bool `state:"nosave"` + // asyncHandler handles O_ASYNC signal generation. It is set with the // F_SETOWN or F_SETOWN_EX fcntls. For asyncHandler to be used, O_ASYNC must // also be set by fcntl(2). @@ -184,7 +189,7 @@ func (fd *FileDescription) DecRef(ctx context.Context) { } fd.vd.DecRef(ctx) fd.flagsMu.Lock() - if fd.statusFlags&linux.O_ASYNC != 0 && fd.asyncHandler != nil { + if !fd.saved && fd.statusFlags&linux.O_ASYNC != 0 && fd.asyncHandler != nil { fd.asyncHandler.Unregister(fd) } fd.asyncHandler = nil diff --git a/pkg/sentry/vfs/save_restore.go b/pkg/sentry/vfs/save_restore.go index 7723ed643..8f070ed53 100644 --- a/pkg/sentry/vfs/save_restore.go +++ b/pkg/sentry/vfs/save_restore.go @@ -18,8 +18,10 @@ import ( "fmt" "sync/atomic" + "gvisor.dev/gvisor/pkg/abi/linux" "gvisor.dev/gvisor/pkg/context" "gvisor.dev/gvisor/pkg/refsvfs2" + "gvisor.dev/gvisor/pkg/waiter" ) // FilesystemImplSaveRestoreExtension is an optional extension to @@ -120,5 +122,20 @@ func (mnt *Mount) afterLoad() { func (epi *epollInterest) afterLoad() { // Mark all epollInterests as ready after restore so that the next call to // EpollInstance.ReadEvents() rechecks their readiness. - epi.Callback(nil) + epi.Callback(nil, waiter.EventMaskFromLinux(epi.mask)) +} + +// beforeSave is called by stateify. +func (fd *FileDescription) beforeSave() { + fd.saved = true + if fd.statusFlags&linux.O_ASYNC != 0 && fd.asyncHandler != nil { + fd.asyncHandler.Unregister(fd) + } +} + +// afterLoad is called by stateify. +func (fd *FileDescription) afterLoad() { + if fd.statusFlags&linux.O_ASYNC != 0 && fd.asyncHandler != nil { + fd.asyncHandler.Register(fd) + } } diff --git a/pkg/waiter/waiter.go b/pkg/waiter/waiter.go index 08519d986..83d4f893a 100644 --- a/pkg/waiter/waiter.go +++ b/pkg/waiter/waiter.go @@ -119,7 +119,10 @@ type EntryCallback interface { // The callback is supposed to perform minimal work, and cannot call // any method on the queue itself because it will be locked while the // callback is running. - Callback(e *Entry) + // + // The mask indicates the events that occurred and that the entry is + // interested in. + Callback(e *Entry, mask EventMask) } // Entry represents a waiter that can be add to the a wait queue. It can @@ -140,7 +143,7 @@ type channelCallback struct { } // Callback implements EntryCallback.Callback. -func (c *channelCallback) Callback(*Entry) { +func (c *channelCallback) Callback(*Entry, EventMask) { select { case c.ch <- struct{}{}: default: @@ -193,8 +196,8 @@ func (q *Queue) EventUnregister(e *Entry) { func (q *Queue) Notify(mask EventMask) { q.mu.RLock() for e := q.list.Front(); e != nil; e = e.Next() { - if mask&e.mask != 0 { - e.Callback.Callback(e) + if m := mask & e.mask; m != 0 { + e.Callback.Callback(e, m) } } q.mu.RUnlock() diff --git a/pkg/waiter/waiter_test.go b/pkg/waiter/waiter_test.go index c1b94a4f3..6928f28b4 100644 --- a/pkg/waiter/waiter_test.go +++ b/pkg/waiter/waiter_test.go @@ -20,12 +20,12 @@ import ( ) type callbackStub struct { - f func(e *Entry) + f func(e *Entry, m EventMask) } // Callback implements EntryCallback.Callback. -func (c *callbackStub) Callback(e *Entry) { - c.f(e) +func (c *callbackStub) Callback(e *Entry, m EventMask) { + c.f(e, m) } func TestEmptyQueue(t *testing.T) { @@ -36,7 +36,7 @@ func TestEmptyQueue(t *testing.T) { // Register then unregister a waiter, then notify the queue. cnt := 0 - e := Entry{Callback: &callbackStub{func(*Entry) { cnt++ }}} + e := Entry{Callback: &callbackStub{func(*Entry, EventMask) { cnt++ }}} q.EventRegister(&e, EventIn) q.EventUnregister(&e) q.Notify(EventIn) @@ -49,7 +49,7 @@ func TestMask(t *testing.T) { // Register a waiter. var q Queue var cnt int - e := Entry{Callback: &callbackStub{func(*Entry) { cnt++ }}} + e := Entry{Callback: &callbackStub{func(*Entry, EventMask) { cnt++ }}} q.EventRegister(&e, EventIn|EventErr) // Notify with an overlapping mask. @@ -101,11 +101,14 @@ func TestConcurrentRegistration(t *testing.T) { for i := 0; i < concurrency; i++ { go func() { var e Entry - e.Callback = &callbackStub{func(entry *Entry) { + e.Callback = &callbackStub{func(entry *Entry, mask EventMask) { cnt++ if entry != &e { t.Errorf("entry = %p, want %p", entry, &e) } + if mask != EventIn { + t.Errorf("mask = %#x want %#x", mask, EventIn) + } }} // Wait for notification, then register. @@ -158,11 +161,14 @@ func TestConcurrentNotification(t *testing.T) { // Register waiters. for i := 0; i < waiterCount; i++ { var e Entry - e.Callback = &callbackStub{func(entry *Entry) { + e.Callback = &callbackStub{func(entry *Entry, mask EventMask) { atomic.AddInt32(&cnt, 1) if entry != &e { t.Errorf("entry = %p, want %p", entry, &e) } + if mask != EventIn { + t.Errorf("mask = %#x want %#x", mask, EventIn) + } }} q.EventRegister(&e, EventIn|EventErr) diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index 946d06cd6..dec8c8100 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -799,8 +799,8 @@ cc_binary( deps = [ ":socket_test_util", "//test/util:cleanup", - "//test/util:epoll_util", "//test/util:eventfd_util", + "//test/util:file_descriptor", "//test/util:fs_util", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/flags:flag", @@ -811,6 +811,7 @@ cc_binary( "//test/util:multiprocess_util", "//test/util:posix_error", "//test/util:save_util", + "//test/util:signal_util", "//test/util:temp_path", "//test/util:test_util", "//test/util:thread_util", diff --git a/test/syscalls/linux/fcntl.cc b/test/syscalls/linux/fcntl.cc index 34016d4bd..4b581045b 100644 --- a/test/syscalls/linux/fcntl.cc +++ b/test/syscalls/linux/fcntl.cc @@ -14,10 +14,13 @@ #include <fcntl.h> #include <signal.h> +#include <sys/epoll.h> #include <sys/types.h> #include <syscall.h> #include <unistd.h> +#include <atomic> +#include <deque> #include <iostream> #include <list> #include <string> @@ -34,25 +37,27 @@ #include "test/syscalls/linux/socket_test_util.h" #include "test/util/cleanup.h" #include "test/util/eventfd_util.h" +#include "test/util/file_descriptor.h" #include "test/util/fs_util.h" #include "test/util/multiprocess_util.h" #include "test/util/posix_error.h" #include "test/util/save_util.h" +#include "test/util/signal_util.h" #include "test/util/temp_path.h" #include "test/util/test_util.h" #include "test/util/thread_util.h" #include "test/util/timer_util.h" -ABSL_FLAG(std::string, child_setlock_on, "", +ABSL_FLAG(std::string, child_set_lock_on, "", "Contains the path to try to set a file lock on."); -ABSL_FLAG(bool, child_setlock_write, false, +ABSL_FLAG(bool, child_set_lock_write, false, "Whether to set a writable lock (otherwise readable)"); ABSL_FLAG(bool, blocking, false, "Whether to set a blocking lock (otherwise non-blocking)."); ABSL_FLAG(bool, retry_eintr, false, "Whether to retry in the subprocess on EINTR."); -ABSL_FLAG(uint64_t, child_setlock_start, 0, "The value of struct flock start"); -ABSL_FLAG(uint64_t, child_setlock_len, 0, "The value of struct flock len"); +ABSL_FLAG(uint64_t, child_set_lock_start, 0, "The value of struct flock start"); +ABSL_FLAG(uint64_t, child_set_lock_len, 0, "The value of struct flock len"); ABSL_FLAG(int32_t, socket_fd, -1, "A socket to use for communicating more state back " "to the parent."); @@ -60,6 +65,11 @@ ABSL_FLAG(int32_t, socket_fd, -1, namespace gvisor { namespace testing { +std::function<void(int, siginfo_t*, void*)> setsig_signal_handle; +void setsig_signal_handler(int signum, siginfo_t* siginfo, void* ucontext) { + setsig_signal_handle(signum, siginfo, ucontext); +} + class FcntlLockTest : public ::testing::Test { public: void SetUp() override { @@ -84,18 +94,93 @@ class FcntlLockTest : public ::testing::Test { int fds_[2] = {}; }; +struct SignalDelivery { + int num; + siginfo_t info; +}; + +class FcntlSignalTest : public ::testing::Test { + public: + void SetUp() override { + int pipe_fds[2]; + ASSERT_THAT(pipe2(pipe_fds, O_NONBLOCK), SyscallSucceeds()); + pipe_read_fd_ = pipe_fds[0]; + pipe_write_fd_ = pipe_fds[1]; + } + + PosixErrorOr<Cleanup> RegisterSignalHandler(int signum) { + struct sigaction handler; + handler.sa_sigaction = setsig_signal_handler; + setsig_signal_handle = [&](int signum, siginfo_t* siginfo, + void* unused_ucontext) { + SignalDelivery sig; + sig.num = signum; + sig.info = *siginfo; + signals_received_.push_back(sig); + num_signals_received_++; + }; + sigemptyset(&handler.sa_mask); + handler.sa_flags = SA_SIGINFO; + return ScopedSigaction(signum, handler); + } + + void FlushAndCloseFD(int fd) { + char buf; + int read_bytes; + do { + read_bytes = read(fd, &buf, 1); + } while (read_bytes > 0); + // read() can also fail with EWOULDBLOCK since the pipe is open in + // non-blocking mode. This is not an error. + EXPECT_TRUE(read_bytes == 0 || (read_bytes == -1 && errno == EWOULDBLOCK)); + EXPECT_THAT(close(fd), SyscallSucceeds()); + } + + void DupReadFD() { + ASSERT_THAT(pipe_read_fd_dup_ = dup(pipe_read_fd_), SyscallSucceeds()); + max_expected_signals++; + } + + void RegisterFD(int fd, int signum) { + ASSERT_THAT(fcntl(fd, F_SETOWN, getpid()), SyscallSucceeds()); + ASSERT_THAT(fcntl(fd, F_SETSIG, signum), SyscallSucceeds()); + int old_flags; + ASSERT_THAT(old_flags = fcntl(fd, F_GETFL), SyscallSucceeds()); + ASSERT_THAT(fcntl(fd, F_SETFL, old_flags | O_ASYNC), SyscallSucceeds()); + } + + void GenerateIOEvent() { + ASSERT_THAT(write(pipe_write_fd_, "test", 4), SyscallSucceedsWithValue(4)); + } + + void WaitForSignalDelivery(absl::Duration timeout) { + absl::Time wait_start = absl::Now(); + while (num_signals_received_ < max_expected_signals && + absl::Now() - wait_start < timeout) { + absl::SleepFor(absl::Milliseconds(10)); + } + } + + int pipe_read_fd_ = -1; + int pipe_read_fd_dup_ = -1; + int pipe_write_fd_ = -1; + int max_expected_signals = 1; + std::deque<SignalDelivery> signals_received_; + std::atomic<int> num_signals_received_ = 0; +}; + namespace { PosixErrorOr<Cleanup> SubprocessLock(std::string const& path, bool for_write, bool blocking, bool retry_eintr, int fd, off_t start, off_t length, pid_t* child) { std::vector<std::string> args = { - "/proc/self/exe", "--child_setlock_on", path, - "--child_setlock_start", absl::StrCat(start), "--child_setlock_len", - absl::StrCat(length), "--socket_fd", absl::StrCat(fd)}; + "/proc/self/exe", "--child_set_lock_on", path, + "--child_set_lock_start", absl::StrCat(start), "--child_set_lock_len", + absl::StrCat(length), "--socket_fd", absl::StrCat(fd)}; if (for_write) { - args.push_back("--child_setlock_write"); + args.push_back("--child_set_lock_write"); } if (blocking) { @@ -965,7 +1050,6 @@ TEST(FcntlTest, GetOwnNone) { // into F_{GET,SET}OWN_EX. EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN), SyscallSucceedsWithValue(0)); - MaybeSave(); } TEST(FcntlTest, GetOwnExNone) { @@ -1009,7 +1093,6 @@ TEST(FcntlTest, SetOwnPid) { EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN), SyscallSucceedsWithValue(pid)); - MaybeSave(); } TEST(FcntlTest, SetOwnPgrp) { @@ -1030,7 +1113,6 @@ TEST(FcntlTest, SetOwnPgrp) { SyscallSucceedsWithValue(0)); EXPECT_EQ(got_owner.type, F_OWNER_PGRP); EXPECT_EQ(got_owner.pid, pgid); - MaybeSave(); } TEST(FcntlTest, SetOwnUnset) { @@ -1058,7 +1140,6 @@ TEST(FcntlTest, SetOwnUnset) { EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN), SyscallSucceedsWithValue(0)); - MaybeSave(); } // F_SETOWN flips the sign of negative values, an operation that is guarded @@ -1130,7 +1211,6 @@ TEST(FcntlTest, SetOwnExTid) { EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN), SyscallSucceedsWithValue(owner.pid)); - MaybeSave(); } TEST(FcntlTest, SetOwnExPid) { @@ -1146,7 +1226,6 @@ TEST(FcntlTest, SetOwnExPid) { EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN), SyscallSucceedsWithValue(owner.pid)); - MaybeSave(); } TEST(FcntlTest, SetOwnExPgrp) { @@ -1168,7 +1247,6 @@ TEST(FcntlTest, SetOwnExPgrp) { SyscallSucceedsWithValue(0)); EXPECT_EQ(got_owner.type, set_owner.type); EXPECT_EQ(got_owner.pid, set_owner.pid); - MaybeSave(); } TEST(FcntlTest, SetOwnExUnset) { @@ -1201,7 +1279,6 @@ TEST(FcntlTest, SetOwnExUnset) { EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETOWN), SyscallSucceedsWithValue(0)); - MaybeSave(); } TEST(FcntlTest, GetOwnExTid) { @@ -1258,9 +1335,269 @@ TEST(FcntlTest, GetOwnExPgrp) { EXPECT_EQ(got_owner.pid, set_owner.pid); } +TEST(FcntlTest, SetSig) { + FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE( + Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0)); + + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETSIG, SIGUSR1), + SyscallSucceedsWithValue(0)); + EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETSIG), + SyscallSucceedsWithValue(SIGUSR1)); +} + +TEST(FcntlTest, SetSigDefaultsToZero) { + FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE( + Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0)); + + // Defaults to returning the zero value, indicating default behavior (SIGIO). + EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETSIG), + SyscallSucceedsWithValue(0)); +} + +TEST(FcntlTest, SetSigToDefault) { + FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE( + Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0)); + + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETSIG, SIGIO), + SyscallSucceedsWithValue(0)); + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_GETSIG), + SyscallSucceedsWithValue(SIGIO)); + + // Can be reset to the default behavior. + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETSIG, 0), + SyscallSucceedsWithValue(0)); + EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETSIG), + SyscallSucceedsWithValue(0)); +} + +TEST(FcntlTest, SetSigInvalid) { + FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE( + Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0)); + + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETSIG, SIGRTMAX + 1), + SyscallFailsWithErrno(EINVAL)); + EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETSIG), + SyscallSucceedsWithValue(0)); +} + +TEST(FcntlTest, SetSigInvalidDoesNotResetPreviousChoice) { + FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE( + Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0)); + + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETSIG, SIGUSR1), + SyscallSucceedsWithValue(0)); + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETSIG, SIGRTMAX + 1), + SyscallFailsWithErrno(EINVAL)); + EXPECT_THAT(syscall(__NR_fcntl, s.get(), F_GETSIG), + SyscallSucceedsWithValue(SIGUSR1)); +} + +TEST_F(FcntlSignalTest, SetSigDefault) { + const auto signal_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGIO)); + RegisterFD(pipe_read_fd_, 0); // Zero = default behavior + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + signals_received_.pop_front(); + EXPECT_EQ(sig.num, SIGIO); + EXPECT_EQ(sig.info.si_signo, SIGIO); + // siginfo contents is undefined in this case. +} + +TEST_F(FcntlSignalTest, SetSigCustom) { + const auto signal_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + RegisterFD(pipe_read_fd_, SIGUSR1); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + signals_received_.pop_front(); + EXPECT_EQ(sig.num, SIGUSR1); + EXPECT_EQ(sig.info.si_signo, SIGUSR1); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigUnregisterStillGetsSigio) { + const auto sigio_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGIO)); + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + RegisterFD(pipe_read_fd_, SIGUSR1); + RegisterFD(pipe_read_fd_, 0); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + signals_received_.pop_front(); + EXPECT_EQ(sig.num, SIGIO); + // siginfo contents is undefined in this case. +} + +TEST_F(FcntlSignalTest, SetSigWithSigioStillGetsSiginfo) { + const auto signal_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGIO)); + RegisterFD(pipe_read_fd_, SIGIO); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + EXPECT_EQ(sig.num, SIGIO); + EXPECT_EQ(sig.info.si_signo, SIGIO); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigDupThenCloseOld) { + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + RegisterFD(pipe_read_fd_, SIGUSR1); + DupReadFD(); + FlushAndCloseFD(pipe_read_fd_); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with the **old** FD (even though it is closed). + EXPECT_EQ(sig.num, SIGUSR1); + EXPECT_EQ(sig.info.si_signo, SIGUSR1); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigDupThenCloseNew) { + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + RegisterFD(pipe_read_fd_, SIGUSR1); + DupReadFD(); + FlushAndCloseFD(pipe_read_fd_dup_); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with the old FD. + EXPECT_EQ(sig.num, SIGUSR1); + EXPECT_EQ(sig.info.si_signo, SIGUSR1); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigDupOldRegistered) { + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + RegisterFD(pipe_read_fd_, SIGUSR1); + DupReadFD(); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with the old FD. + EXPECT_EQ(sig.num, SIGUSR1); + EXPECT_EQ(sig.info.si_signo, SIGUSR1); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigDupNewRegistered) { + const auto sigusr2_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR2)); + DupReadFD(); + RegisterFD(pipe_read_fd_dup_, SIGUSR2); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with the new FD. + EXPECT_EQ(sig.num, SIGUSR2); + EXPECT_EQ(sig.info.si_signo, SIGUSR2); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_dup_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigDupBothRegistered) { + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + const auto sigusr2_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR2)); + RegisterFD(pipe_read_fd_, SIGUSR1); + DupReadFD(); + RegisterFD(pipe_read_fd_dup_, SIGUSR2); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with the **new** signal number, but the **old** FD. + EXPECT_EQ(sig.num, SIGUSR2); + EXPECT_EQ(sig.info.si_signo, SIGUSR2); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigDupBothRegisteredAfterDup) { + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + const auto sigusr2_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR2)); + DupReadFD(); + RegisterFD(pipe_read_fd_, SIGUSR1); + RegisterFD(pipe_read_fd_dup_, SIGUSR2); + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with the **new** signal number, but the **old** FD. + EXPECT_EQ(sig.num, SIGUSR2); + EXPECT_EQ(sig.info.si_signo, SIGUSR2); + EXPECT_EQ(sig.info.si_fd, pipe_read_fd_); + EXPECT_EQ(sig.info.si_band, EPOLLIN | EPOLLRDNORM); +} + +TEST_F(FcntlSignalTest, SetSigDupUnregisterOld) { + const auto sigio_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGIO)); + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + const auto sigusr2_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR2)); + RegisterFD(pipe_read_fd_, SIGUSR1); + DupReadFD(); + RegisterFD(pipe_read_fd_dup_, SIGUSR2); + RegisterFD(pipe_read_fd_, 0); // Should go back to SIGIO behavior. + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with SIGIO. + EXPECT_EQ(sig.num, SIGIO); + // siginfo is undefined in this case. +} + +TEST_F(FcntlSignalTest, SetSigDupUnregisterNew) { + const auto sigio_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGIO)); + const auto sigusr1_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR1)); + const auto sigusr2_cleanup = + ASSERT_NO_ERRNO_AND_VALUE(RegisterSignalHandler(SIGUSR2)); + RegisterFD(pipe_read_fd_, SIGUSR1); + DupReadFD(); + RegisterFD(pipe_read_fd_dup_, SIGUSR2); + RegisterFD(pipe_read_fd_dup_, 0); // Should go back to SIGIO behavior. + GenerateIOEvent(); + WaitForSignalDelivery(absl::Seconds(1)); + ASSERT_EQ(num_signals_received_, 1); + SignalDelivery sig = signals_received_.front(); + // We get a signal with SIGIO. + EXPECT_EQ(sig.num, SIGIO); + // siginfo is undefined in this case. +} + // Make sure that making multiple concurrent changes to async signal generation // does not cause any race issues. -TEST(FcntlTest, SetFlSetOwnDoNotRace) { +TEST(FcntlTest, SetFlSetOwnSetSigDoNotRace) { FileDescriptor s = ASSERT_NO_ERRNO_AND_VALUE( Socket(AF_UNIX, SOCK_SEQPACKET | SOCK_NONBLOCK | SOCK_CLOEXEC, 0)); @@ -1268,32 +1605,40 @@ TEST(FcntlTest, SetFlSetOwnDoNotRace) { EXPECT_THAT(pid = getpid(), SyscallSucceeds()); constexpr absl::Duration runtime = absl::Milliseconds(300); - auto setAsync = [&s, &runtime] { + auto set_async = [&s, &runtime] { for (auto start = absl::Now(); absl::Now() - start < runtime;) { ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETFL, O_ASYNC), SyscallSucceeds()); sched_yield(); } }; - auto resetAsync = [&s, &runtime] { + auto reset_async = [&s, &runtime] { for (auto start = absl::Now(); absl::Now() - start < runtime;) { ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETFL, 0), SyscallSucceeds()); sched_yield(); } }; - auto setOwn = [&s, &pid, &runtime] { + auto set_own = [&s, &pid, &runtime] { for (auto start = absl::Now(); absl::Now() - start < runtime;) { ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETOWN, pid), SyscallSucceeds()); sched_yield(); } }; + auto set_sig = [&s, &runtime] { + for (auto start = absl::Now(); absl::Now() - start < runtime;) { + ASSERT_THAT(syscall(__NR_fcntl, s.get(), F_SETSIG, SIGUSR1), + SyscallSucceeds()); + sched_yield(); + } + }; std::list<ScopedThread> threads; for (int i = 0; i < 10; i++) { - threads.emplace_back(setAsync); - threads.emplace_back(resetAsync); - threads.emplace_back(setOwn); + threads.emplace_back(set_async); + threads.emplace_back(reset_async); + threads.emplace_back(set_own); + threads.emplace_back(set_sig); } } @@ -1302,57 +1647,60 @@ TEST(FcntlTest, SetFlSetOwnDoNotRace) { } // namespace testing } // namespace gvisor -int main(int argc, char** argv) { - gvisor::testing::TestInit(&argc, &argv); - - const std::string setlock_on = absl::GetFlag(FLAGS_child_setlock_on); - if (!setlock_on.empty()) { - int socket_fd = absl::GetFlag(FLAGS_socket_fd); - int fd = open(setlock_on.c_str(), O_RDWR, 0666); - if (fd == -1 && errno != 0) { - int err = errno; - std::cerr << "CHILD open " << setlock_on << " failed " << err - << std::endl; - exit(err); - } +int set_lock() { + const std::string set_lock_on = absl::GetFlag(FLAGS_child_set_lock_on); + int socket_fd = absl::GetFlag(FLAGS_socket_fd); + int fd = open(set_lock_on.c_str(), O_RDWR, 0666); + if (fd == -1 && errno != 0) { + int err = errno; + std::cerr << "CHILD open " << set_lock_on << " failed: " << err + << std::endl; + return err; + } - struct flock fl; - if (absl::GetFlag(FLAGS_child_setlock_write)) { - fl.l_type = F_WRLCK; - } else { - fl.l_type = F_RDLCK; - } - fl.l_whence = SEEK_SET; - fl.l_start = absl::GetFlag(FLAGS_child_setlock_start); - fl.l_len = absl::GetFlag(FLAGS_child_setlock_len); + struct flock fl; + if (absl::GetFlag(FLAGS_child_set_lock_write)) { + fl.l_type = F_WRLCK; + } else { + fl.l_type = F_RDLCK; + } + fl.l_whence = SEEK_SET; + fl.l_start = absl::GetFlag(FLAGS_child_set_lock_start); + fl.l_len = absl::GetFlag(FLAGS_child_set_lock_len); + + // Test the fcntl. + int err = 0; + int ret = 0; + + gvisor::testing::MonotonicTimer timer; + timer.Start(); + do { + ret = fcntl(fd, absl::GetFlag(FLAGS_blocking) ? F_SETLKW : F_SETLK, &fl); + } while (absl::GetFlag(FLAGS_retry_eintr) && ret == -1 && errno == EINTR); + auto usec = absl::ToInt64Microseconds(timer.Duration()); + + if (ret == -1 && errno != 0) { + err = errno; + std::cerr << "CHILD lock " << set_lock_on << " failed " << err << std::endl; + } - // Test the fcntl. - int err = 0; - int ret = 0; + // If there is a socket fd let's send back the time in microseconds it took + // to execute this syscall. + if (socket_fd != -1) { + gvisor::testing::WriteFd(socket_fd, reinterpret_cast<void*>(&usec), + sizeof(usec)); + close(socket_fd); + } - gvisor::testing::MonotonicTimer timer; - timer.Start(); - do { - ret = fcntl(fd, absl::GetFlag(FLAGS_blocking) ? F_SETLKW : F_SETLK, &fl); - } while (absl::GetFlag(FLAGS_retry_eintr) && ret == -1 && errno == EINTR); - auto usec = absl::ToInt64Microseconds(timer.Duration()); - - if (ret == -1 && errno != 0) { - err = errno; - std::cerr << "CHILD lock " << setlock_on << " failed " << err - << std::endl; - } + close(fd); + return err; +} - // If there is a socket fd let's send back the time in microseconds it took - // to execute this syscall. - if (socket_fd != -1) { - gvisor::testing::WriteFd(socket_fd, reinterpret_cast<void*>(&usec), - sizeof(usec)); - close(socket_fd); - } +int main(int argc, char** argv) { + gvisor::testing::TestInit(&argc, &argv); - close(fd); - exit(err); + if (!absl::GetFlag(FLAGS_child_set_lock_on).empty()) { + exit(set_lock()); } return gvisor::testing::RunAllTests(); |