diff options
author | Fabricio Voznika <fvoznika@google.com> | 2019-01-25 17:22:04 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2019-01-25 17:23:07 -0800 |
commit | 55e8eb775b422a7485d6d1dc4f8e4c8fd32096da (patch) | |
tree | 7dff5a374ff29f6bffb3c543c0a37b1657f36ea0 | |
parent | c6facd0358ae61849786dbbc0f4f5a07a25cb6f1 (diff) |
Make cacheRemoteRevalidating detect changes to file size
When file size changes outside the sandbox, page cache was not
refreshing file size which is required for cacheRemoteRevalidating.
In fact, cacheRemoteRevalidating should be skipping the cache
completely since it's not really benefiting from it. The cache is
cache is already bypassed for unstable attributes (see
cachePolicy.cacheUAttrs). And althought the cache is called to
map pages, they will always miss the cache and map directly from
the host.
Created a HostMappable struct that maps directly to the host and
use it for files with cacheRemoteRevalidating.
Closes #124
PiperOrigin-RevId: 230998440
Change-Id: Ic5f632eabe33b47241e05e98c95e9b2090ae08fc
-rw-r--r-- | pkg/sentry/fs/fsutil/BUILD | 2 | ||||
-rw-r--r-- | pkg/sentry/fs/fsutil/host_mappable.go | 136 | ||||
-rw-r--r-- | pkg/sentry/fs/fsutil/host_mappable_state.go | 22 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/cache_policy.go | 22 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/file.go | 9 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/inode.go | 30 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/path.go | 2 | ||||
-rw-r--r-- | pkg/sentry/fs/gofer/session.go | 14 | ||||
-rw-r--r-- | runsc/container/BUILD | 1 | ||||
-rw-r--r-- | runsc/container/container_test.go | 137 | ||||
-rw-r--r-- | runsc/container/shared_volume_test.go | 267 |
11 files changed, 486 insertions, 156 deletions
diff --git a/pkg/sentry/fs/fsutil/BUILD b/pkg/sentry/fs/fsutil/BUILD index 4965e1a5f..d4767642b 100644 --- a/pkg/sentry/fs/fsutil/BUILD +++ b/pkg/sentry/fs/fsutil/BUILD @@ -70,6 +70,8 @@ go_library( "host_file_mapper.go", "host_file_mapper_state.go", "host_file_mapper_unsafe.go", + "host_mappable.go", + "host_mappable_state.go", "inode.go", "inode_cached.go", ], diff --git a/pkg/sentry/fs/fsutil/host_mappable.go b/pkg/sentry/fs/fsutil/host_mappable.go new file mode 100644 index 000000000..4e4bcf4a4 --- /dev/null +++ b/pkg/sentry/fs/fsutil/host_mappable.go @@ -0,0 +1,136 @@ +// Copyright 2019 Google LLC +// +// 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 fsutil + +import ( + "sync" + + "gvisor.googlesource.com/gvisor/pkg/sentry/context" + "gvisor.googlesource.com/gvisor/pkg/sentry/memmap" + "gvisor.googlesource.com/gvisor/pkg/sentry/platform" + "gvisor.googlesource.com/gvisor/pkg/sentry/safemem" + "gvisor.googlesource.com/gvisor/pkg/sentry/usermem" +) + +// HostMappable implements memmap.Mappable and platform.File over an arbitrary +// host file descriptor. +// +// +stateify savable +type HostMappable struct { + hostFileMapper *HostFileMapper + + mu sync.Mutex `state:"nosave"` + + // fd is the file descriptor to the host. Protected by mu. + fd int `state:"nosave"` + + // mappings tracks mappings of the cached file object into + // memmap.MappingSpaces so it can invalidated upon save. Protected by mu. + mappings memmap.MappingSet +} + +// NewHostMappable creates a new mappable that maps directly to host FD. +func NewHostMappable() *HostMappable { + return &HostMappable{ + hostFileMapper: NewHostFileMapper(), + fd: -1, + } +} + +func (h *HostMappable) getFD() int { + h.mu.Lock() + defer h.mu.Unlock() + if h.fd < 0 { + panic("HostMappable FD isn't set") + } + return h.fd +} + +// UpdateFD sets the host FD iff FD hasn't been set before or if there are +// no mappings. +func (h *HostMappable) UpdateFD(fd int) { + h.mu.Lock() + defer h.mu.Unlock() + h.fd = fd +} + +// AddMapping implements memmap.Mappable.AddMapping. +func (h *HostMappable) AddMapping(ctx context.Context, ms memmap.MappingSpace, ar usermem.AddrRange, offset uint64, writable bool) error { + // Hot path. Avoid defers. + h.mu.Lock() + mapped := h.mappings.AddMapping(ms, ar, offset, writable) + for _, r := range mapped { + h.hostFileMapper.IncRefOn(r) + } + h.mu.Unlock() + return nil +} + +// RemoveMapping implements memmap.Mappable.RemoveMapping. +func (h *HostMappable) RemoveMapping(ctx context.Context, ms memmap.MappingSpace, ar usermem.AddrRange, offset uint64, writable bool) { + // Hot path. Avoid defers. + h.mu.Lock() + unmapped := h.mappings.RemoveMapping(ms, ar, offset, writable) + for _, r := range unmapped { + h.hostFileMapper.DecRefOn(r) + } + h.mu.Unlock() +} + +// CopyMapping implements memmap.Mappable.CopyMapping. +func (h *HostMappable) CopyMapping(ctx context.Context, ms memmap.MappingSpace, srcAR, dstAR usermem.AddrRange, offset uint64, writable bool) error { + return h.AddMapping(ctx, ms, dstAR, offset, writable) +} + +// Translate implements memmap.Mappable.Translate. +func (h *HostMappable) Translate(ctx context.Context, required, optional memmap.MappableRange, at usermem.AccessType) ([]memmap.Translation, error) { + return []memmap.Translation{ + { + Source: optional, + File: h, + Offset: optional.Start, + }, + }, nil +} + +// InvalidateUnsavable implements memmap.Mappable.InvalidateUnsavable. +func (h *HostMappable) InvalidateUnsavable(ctx context.Context) error { + h.mu.Lock() + h.mappings.InvalidateAll(memmap.InvalidateOpts{}) + h.mu.Unlock() + return nil +} + +// MapInto implements platform.File.MapInto. +func (h *HostMappable) MapInto(as platform.AddressSpace, addr usermem.Addr, fr platform.FileRange, at usermem.AccessType, precommit bool) error { + return as.MapFile(addr, h.getFD(), fr, at, precommit) +} + +// MapInternal implements platform.File.MapInternal. +func (h *HostMappable) MapInternal(fr platform.FileRange, at usermem.AccessType) (safemem.BlockSeq, error) { + return h.hostFileMapper.MapInternal(fr, h.getFD(), at.Write) +} + +// IncRef implements platform.File.IncRef. +func (h *HostMappable) IncRef(fr platform.FileRange) { + mr := memmap.MappableRange{Start: fr.Start, End: fr.End} + h.hostFileMapper.IncRefOn(mr) +} + +// DecRef implements platform.File.DecRef. +func (h *HostMappable) DecRef(fr platform.FileRange) { + mr := memmap.MappableRange{Start: fr.Start, End: fr.End} + h.hostFileMapper.DecRefOn(mr) +} diff --git a/pkg/sentry/fs/fsutil/host_mappable_state.go b/pkg/sentry/fs/fsutil/host_mappable_state.go new file mode 100644 index 000000000..765f1ec87 --- /dev/null +++ b/pkg/sentry/fs/fsutil/host_mappable_state.go @@ -0,0 +1,22 @@ +// Copyright 2019 Google LLC +// +// 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 fsutil + +// afterLoad is invoked by stateify. +func (h *HostMappable) afterLoad() { + h.mu.Lock() + defer h.mu.Unlock() + h.fd = -1 +} diff --git a/pkg/sentry/fs/gofer/cache_policy.go b/pkg/sentry/fs/gofer/cache_policy.go index 3d380f0e8..507d6900f 100644 --- a/pkg/sentry/fs/gofer/cache_policy.go +++ b/pkg/sentry/fs/gofer/cache_policy.go @@ -90,17 +90,29 @@ func (cp cachePolicy) cacheReaddir() bool { return cp == cacheAll || cp == cacheAllWritethrough } -// usePageCache determines whether the page cache should be used for the given -// inode. If the remote filesystem donates host FDs to the sentry, then the -// host kernel's page cache will be used, otherwise we will use a +// useCachingInodeOps determines whether the page cache should be used for the +// given inode. If the remote filesystem donates host FDs to the sentry, then +// the host kernel's page cache will be used, otherwise we will use a // sentry-internal page cache. -func (cp cachePolicy) usePageCache(inode *fs.Inode) bool { +func (cp cachePolicy) useCachingInodeOps(inode *fs.Inode) bool { // Do cached IO for regular files only. Some "character devices" expect // no caching. if !fs.IsFile(inode.StableAttr) { return false } - return cp == cacheAll || cp == cacheAllWritethrough || cp == cacheRemoteRevalidating + return cp == cacheAll || cp == cacheAllWritethrough +} + +// cacheHandles determine whether handles need to be cached with the given +// inode. Handles must be cached when inode can be mapped into memory to +// implement InodeOperations.Mappable with stable handles. +func (cp cachePolicy) cacheHandles(inode *fs.Inode) bool { + // Do cached IO for regular files only. Some "character devices" expect + // no caching. + if !fs.IsFile(inode.StableAttr) { + return false + } + return cp.useCachingInodeOps(inode) || cp == cacheRemoteRevalidating } // writeThough indicates whether writes to the file should be synced to the diff --git a/pkg/sentry/fs/gofer/file.go b/pkg/sentry/fs/gofer/file.go index 3578b07a0..2181ddc68 100644 --- a/pkg/sentry/fs/gofer/file.go +++ b/pkg/sentry/fs/gofer/file.go @@ -204,7 +204,7 @@ func (f *fileOperations) Write(ctx context.Context, file *fs.File, src usermem.I return 0, syserror.EISDIR } cp := f.inodeOperations.session().cachePolicy - if cp.usePageCache(file.Dirent.Inode) { + if cp.useCachingInodeOps(file.Dirent.Inode) { n, err := f.inodeOperations.cachingInodeOps.Write(ctx, src, offset) if err != nil { return n, err @@ -225,7 +225,7 @@ func (f *fileOperations) Read(ctx context.Context, file *fs.File, dst usermem.IO return 0, syserror.EISDIR } - if f.inodeOperations.session().cachePolicy.usePageCache(file.Dirent.Inode) { + if f.inodeOperations.session().cachePolicy.useCachingInodeOps(file.Dirent.Inode) { return f.inodeOperations.cachingInodeOps.Read(ctx, file, dst, offset) } return dst.CopyOutFrom(ctx, f.handles.readWriterAt(ctx, offset)) @@ -267,10 +267,7 @@ func (f *fileOperations) Flush(ctx context.Context, file *fs.File) error { // ConfigureMMap implements fs.FileOperations.ConfigureMMap. func (f *fileOperations) ConfigureMMap(ctx context.Context, file *fs.File, opts *memmap.MMapOpts) error { - if !f.inodeOperations.session().cachePolicy.usePageCache(file.Dirent.Inode) { - return syserror.ENODEV - } - return fsutil.GenericConfigureMMap(file, f.inodeOperations.cachingInodeOps, opts) + return f.inodeOperations.configureMMap(file, opts) } // Seek implements fs.FileOperations.Seek. diff --git a/pkg/sentry/fs/gofer/inode.go b/pkg/sentry/fs/gofer/inode.go index f0dc99fd0..043705c58 100644 --- a/pkg/sentry/fs/gofer/inode.go +++ b/pkg/sentry/fs/gofer/inode.go @@ -125,6 +125,10 @@ type inodeFileState struct { // failures. S/R is transparent to Sentry and the latter will continue // using its cached values after restore. savedUAttr *fs.UnstableAttr + + // hostMappable is created when using 'cacheRemoteRevalidating' to map pages + // directly from host. + hostMappable *fsutil.HostMappable } // Release releases file handles. @@ -166,6 +170,9 @@ func (i *inodeFileState) setHandlesForCachedIO(flags fs.FileFlags, h *handles) { i.writebackRW = true } } + if i.hostMappable != nil { + i.hostMappable.UpdateFD(i.fdLocked()) + } } // getCachedHandles returns any cached handles which would accelerate @@ -287,7 +294,10 @@ func (i *inodeFileState) Sync(ctx context.Context) error { func (i *inodeFileState) FD() int { i.handlesMu.RLock() defer i.handlesMu.RUnlock() + return i.fdLocked() +} +func (i *inodeFileState) fdLocked() int { // Assert that the file was actually opened. if i.writeback == nil && i.readthrough == nil { panic("cannot get host FD for a file that was never opened") @@ -344,9 +354,13 @@ func (i *inodeOperations) Release(ctx context.Context) { // Mappable implements fs.InodeOperations.Mappable. func (i *inodeOperations) Mappable(inode *fs.Inode) memmap.Mappable { - if i.session().cachePolicy.usePageCache(inode) { + if i.session().cachePolicy.useCachingInodeOps(inode) { return i.cachingInodeOps } + // This check is necessary because it's returning an interface type. + if i.fileState.hostMappable != nil { + return i.fileState.hostMappable + } return nil } @@ -434,7 +448,7 @@ func (i *inodeOperations) NonBlockingOpen(ctx context.Context, p fs.PermMask) (* } func (i *inodeOperations) getFileDefault(ctx context.Context, d *fs.Dirent, flags fs.FileFlags) (*fs.File, error) { - if !i.session().cachePolicy.usePageCache(d.Inode) { + if !i.session().cachePolicy.cacheHandles(d.Inode) { h, err := newHandles(ctx, i.fileState.file, flags) if err != nil { return nil, err @@ -503,7 +517,7 @@ func (i *inodeOperations) SetTimestamps(ctx context.Context, inode *fs.Inode, ts // Truncate implements fs.InodeOperations.Truncate. func (i *inodeOperations) Truncate(ctx context.Context, inode *fs.Inode, length int64) error { // This can only be called for files anyway. - if i.session().cachePolicy.usePageCache(inode) { + if i.session().cachePolicy.useCachingInodeOps(inode) { return i.cachingInodeOps.Truncate(ctx, inode, length) } @@ -561,6 +575,16 @@ func (i *inodeOperations) StatFS(ctx context.Context) (fs.Info, error) { return info, nil } +func (i *inodeOperations) configureMMap(file *fs.File, opts *memmap.MMapOpts) error { + if i.session().cachePolicy.useCachingInodeOps(file.Dirent.Inode) { + return fsutil.GenericConfigureMMap(file, i.cachingInodeOps, opts) + } + if i.fileState.hostMappable != nil { + return fsutil.GenericConfigureMMap(file, i.fileState.hostMappable, opts) + } + return syserror.ENODEV +} + func init() { syserror.AddErrorUnwrapper(func(err error) (syscall.Errno, bool) { if _, ok := err.(p9.ErrSocket); ok { diff --git a/pkg/sentry/fs/gofer/path.go b/pkg/sentry/fs/gofer/path.go index a324dc990..faedfb81c 100644 --- a/pkg/sentry/fs/gofer/path.go +++ b/pkg/sentry/fs/gofer/path.go @@ -128,7 +128,7 @@ func (i *inodeOperations) Create(ctx context.Context, dir *fs.Inode, name string File: newFile, Host: hostFile, } - if iops.session().cachePolicy.usePageCache(d.Inode) { + if iops.session().cachePolicy.cacheHandles(d.Inode) { iops.fileState.setHandlesForCachedIO(flags, h) } return NewFile(ctx, d, name, flags, iops, h), nil diff --git a/pkg/sentry/fs/gofer/session.go b/pkg/sentry/fs/gofer/session.go index f76a83cd9..b5b1c8202 100644 --- a/pkg/sentry/fs/gofer/session.go +++ b/pkg/sentry/fs/gofer/session.go @@ -197,11 +197,17 @@ func newInodeOperations(ctx context.Context, s *session, file contextFile, qid p } } + var hm *fsutil.HostMappable + if s.cachePolicy == cacheRemoteRevalidating && fs.IsFile(sattr) { + hm = fsutil.NewHostMappable() + } + fileState := &inodeFileState{ - s: s, - file: file, - sattr: sattr, - key: deviceKey, + s: s, + file: file, + sattr: sattr, + key: deviceKey, + hostMappable: hm, } uattr := unstable(ctx, valid, attr, s.mounter, s.client) diff --git a/runsc/container/BUILD b/runsc/container/BUILD index 5dfff5c5e..354ce2661 100644 --- a/runsc/container/BUILD +++ b/runsc/container/BUILD @@ -36,6 +36,7 @@ go_test( "container_test.go", "fs_test.go", "multi_container_test.go", + "shared_volume_test.go", ], data = [ ":test_app", diff --git a/runsc/container/container_test.go b/runsc/container/container_test.go index 9f3d6b454..06a25de6d 100644 --- a/runsc/container/container_test.go +++ b/runsc/container/container_test.go @@ -1353,143 +1353,6 @@ func TestAbbreviatedIDs(t *testing.T) { } } -// Check that modifications to a volume mount are propigated into and out of -// the sandbox. -func TestContainerVolumeContentsShared(t *testing.T) { - // Only run this test with shared file access, since that is the only - // behavior it is testing. - conf := testutil.TestConfig() - conf.FileAccess = boot.FileAccessShared - t.Logf("Running test with conf: %+v", conf) - - // Main process just sleeps. We will use "exec" to probe the state of - // the filesystem. - spec := testutil.NewSpecWithArgs("sleep", "1000") - - dir, err := ioutil.TempDir(testutil.TmpDir(), "root-fs-test") - if err != nil { - t.Fatalf("TempDir failed: %v", err) - } - - rootDir, bundleDir, err := testutil.SetupContainer(spec, conf) - if err != nil { - t.Fatalf("error setting up container: %v", err) - } - defer os.RemoveAll(rootDir) - defer os.RemoveAll(bundleDir) - - // Create and start the container. - c, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "", "") - if err != nil { - t.Fatalf("error creating container: %v", err) - } - defer c.Destroy() - if err := c.Start(conf); err != nil { - t.Fatalf("error starting container: %v", err) - } - - // File that will be used to check consistency inside/outside sandbox. - filename := filepath.Join(dir, "file") - - // File does not exist yet. Reading from the sandbox should fail. - argsTestFile := &control.ExecArgs{ - Filename: "/usr/bin/test", - Argv: []string{"test", "-f", filename}, - } - if ws, err := c.executeSync(argsTestFile); err != nil { - t.Fatalf("unexpected error testing file %q: %v", filename, err) - } else if ws.ExitStatus() == 0 { - t.Errorf("test %q exited with code %v, wanted not zero", ws.ExitStatus(), err) - } - - // Create the file from outside of the sandbox. - if err := ioutil.WriteFile(filename, []byte("foobar"), 0777); err != nil { - t.Fatalf("error writing to file %q: %v", filename, err) - } - - // Now we should be able to test the file from within the sandbox. - if ws, err := c.executeSync(argsTestFile); err != nil { - t.Fatalf("unexpected error testing file %q: %v", filename, err) - } else if ws.ExitStatus() != 0 { - t.Errorf("test %q exited with code %v, wanted zero", filename, ws.ExitStatus()) - } - - // Rename the file from outside of the sandbox. - newFilename := filepath.Join(dir, "newfile") - if err := os.Rename(filename, newFilename); err != nil { - t.Fatalf("os.Rename(%q, %q) failed: %v", filename, newFilename, err) - } - - // File should no longer exist at the old path within the sandbox. - if ws, err := c.executeSync(argsTestFile); err != nil { - t.Fatalf("unexpected error testing file %q: %v", filename, err) - } else if ws.ExitStatus() == 0 { - t.Errorf("test %q exited with code %v, wanted not zero", filename, ws.ExitStatus()) - } - - // We should be able to test the new filename from within the sandbox. - argsTestNewFile := &control.ExecArgs{ - Filename: "/usr/bin/test", - Argv: []string{"test", "-f", newFilename}, - } - if ws, err := c.executeSync(argsTestNewFile); err != nil { - t.Fatalf("unexpected error testing file %q: %v", newFilename, err) - } else if ws.ExitStatus() != 0 { - t.Errorf("test %q exited with code %v, wanted zero", newFilename, ws.ExitStatus()) - } - - // Delete the renamed file from outside of the sandbox. - if err := os.Remove(newFilename); err != nil { - t.Fatalf("error removing file %q: %v", filename, err) - } - - // Renamed file should no longer exist at the old path within the sandbox. - if ws, err := c.executeSync(argsTestNewFile); err != nil { - t.Fatalf("unexpected error testing file %q: %v", newFilename, err) - } else if ws.ExitStatus() == 0 { - t.Errorf("test %q exited with code %v, wanted not zero", newFilename, ws.ExitStatus()) - } - - // Now create the file from WITHIN the sandbox. - argsTouch := &control.ExecArgs{ - Filename: "/usr/bin/touch", - Argv: []string{"touch", filename}, - KUID: auth.KUID(os.Getuid()), - KGID: auth.KGID(os.Getgid()), - } - if ws, err := c.executeSync(argsTouch); err != nil { - t.Fatalf("unexpected error touching file %q: %v", filename, err) - } else if ws.ExitStatus() != 0 { - t.Errorf("touch %q exited with code %v, wanted zero", filename, ws.ExitStatus()) - } - - // File should exist outside the sandbox. - if _, err := os.Stat(filename); err != nil { - t.Errorf("stat %q got error %v, wanted nil", filename, err) - } - - // File should exist outside the sandbox. - if _, err := os.Stat(filename); err != nil { - t.Errorf("stat %q got error %v, wanted nil", filename, err) - } - - // Delete the file from within the sandbox. - argsRemove := &control.ExecArgs{ - Filename: "/bin/rm", - Argv: []string{"rm", filename}, - } - if ws, err := c.executeSync(argsRemove); err != nil { - t.Fatalf("unexpected error removing file %q: %v", filename, err) - } else if ws.ExitStatus() != 0 { - t.Errorf("remove %q exited with code %v, wanted zero", filename, ws.ExitStatus()) - } - - // File should not exist outside the sandbox. - if _, err := os.Stat(filename); !os.IsNotExist(err) { - t.Errorf("stat %q got error %v, wanted ErrNotExist", filename, err) - } -} - func TestGoferExits(t *testing.T) { spec := testutil.NewSpecWithArgs("/bin/sleep", "10000") conf := testutil.TestConfig() diff --git a/runsc/container/shared_volume_test.go b/runsc/container/shared_volume_test.go new file mode 100644 index 000000000..8f81ed630 --- /dev/null +++ b/runsc/container/shared_volume_test.go @@ -0,0 +1,267 @@ +// Copyright 2019 Google LLC +// +// 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 container + +import ( + "bytes" + "fmt" + "io/ioutil" + "os" + "path/filepath" + "testing" + + "gvisor.googlesource.com/gvisor/pkg/sentry/control" + "gvisor.googlesource.com/gvisor/pkg/sentry/kernel/auth" + "gvisor.googlesource.com/gvisor/runsc/boot" + "gvisor.googlesource.com/gvisor/runsc/test/testutil" +) + +// TestSharedVolume checks that modifications to a volume mount are propagated +// into and out of the sandbox. +func TestSharedVolume(t *testing.T) { + conf := testutil.TestConfig() + conf.FileAccess = boot.FileAccessShared + t.Logf("Running test with conf: %+v", conf) + + // Main process just sleeps. We will use "exec" to probe the state of + // the filesystem. + spec := testutil.NewSpecWithArgs("sleep", "1000") + + dir, err := ioutil.TempDir(testutil.TmpDir(), "shared-volume-test") + if err != nil { + t.Fatalf("TempDir failed: %v", err) + } + + rootDir, bundleDir, err := testutil.SetupContainer(spec, conf) + if err != nil { + t.Fatalf("error setting up container: %v", err) + } + defer os.RemoveAll(rootDir) + defer os.RemoveAll(bundleDir) + + // Create and start the container. + c, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "", "") + if err != nil { + t.Fatalf("error creating container: %v", err) + } + defer c.Destroy() + if err := c.Start(conf); err != nil { + t.Fatalf("error starting container: %v", err) + } + + // File that will be used to check consistency inside/outside sandbox. + filename := filepath.Join(dir, "file") + + // File does not exist yet. Reading from the sandbox should fail. + argsTestFile := &control.ExecArgs{ + Filename: "/usr/bin/test", + Argv: []string{"test", "-f", filename}, + } + if ws, err := c.executeSync(argsTestFile); err != nil { + t.Fatalf("unexpected error testing file %q: %v", filename, err) + } else if ws.ExitStatus() == 0 { + t.Errorf("test %q exited with code %v, wanted not zero", ws.ExitStatus(), err) + } + + // Create the file from outside of the sandbox. + if err := ioutil.WriteFile(filename, []byte("foobar"), 0777); err != nil { + t.Fatalf("error writing to file %q: %v", filename, err) + } + + // Now we should be able to test the file from within the sandbox. + if ws, err := c.executeSync(argsTestFile); err != nil { + t.Fatalf("unexpected error testing file %q: %v", filename, err) + } else if ws.ExitStatus() != 0 { + t.Errorf("test %q exited with code %v, wanted zero", filename, ws.ExitStatus()) + } + + // Rename the file from outside of the sandbox. + newFilename := filepath.Join(dir, "newfile") + if err := os.Rename(filename, newFilename); err != nil { + t.Fatalf("os.Rename(%q, %q) failed: %v", filename, newFilename, err) + } + + // File should no longer exist at the old path within the sandbox. + if ws, err := c.executeSync(argsTestFile); err != nil { + t.Fatalf("unexpected error testing file %q: %v", filename, err) + } else if ws.ExitStatus() == 0 { + t.Errorf("test %q exited with code %v, wanted not zero", filename, ws.ExitStatus()) + } + + // We should be able to test the new filename from within the sandbox. + argsTestNewFile := &control.ExecArgs{ + Filename: "/usr/bin/test", + Argv: []string{"test", "-f", newFilename}, + } + if ws, err := c.executeSync(argsTestNewFile); err != nil { + t.Fatalf("unexpected error testing file %q: %v", newFilename, err) + } else if ws.ExitStatus() != 0 { + t.Errorf("test %q exited with code %v, wanted zero", newFilename, ws.ExitStatus()) + } + + // Delete the renamed file from outside of the sandbox. + if err := os.Remove(newFilename); err != nil { + t.Fatalf("error removing file %q: %v", filename, err) + } + + // Renamed file should no longer exist at the old path within the sandbox. + if ws, err := c.executeSync(argsTestNewFile); err != nil { + t.Fatalf("unexpected error testing file %q: %v", newFilename, err) + } else if ws.ExitStatus() == 0 { + t.Errorf("test %q exited with code %v, wanted not zero", newFilename, ws.ExitStatus()) + } + + // Now create the file from WITHIN the sandbox. + argsTouch := &control.ExecArgs{ + Filename: "/usr/bin/touch", + Argv: []string{"touch", filename}, + KUID: auth.KUID(os.Getuid()), + KGID: auth.KGID(os.Getgid()), + } + if ws, err := c.executeSync(argsTouch); err != nil { + t.Fatalf("unexpected error touching file %q: %v", filename, err) + } else if ws.ExitStatus() != 0 { + t.Errorf("touch %q exited with code %v, wanted zero", filename, ws.ExitStatus()) + } + + // File should exist outside the sandbox. + if _, err := os.Stat(filename); err != nil { + t.Errorf("stat %q got error %v, wanted nil", filename, err) + } + + // File should exist outside the sandbox. + if _, err := os.Stat(filename); err != nil { + t.Errorf("stat %q got error %v, wanted nil", filename, err) + } + + // Delete the file from within the sandbox. + argsRemove := &control.ExecArgs{ + Filename: "/bin/rm", + Argv: []string{"rm", filename}, + } + if ws, err := c.executeSync(argsRemove); err != nil { + t.Fatalf("unexpected error removing file %q: %v", filename, err) + } else if ws.ExitStatus() != 0 { + t.Errorf("remove %q exited with code %v, wanted zero", filename, ws.ExitStatus()) + } + + // File should not exist outside the sandbox. + if _, err := os.Stat(filename); !os.IsNotExist(err) { + t.Errorf("stat %q got error %v, wanted ErrNotExist", filename, err) + } +} + +func checkFile(c *Container, filename string, want []byte) error { + cpy := filename + ".copy" + argsCp := &control.ExecArgs{ + Filename: "/bin/cp", + Argv: []string{"cp", "-f", filename, cpy}, + } + if _, err := c.executeSync(argsCp); err != nil { + return fmt.Errorf("unexpected error copying file %q to %q: %v", filename, cpy, err) + } + got, err := ioutil.ReadFile(cpy) + if err != nil { + return fmt.Errorf("Error reading file %q: %v", filename, err) + } + if !bytes.Equal(got, want) { + return fmt.Errorf("file content inside the sandbox is wrong, got: %q, want: %q", got, want) + } + return nil +} + +// TestSharedVolumeFile tests that changes to file content outside the sandbox +// is reflected inside. +func TestSharedVolumeFile(t *testing.T) { + conf := testutil.TestConfig() + conf.FileAccess = boot.FileAccessShared + t.Logf("Running test with conf: %+v", conf) + + // Main process just sleeps. We will use "exec" to probe the state of + // the filesystem. + spec := testutil.NewSpecWithArgs("sleep", "1000") + + dir, err := ioutil.TempDir(testutil.TmpDir(), "shared-volume-test") + if err != nil { + t.Fatalf("TempDir failed: %v", err) + } + + rootDir, bundleDir, err := testutil.SetupContainer(spec, conf) + if err != nil { + t.Fatalf("error setting up container: %v", err) + } + defer os.RemoveAll(rootDir) + defer os.RemoveAll(bundleDir) + + // Create and start the container. + c, err := Create(testutil.UniqueContainerID(), spec, conf, bundleDir, "", "", "") + if err != nil { + t.Fatalf("error creating container: %v", err) + } + defer c.Destroy() + if err := c.Start(conf); err != nil { + t.Fatalf("error starting container: %v", err) + } + + // File that will be used to check consistency inside/outside sandbox. + filename := filepath.Join(dir, "file") + + // Write file from outside the container and check that the same content is + // read inside. + want := []byte("host-") + if err := ioutil.WriteFile(filename, []byte(want), 0666); err != nil { + t.Fatalf("Error writing to %q: %v", filename, err) + } + if err := checkFile(c, filename, want); err != nil { + t.Fatal(err.Error()) + } + + // Append to file inside the container and check that content is not lost. + argsAppend := &control.ExecArgs{ + Filename: "/bin/bash", + Argv: []string{"bash", "-c", "echo -n sandbox- >> " + filename}, + } + if _, err := c.executeSync(argsAppend); err != nil { + t.Fatalf("unexpected error appending file %q: %v", filename, err) + } + want = []byte("host-sandbox-") + if err := checkFile(c, filename, want); err != nil { + t.Fatal(err.Error()) + } + + // Write again from outside the container and check that the same content is + // read inside. + f, err := os.OpenFile(filename, os.O_APPEND|os.O_WRONLY, 0) + if err != nil { + t.Fatalf("Error openning file %q: %v", filename, err) + } + defer f.Close() + if _, err := f.Write([]byte("host")); err != nil { + t.Fatalf("Error writing to file %q: %v", filename, err) + } + want = []byte("host-sandbox-host") + if err := checkFile(c, filename, want); err != nil { + t.Fatal(err.Error()) + } + + // Shrink file outside and check that the same content is read inside. + if err := f.Truncate(5); err != nil { + t.Fatalf("Error truncating file %q: %v", filename, err) + } + want = want[:5] + if err := checkFile(c, filename, want); err != nil { + t.Fatal(err.Error()) + } +} |