Skip to content

Commit

Permalink
Don't eat error from epoll_ctl EPOLL_CTL_ADD
Browse files Browse the repository at this point in the history
Docker maps stdin to `/dev/null` which doesn't support epoll. Host FD
was ignoring the error and suceeding the epoll_ctl call from the
container, giving false impressing that epoll would be notified.

This required plumbing failure to all waiter.Waitable.EventRegister
callers and implementers.

Closes #6795

PiperOrigin-RevId: 414797621
  • Loading branch information
fvoznika authored and gvisor-bot committed Dec 7, 2021
1 parent d62190f commit 9768009
Show file tree
Hide file tree
Showing 54 changed files with 358 additions and 140 deletions.
29 changes: 28 additions & 1 deletion images/basic/integrationtest/host_fd.c
Expand Up @@ -13,17 +13,44 @@
// limitations under the License.

#include <err.h>
#include <errno.h>
#include <sys/epoll.h>
#include <sys/ioctl.h>
#include <unistd.h>

// Tests that FIONREAD is supported with host FD.
int main(int argc, char** argv) {
void testFionread() {
int size = 0;
if (ioctl(STDOUT_FILENO, FIONREAD, &size) < 0) {
err(1, "ioctl(stdin, FIONREAD)");
}
if (size != 0) {
err(1, "FIONREAD wrong size, want: 0, got: %d", size);
}
}

// Docker maps stdin to /dev/null which doesn't support epoll. Check that error
// is correctly propagated.
void testEpoll() {
int fd = epoll_create(1);
if (fd < 0) {
err(1, "epoll_create");
}

struct epoll_event event;
event.events = EPOLLIN;
event.data.u64 = 123;
int res = epoll_ctl(fd, EPOLL_CTL_ADD, 0, &event);
if (res != -1) {
err(1, "epoll_ctl(EPOLL_CTL_ADD, stdin) should have failed");
}
if (errno != EPERM) {
err(1, "epoll_ctl(EPOLL_CTL_ADD, stdin) should have returned EPERM");
}
}

int main(int argc, char** argv) {
testFionread();
testEpoll();
return 0;
}
3 changes: 2 additions & 1 deletion pkg/sentry/devices/tundev/tundev.go
Expand Up @@ -152,8 +152,9 @@ func (fd *tunFD) Readiness(mask waiter.EventMask) waiter.EventMask {
}

// EventRegister implements watier.Waitable.EventRegister.
func (fd *tunFD) EventRegister(e *waiter.Entry) {
func (fd *tunFD) EventRegister(e *waiter.Entry) error {
fd.device.EventRegister(e)
return nil
}

// EventUnregister implements watier.Waitable.EventUnregister.
Expand Down
3 changes: 2 additions & 1 deletion pkg/sentry/fs/dev/net_tun.go
Expand Up @@ -158,8 +158,9 @@ func (n *netTunFileOperations) Readiness(mask waiter.EventMask) waiter.EventMask
}

// EventRegister implements watier.Waitable.EventRegister.
func (n *netTunFileOperations) EventRegister(e *waiter.Entry) {
func (n *netTunFileOperations) EventRegister(e *waiter.Entry) error {
n.device.EventRegister(e)
return nil
}

// EventUnregister implements watier.Waitable.EventUnregister.
Expand Down
13 changes: 10 additions & 3 deletions pkg/sentry/fs/fdpipe/pipe.go
Expand Up @@ -16,6 +16,7 @@
package fdpipe

import (
"fmt"
"os"

"golang.org/x/sys/unix"
Expand Down Expand Up @@ -99,15 +100,21 @@ func (p *pipeOperations) init() error {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (p *pipeOperations) EventRegister(e *waiter.Entry) {
func (p *pipeOperations) EventRegister(e *waiter.Entry) error {
p.Queue.EventRegister(e)
fdnotifier.UpdateFD(int32(p.file.FD()))
if err := fdnotifier.UpdateFD(int32(p.file.FD())); err != nil {
p.Queue.EventUnregister(e)
return err
}
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
func (p *pipeOperations) EventUnregister(e *waiter.Entry) {
p.Queue.EventUnregister(e)
fdnotifier.UpdateFD(int32(p.file.FD()))
if err := fdnotifier.UpdateFD(int32(p.file.FD())); err != nil {
panic(fmt.Sprint("UpdateFD:", err))
}
}

// Readiness returns a mask of ready events for stream.
Expand Down
17 changes: 10 additions & 7 deletions pkg/sentry/fs/file.go
Expand Up @@ -159,7 +159,8 @@ func (f *File) SetFlags(newFlags SettableFileFlags) {
f.flags.Append = newFlags.Append
if f.async != nil {
if newFlags.Async && !f.flags.Async {
f.async.Register(f)
// Ignore error given that VFS1 will not be here much longer.
_ = f.async.Register(f)
}
if !newFlags.Async && f.flags.Async {
f.async.Unregister(f)
Expand All @@ -180,8 +181,8 @@ func (f *File) Readiness(mask waiter.EventMask) waiter.EventMask {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (f *File) EventRegister(e *waiter.Entry) {
f.FileOperations.EventRegister(e)
func (f *File) EventRegister(e *waiter.Entry) error {
return f.FileOperations.EventRegister(e)
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand Down Expand Up @@ -424,23 +425,25 @@ func (f *File) Msync(ctx context.Context, mr memmap.MappableRange) error {

// A FileAsync sends signals to its owner when w is ready for IO.
type FileAsync interface {
Register(w waiter.Waitable)
Register(w waiter.Waitable) error
Unregister(w waiter.Waitable)
}

// Async gets the stored FileAsync or creates a new one with the supplied
// function. If the supplied function is nil, no FileAsync is created and the
// current value is returned.
func (f *File) Async(newAsync func() FileAsync) FileAsync {
func (f *File) Async(newAsync func() FileAsync) (FileAsync, error) {
f.flagsMu.Lock()
defer f.flagsMu.Unlock()
if f.async == nil && newAsync != nil {
f.async = newAsync()
if f.flags.Async {
f.async.Register(f)
if err := f.async.Register(f); err != nil {
return nil, err
}
}
}
return f.async
return f.async, nil
}

// lockedReader implements io.Reader and io.ReaderAt.
Expand Down
7 changes: 3 additions & 4 deletions pkg/sentry/fs/file_overlay.go
Expand Up @@ -100,14 +100,13 @@ func (f *overlayFileOperations) Release(ctx context.Context) {
}

// EventRegister implements FileOperations.EventRegister.
func (f *overlayFileOperations) EventRegister(we *waiter.Entry) {
func (f *overlayFileOperations) EventRegister(we *waiter.Entry) error {
f.upperMu.Lock()
defer f.upperMu.Unlock()
if f.upper != nil {
f.upper.EventRegister(we)
return
return f.upper.EventRegister(we)
}
f.lower.EventRegister(we)
return f.lower.EventRegister(we)
}

// EventUnregister implements FileOperations.Unregister.
Expand Down
6 changes: 5 additions & 1 deletion pkg/sentry/fs/file_state.go
Expand Up @@ -14,6 +14,8 @@

package fs

import "fmt"

// beforeSave is invoked by stateify.
func (f *File) beforeSave() {
f.saving = true
Expand All @@ -25,6 +27,8 @@ func (f *File) beforeSave() {
// afterLoad is invoked by stateify.
func (f *File) afterLoad() {
if f.flags.Async && f.async != nil {
f.async.Register(f)
if err := f.async.Register(f); err != nil {
panic(fmt.Sprint("async.Register:", err))
}
}
}
12 changes: 9 additions & 3 deletions pkg/sentry/fs/host/file.go
Expand Up @@ -149,15 +149,21 @@ func newFile(ctx context.Context, dirent *fs.Dirent, flags fs.FileFlags, iops *i
}

// EventRegister implements waiter.Waitable.EventRegister.
func (f *fileOperations) EventRegister(e *waiter.Entry) {
func (f *fileOperations) EventRegister(e *waiter.Entry) error {
f.iops.fileState.queue.EventRegister(e)
fdnotifier.UpdateFD(int32(f.iops.fileState.FD()))
if err := fdnotifier.UpdateFD(int32(f.iops.fileState.FD())); err != nil {
f.iops.fileState.queue.EventUnregister(e)
return err
}
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
func (f *fileOperations) EventUnregister(e *waiter.Entry) {
f.iops.fileState.queue.EventUnregister(e)
fdnotifier.UpdateFD(int32(f.iops.fileState.FD()))
if err := fdnotifier.UpdateFD(int32(f.iops.fileState.FD())); err != nil {
panic(fmt.Sprint("UpdateFD:", err))
}
}

// Readiness uses the poll() syscall to check the status of the underlying FD.
Expand Down
7 changes: 5 additions & 2 deletions pkg/sentry/fs/host/socket.go
Expand Up @@ -259,12 +259,15 @@ func (c *ConnectedEndpoint) GetLocalAddress() (tcpip.FullAddress, tcpip.Error) {
}

// EventUpdate implements transport.ConnectedEndpoint.EventUpdate.
func (c *ConnectedEndpoint) EventUpdate() {
func (c *ConnectedEndpoint) EventUpdate() error {
c.mu.RLock()
defer c.mu.RUnlock()
if c.file.FD() != -1 {
fdnotifier.UpdateFD(int32(c.file.FD()))
if err := fdnotifier.UpdateFD(int32(c.file.FD())); err != nil {
return err
}
}
return nil
}

// Recv implements transport.Receiver.Recv.
Expand Down
6 changes: 6 additions & 0 deletions pkg/sentry/fs/inotify.go
Expand Up @@ -351,3 +351,9 @@ func (i *Inotify) RmWatch(ctx context.Context, wd int32) error {

return nil
}

// EventRegister implements waiter.Waitable.
func (i *Inotify) EventRegister(e *waiter.Entry) error {
i.Queue.EventRegister(e)
return nil
}
3 changes: 2 additions & 1 deletion pkg/sentry/fs/timerfd/timerfd.go
Expand Up @@ -108,8 +108,9 @@ func (t *TimerOperations) Readiness(mask waiter.EventMask) waiter.EventMask {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (t *TimerOperations) EventRegister(e *waiter.Entry) {
func (t *TimerOperations) EventRegister(e *waiter.Entry) error {
t.events.EventRegister(e)
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand Down
3 changes: 2 additions & 1 deletion pkg/sentry/fs/tty/master.go
Expand Up @@ -128,8 +128,9 @@ func (mf *masterFileOperations) Release(ctx context.Context) {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (mf *masterFileOperations) EventRegister(e *waiter.Entry) {
func (mf *masterFileOperations) EventRegister(e *waiter.Entry) error {
mf.t.ld.masterWaiter.EventRegister(e)
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand Down
3 changes: 2 additions & 1 deletion pkg/sentry/fs/tty/replica.go
Expand Up @@ -113,8 +113,9 @@ func (sf *replicaFileOperations) Release(context.Context) {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (sf *replicaFileOperations) EventRegister(e *waiter.Entry) {
func (sf *replicaFileOperations) EventRegister(e *waiter.Entry) error {
sf.si.t.ld.replicaWaiter.EventRegister(e)
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand Down
3 changes: 2 additions & 1 deletion pkg/sentry/fsimpl/devpts/master.go
Expand Up @@ -103,8 +103,9 @@ func (mfd *masterFileDescription) Release(ctx context.Context) {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (mfd *masterFileDescription) EventRegister(e *waiter.Entry) {
func (mfd *masterFileDescription) EventRegister(e *waiter.Entry) error {
mfd.t.ld.masterWaiter.EventRegister(e)
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand Down
3 changes: 2 additions & 1 deletion pkg/sentry/fsimpl/devpts/replica.go
Expand Up @@ -112,8 +112,9 @@ var _ vfs.FileDescriptionImpl = (*replicaFileDescription)(nil)
func (rfd *replicaFileDescription) Release(ctx context.Context) {}

// EventRegister implements waiter.Waitable.EventRegister.
func (rfd *replicaFileDescription) EventRegister(e *waiter.Entry) {
func (rfd *replicaFileDescription) EventRegister(e *waiter.Entry) error {
rfd.inode.t.ld.replicaWaiter.EventRegister(e)
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand Down
13 changes: 10 additions & 3 deletions pkg/sentry/fsimpl/eventfd/eventfd.go
Expand Up @@ -16,6 +16,7 @@
package eventfd

import (
"fmt"
"math"
"sync"

Expand Down Expand Up @@ -266,14 +267,18 @@ func (efd *EventFileDescription) Readiness(mask waiter.EventMask) waiter.EventMa
}

// EventRegister implements waiter.Waitable.EventRegister.
func (efd *EventFileDescription) EventRegister(entry *waiter.Entry) {
func (efd *EventFileDescription) EventRegister(entry *waiter.Entry) error {
efd.queue.EventRegister(entry)

efd.mu.Lock()
defer efd.mu.Unlock()
if efd.hostfd >= 0 {
fdnotifier.UpdateFD(int32(efd.hostfd))
if err := fdnotifier.UpdateFD(int32(efd.hostfd)); err != nil {
efd.queue.EventUnregister(entry)
return err
}
}
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand All @@ -283,6 +288,8 @@ func (efd *EventFileDescription) EventUnregister(entry *waiter.Entry) {
efd.mu.Lock()
defer efd.mu.Unlock()
if efd.hostfd >= 0 {
fdnotifier.UpdateFD(int32(efd.hostfd))
if err := fdnotifier.UpdateFD(int32(efd.hostfd)); err != nil {
panic(fmt.Sprint("UpdateFD:", err))
}
}
}
4 changes: 3 additions & 1 deletion pkg/sentry/fsimpl/eventfd/eventfd_test.go
Expand Up @@ -49,7 +49,9 @@ func TestEventFD(t *testing.T) {

// Register a callback for a write event.
w, ch := waiter.NewChannelEntry(waiter.ReadableEvents)
eventfd.EventRegister(&w)
if err := eventfd.EventRegister(&w); err != nil {
t.Fatalf("EventRegister(): %v", err)
}
defer eventfd.EventUnregister(&w)

data := []byte("00000124")
Expand Down
3 changes: 2 additions & 1 deletion pkg/sentry/fsimpl/fuse/dev.go
Expand Up @@ -378,8 +378,9 @@ func (fd *DeviceFD) readinessLocked(mask waiter.EventMask) waiter.EventMask {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (fd *DeviceFD) EventRegister(e *waiter.Entry) {
func (fd *DeviceFD) EventRegister(e *waiter.Entry) error {
fd.waitQueue.EventRegister(e)
return nil
}

// EventUnregister implements waiter.Waitable.EventUnregister.
Expand Down
16 changes: 11 additions & 5 deletions pkg/sentry/fsimpl/gofer/special_file.go
Expand Up @@ -15,6 +15,7 @@
package gofer

import (
"fmt"
"sync/atomic"

"golang.org/x/sys/unix"
Expand Down Expand Up @@ -165,20 +166,25 @@ func (fd *specialFileFD) Readiness(mask waiter.EventMask) waiter.EventMask {
}

// EventRegister implements waiter.Waitable.EventRegister.
func (fd *specialFileFD) EventRegister(e *waiter.Entry) {
func (fd *specialFileFD) EventRegister(e *waiter.Entry) error {
if fd.haveQueue {
fd.queue.EventRegister(e)
fdnotifier.UpdateFD(fd.handle.fd)
return
if err := fdnotifier.UpdateFD(fd.handle.fd); err != nil {
fd.queue.EventUnregister(e)
return err
}
return nil
}
fd.fileDescription.EventRegister(e)
return fd.fileDescription.EventRegister(e)
}

// EventUnregister implements waiter.Waitable.EventUnregister.
func (fd *specialFileFD) EventUnregister(e *waiter.Entry) {
if fd.haveQueue {
fd.queue.EventUnregister(e)
fdnotifier.UpdateFD(fd.handle.fd)
if err := fdnotifier.UpdateFD(fd.handle.fd); err != nil {
panic(fmt.Sprint("UpdateFD:", err))
}
return
}
fd.fileDescription.EventUnregister(e)
Expand Down

0 comments on commit 9768009

Please sign in to comment.