Filesystems shouldn't be saving references to Platform.

Platform objects are not savable, storing references to them in
filesystem datastructures would cause save to fail if someone actually
passed in a Platform.

Current implementations work because everywhere a Platform is
expected, we currently pass in a Kernel object which embeds Platform
and thus satisfies the interface.

Eliminate this indirection and save pointers to Kernel directly.

PiperOrigin-RevId: 225288336
Change-Id: Ica399ff43f425e15bc150a0d7102196c3d54a2ab
This commit is contained in:
Rahat Mahmood
2018-12-12 17:47:55 -08:00
committed by Shentubot
parent f93c288dd7
commit ccce1d4281
16 changed files with 116 additions and 44 deletions
+24 -8
View File
@@ -31,23 +31,30 @@ import (
// Context returns a Context that may be used in tests. Uses ptrace as the
// platform.Platform.
//
// Note that some filesystems may require a minimal kernel for testing, which
// this test context does not provide. For such tests, see kernel/contexttest.
func Context(tb testing.TB) context.Context {
p, err := ptrace.New()
if err != nil {
tb.Fatal(err)
}
// Test usage of context.Background is fine.
return &testContext{
Context: context.Background(),
l: limits.NewLimitSet(),
platform: p,
return &TestContext{
Context: context.Background(),
l: limits.NewLimitSet(),
platform: p,
otherValues: make(map[interface{}]interface{}),
}
}
type testContext struct {
// TestContext represents a context with minimal functionality suitable for
// running tests.
type TestContext struct {
context.Context
l *limits.LimitSet
platform platform.Platform
l *limits.LimitSet
platform platform.Platform
otherValues map[interface{}]interface{}
}
// globalUniqueID tracks incremental unique identifiers for tests.
@@ -76,8 +83,14 @@ func (hostClock) Now() ktime.Time {
return ktime.FromNanoseconds(time.Now().UnixNano())
}
// RegisterValue registers additional values with this test context. Useful for
// providing values from external packages that contexttest can't depend on.
func (t *TestContext) RegisterValue(key, value interface{}) {
t.otherValues[key] = value
}
// Value implements context.Context.
func (t *testContext) Value(key interface{}) interface{} {
func (t *TestContext) Value(key interface{}) interface{} {
switch key {
case limits.CtxLimits:
return t.l
@@ -92,6 +105,9 @@ func (t *testContext) Value(key interface{}) interface{} {
case ktime.CtxRealtimeClock:
return hostClock{}
default:
if val, ok := t.otherValues[key]; ok {
return val
}
return t.Context.Value(key)
}
}
+1 -1
View File
@@ -94,9 +94,9 @@ go_test(
deps = [
":fs",
"//pkg/sentry/context",
"//pkg/sentry/context/contexttest",
"//pkg/sentry/fs/ramfs/test",
"//pkg/sentry/fs/tmpfs",
"//pkg/sentry/kernel/contexttest",
"//pkg/sentry/usermem",
"//pkg/syserror",
],
+4 -4
View File
@@ -23,8 +23,8 @@ import (
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs/fsutil"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs/tmpfs"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
"gvisor.googlesource.com/gvisor/pkg/sentry/memmap"
"gvisor.googlesource.com/gvisor/pkg/sentry/platform"
"gvisor.googlesource.com/gvisor/pkg/sentry/usage"
"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
"gvisor.googlesource.com/gvisor/pkg/syserror"
@@ -117,11 +117,11 @@ func (a *Area) ConfigureMMap(ctx context.Context, file *fs.File, opts *memmap.MM
opts.MaxPerms = opts.MaxPerms.Intersect(a.perms)
if a.tmpfsFile == nil {
p := platform.FromContext(ctx)
if p == nil {
k := kernel.KernelFromContext(ctx)
if k == nil {
return syserror.ENOMEM
}
tmpfsInodeOps := tmpfs.NewInMemoryFile(ctx, usage.Tmpfs, fs.UnstableAttr{}, p)
tmpfsInodeOps := tmpfs.NewInMemoryFile(ctx, usage.Tmpfs, fs.UnstableAttr{}, k)
// This is not backed by a real filesystem, so we pass in nil.
tmpfsInode := fs.NewInode(tmpfsInodeOps, fs.NewNonCachingMountSource(nil, fs.MountSourceFlags{}), fs.StableAttr{})
dirent := fs.NewDirent(tmpfsInode, namePrefix+"/"+a.name)
+1 -1
View File
@@ -22,9 +22,9 @@ import (
"sync"
"testing"
"gvisor.googlesource.com/gvisor/pkg/sentry/context/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
_ "gvisor.googlesource.com/gvisor/pkg/sentry/fs/tmpfs"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
)
+1
View File
@@ -25,6 +25,7 @@ go_library(
"//pkg/sentry/fs/fsutil",
"//pkg/sentry/fs/ramfs",
"//pkg/sentry/fs/tmpfs",
"//pkg/sentry/kernel",
"//pkg/sentry/memmap",
"//pkg/sentry/mm",
"//pkg/sentry/platform",
+2 -2
View File
@@ -22,7 +22,7 @@ import (
"gvisor.googlesource.com/gvisor/pkg/sentry/fs/binder"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs/ramfs"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs/tmpfs"
"gvisor.googlesource.com/gvisor/pkg/sentry/platform"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
)
@@ -86,7 +86,7 @@ func New(ctx context.Context, msrc *fs.MountSource, binderEnabled bool, ashmemEn
"random": newCharacterDevice(newRandomDevice(ctx, fs.RootOwner, 0444), msrc),
"urandom": newCharacterDevice(newRandomDevice(ctx, fs.RootOwner, 0444), msrc),
"shm": tmpfs.NewDir(ctx, nil, fs.RootOwner, fs.FilePermsFromMode(0777), msrc, platform.FromContext(ctx)),
"shm": tmpfs.NewDir(ctx, nil, fs.RootOwner, fs.FilePermsFromMode(0777), msrc, kernel.KernelFromContext(ctx)),
// A devpts is typically mounted at /dev/pts to provide
// pseudoterminal support. Place an empty directory there for
+1 -1
View File
@@ -19,9 +19,9 @@ import (
"testing"
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/context/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
ramfstest "gvisor.googlesource.com/gvisor/pkg/sentry/fs/ramfs/test"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel/contexttest"
)
func TestReaddir(t *testing.T) {
+1 -1
View File
@@ -18,9 +18,9 @@ import (
"testing"
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/context/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
ramfstest "gvisor.googlesource.com/gvisor/pkg/sentry/fs/ramfs/test"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel/contexttest"
"gvisor.googlesource.com/gvisor/pkg/syserror"
)
+1 -1
View File
@@ -18,9 +18,9 @@ import (
"testing"
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/context/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
ramfstest "gvisor.googlesource.com/gvisor/pkg/sentry/fs/ramfs/test"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel/contexttest"
)
// Creates a new MountNamespace with filesystem:
+3 -3
View File
@@ -20,10 +20,10 @@ go_library(
"//pkg/sentry/fs",
"//pkg/sentry/fs/fsutil",
"//pkg/sentry/fs/ramfs",
"//pkg/sentry/kernel",
"//pkg/sentry/kernel/auth",
"//pkg/sentry/kernel/pipe",
"//pkg/sentry/memmap",
"//pkg/sentry/platform",
"//pkg/sentry/safemem",
"//pkg/sentry/socket/unix/transport",
"//pkg/sentry/usage",
@@ -39,9 +39,9 @@ go_test(
embed = [":tmpfs"],
deps = [
"//pkg/sentry/context",
"//pkg/sentry/context/contexttest",
"//pkg/sentry/fs",
"//pkg/sentry/platform",
"//pkg/sentry/kernel",
"//pkg/sentry/kernel/contexttest",
"//pkg/sentry/usage",
"//pkg/sentry/usermem",
],
+3 -3
View File
@@ -19,16 +19,16 @@ import (
"testing"
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/context/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
"gvisor.googlesource.com/gvisor/pkg/sentry/platform"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/usage"
"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
)
func newFileInode(ctx context.Context) *fs.Inode {
m := fs.NewCachingMountSource(&Filesystem{}, fs.MountSourceFlags{})
iops := NewInMemoryFile(ctx, usage.Tmpfs, fs.WithCurrentTime(ctx, fs.UnstableAttr{}), platform.FromContext(ctx))
iops := NewInMemoryFile(ctx, usage.Tmpfs, fs.WithCurrentTime(ctx, fs.UnstableAttr{}), kernel.KernelFromContext(ctx))
return fs.NewInode(iops, m, fs.StableAttr{
DeviceID: tmpfsDevice.DeviceID(),
InodeID: tmpfsDevice.NextIno(),
+2 -2
View File
@@ -21,8 +21,8 @@ import (
"gvisor.googlesource.com/gvisor/pkg/abi/linux"
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel/auth"
"gvisor.googlesource.com/gvisor/pkg/sentry/platform"
)
const (
@@ -131,5 +131,5 @@ func (f *Filesystem) Mount(ctx context.Context, device string, flags fs.MountSou
msrc := fs.NewCachingMountSource(f, flags)
// Construct the tmpfs root.
return NewDir(ctx, nil, owner, perms, msrc, platform.FromContext(ctx)), nil
return NewDir(ctx, nil, owner, perms, msrc, kernel.KernelFromContext(ctx)), nil
}
+10 -10
View File
@@ -21,8 +21,8 @@ import (
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs/fsutil"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
"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/usage"
"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
@@ -52,8 +52,8 @@ type fileInodeOperations struct {
fsutil.InodeNotSymlink `state:"nosave"`
fsutil.NoopWriteOut `state:"nosave"`
// platform is used to allocate memory that stores the file's contents.
platform platform.Platform
// kernel is used to allocate platform memory that stores the file's contents.
kernel *kernel.Kernel
// memUsage is the default memory usage that will be reported by this file.
memUsage usage.MemoryKind
@@ -84,12 +84,12 @@ type fileInodeOperations struct {
}
// NewInMemoryFile returns a new file backed by p.Memory().
func NewInMemoryFile(ctx context.Context, usage usage.MemoryKind, uattr fs.UnstableAttr, p platform.Platform) fs.InodeOperations {
func NewInMemoryFile(ctx context.Context, usage usage.MemoryKind, uattr fs.UnstableAttr, k *kernel.Kernel) fs.InodeOperations {
return &fileInodeOperations{
attr: fsutil.InMemoryAttributes{
Unstable: uattr,
},
platform: p,
kernel: k,
memUsage: usage,
}
}
@@ -98,7 +98,7 @@ func NewInMemoryFile(ctx context.Context, usage usage.MemoryKind, uattr fs.Unsta
func (f *fileInodeOperations) Release(context.Context) {
f.dataMu.Lock()
defer f.dataMu.Unlock()
f.data.DropAll(f.platform.Memory())
f.data.DropAll(f.kernel.Platform.Memory())
}
// Mappable implements fs.InodeOperations.Mappable.
@@ -212,7 +212,7 @@ func (f *fileInodeOperations) Truncate(ctx context.Context, inode *fs.Inode, siz
// and can remove them.
f.dataMu.Lock()
defer f.dataMu.Unlock()
f.data.Truncate(uint64(size), f.platform.Memory())
f.data.Truncate(uint64(size), f.kernel.Platform.Memory())
return nil
}
@@ -310,7 +310,7 @@ func (rw *fileReadWriter) ReadToBlocks(dsts safemem.BlockSeq) (uint64, error) {
return 0, nil
}
mem := rw.f.platform.Memory()
mem := rw.f.kernel.Platform.Memory()
var done uint64
seg, gap := rw.f.data.Find(uint64(rw.offset))
for rw.offset < end {
@@ -376,7 +376,7 @@ func (rw *fileReadWriter) WriteFromBlocks(srcs safemem.BlockSeq) (uint64, error)
}
}()
mem := rw.f.platform.Memory()
mem := rw.f.kernel.Platform.Memory()
// Page-aligned mr for when we need to allocate memory. RoundUp can't
// overflow since end is an int64.
pgstartaddr := usermem.Addr(rw.offset).RoundDown()
@@ -465,7 +465,7 @@ func (f *fileInodeOperations) Translate(ctx context.Context, required, optional
optional.End = pgend
}
mem := f.platform.Memory()
mem := f.kernel.Platform.Memory()
cerr := f.data.Fill(ctx, required, optional, mem, f.memUsage, func(_ context.Context, dsts safemem.BlockSeq, _ uint64) (uint64, error) {
// Newly-allocated pages are zeroed, so we don't need to do anything.
return dsts.NumBytes(), nil
+7 -7
View File
@@ -20,8 +20,8 @@ import (
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs"
"gvisor.googlesource.com/gvisor/pkg/sentry/fs/ramfs"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel/pipe"
"gvisor.googlesource.com/gvisor/pkg/sentry/platform"
"gvisor.googlesource.com/gvisor/pkg/sentry/socket/unix/transport"
"gvisor.googlesource.com/gvisor/pkg/sentry/usage"
"gvisor.googlesource.com/gvisor/pkg/sentry/usermem"
@@ -54,13 +54,13 @@ func rename(ctx context.Context, oldParent *fs.Inode, oldName string, newParent
type Dir struct {
ramfs.Dir
// platform is used to allocate storage for tmpfs Files.
platform platform.Platform
// kernel is used to allocate platform memory as storage for tmpfs Files.
kernel *kernel.Kernel
}
// NewDir returns a new directory.
func NewDir(ctx context.Context, contents map[string]*fs.Inode, owner fs.FileOwner, perms fs.FilePermissions, msrc *fs.MountSource, platform platform.Platform) *fs.Inode {
d := &Dir{platform: platform}
func NewDir(ctx context.Context, contents map[string]*fs.Inode, owner fs.FileOwner, perms fs.FilePermissions, msrc *fs.MountSource, kernel *kernel.Kernel) *fs.Inode {
d := &Dir{kernel: kernel}
d.InitDir(ctx, contents, owner, perms)
// Manually set the CreateOps.
@@ -84,7 +84,7 @@ func (d *Dir) afterLoad() {
func (d *Dir) newCreateOps() *ramfs.CreateOps {
return &ramfs.CreateOps{
NewDir: func(ctx context.Context, dir *fs.Inode, perms fs.FilePermissions) (*fs.Inode, error) {
return NewDir(ctx, nil, fs.FileOwnerFromContext(ctx), perms, dir.MountSource, d.platform), nil
return NewDir(ctx, nil, fs.FileOwnerFromContext(ctx), perms, dir.MountSource, d.kernel), nil
},
NewFile: func(ctx context.Context, dir *fs.Inode, perms fs.FilePermissions) (*fs.Inode, error) {
uattr := fs.WithCurrentTime(ctx, fs.UnstableAttr{
@@ -93,7 +93,7 @@ func (d *Dir) newCreateOps() *ramfs.CreateOps {
// Always start unlinked.
Links: 0,
})
iops := NewInMemoryFile(ctx, usage.Tmpfs, uattr, d.platform)
iops := NewInMemoryFile(ctx, usage.Tmpfs, uattr, d.kernel)
return fs.NewInode(iops, dir.MountSource, fs.StableAttr{
DeviceID: tmpfsDevice.DeviceID(),
InodeID: tmpfsDevice.NextIno(),
+17
View File
@@ -0,0 +1,17 @@
package(licenses = ["notice"]) # Apache 2.0
load("//tools/go_stateify:defs.bzl", "go_library")
go_library(
name = "contexttest",
testonly = 1,
srcs = ["contexttest.go"],
importpath = "gvisor.googlesource.com/gvisor/pkg/sentry/kernel/contexttest",
visibility = ["//pkg/sentry:internal"],
deps = [
"//pkg/sentry/context",
"//pkg/sentry/context/contexttest",
"//pkg/sentry/kernel",
"//pkg/sentry/platform",
],
)
@@ -0,0 +1,38 @@
// Copyright 2018 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 contexttest provides a test context.Context which includes
// a dummy kernel pointing to a valid platform.
package contexttest
import (
"testing"
"gvisor.googlesource.com/gvisor/pkg/sentry/context"
"gvisor.googlesource.com/gvisor/pkg/sentry/context/contexttest"
"gvisor.googlesource.com/gvisor/pkg/sentry/kernel"
"gvisor.googlesource.com/gvisor/pkg/sentry/platform"
)
// Context returns a Context that may be used in tests. Uses ptrace as the
// platform.Platform, and provides a stub kernel that only serves to point to
// the platform.
func Context(tb testing.TB) context.Context {
ctx := contexttest.Context(tb)
k := &kernel.Kernel{
Platform: platform.FromContext(ctx),
}
ctx.(*contexttest.TestContext).RegisterValue(kernel.CtxKernel, k)
return ctx
}