From d9fcdc771447d30f92770d7e604656ed04047ef4 Mon Sep 17 00:00:00 2001 From: Etienne Perot Date: Tue, 7 Mar 2023 12:50:36 -0800 Subject: [PATCH] seccheck: Initialize `seccheck.Points` only when needed. This moves the initialization of `seccheck.Points` out of package-level `init` and instead moves it to an explicit `seccheck.Initialize()` function. On AMD64, this saves about 614KiB of heap memory that would otherwise always be live. PiperOrigin-RevId: 514813332 --- pkg/sentry/fsimpl/testutil/BUILD | 1 + pkg/sentry/fsimpl/testutil/kernel.go | 3 +++ pkg/sentry/seccheck/metadata.go | 16 ++++++++++++++-- pkg/sentry/seccheck/metadata_amd64.go | 4 ++-- pkg/sentry/seccheck/metadata_arm64.go | 4 ++-- pkg/sentry/seccheck/seccheck_test.go | 18 +++++++++++------- pkg/sentry/syscalls/linux/linux64_test.go | 6 ++++++ runsc/boot/loader.go | 3 +++ runsc/cmd/trace/create_test.go | 5 +++++ runsc/cmd/trace/trace.go | 2 ++ 10 files changed, 49 insertions(+), 13 deletions(-) diff --git a/pkg/sentry/fsimpl/testutil/BUILD b/pkg/sentry/fsimpl/testutil/BUILD index 61423ad92..d0cf0c7ce 100644 --- a/pkg/sentry/fsimpl/testutil/BUILD +++ b/pkg/sentry/fsimpl/testutil/BUILD @@ -30,6 +30,7 @@ go_library( "//pkg/sentry/platform", "//pkg/sentry/platform/kvm", "//pkg/sentry/platform/ptrace", + "//pkg/sentry/seccheck", "//pkg/sentry/time", "//pkg/sentry/vfs", "//pkg/sync", diff --git a/pkg/sentry/fsimpl/testutil/kernel.go b/pkg/sentry/fsimpl/testutil/kernel.go index 4b56eddc6..420593808 100644 --- a/pkg/sentry/fsimpl/testutil/kernel.go +++ b/pkg/sentry/fsimpl/testutil/kernel.go @@ -34,6 +34,7 @@ import ( "gvisor.dev/gvisor/pkg/sentry/mm" "gvisor.dev/gvisor/pkg/sentry/pgalloc" "gvisor.dev/gvisor/pkg/sentry/platform" + "gvisor.dev/gvisor/pkg/sentry/seccheck" "gvisor.dev/gvisor/pkg/sentry/time" "gvisor.dev/gvisor/pkg/sentry/vfs" @@ -49,6 +50,8 @@ var ( // Boot initializes a new bare bones kernel for test. func Boot() (*kernel.Kernel, error) { + seccheck.Initialize() + platformCtr, err := platform.Lookup(*platformFlag) if err != nil { return nil, fmt.Errorf("platform not found: %v", err) diff --git a/pkg/sentry/seccheck/metadata.go b/pkg/sentry/seccheck/metadata.go index fa2e55227..c4d949d4d 100644 --- a/pkg/sentry/seccheck/metadata.go +++ b/pkg/sentry/seccheck/metadata.go @@ -20,6 +20,7 @@ import ( "path" "gvisor.dev/gvisor/pkg/fd" + "gvisor.dev/gvisor/pkg/sync" ) // PointX represents the checkpoint X. @@ -210,8 +211,8 @@ func addSyscallPointHelper(typ SyscallType, sysno uintptr, name string, optional }) } -// These are all the Points available in the system. -func init() { +// genericInit initializes non-architecture-specific Points available in the system. +func genericInit() { // Points from the container namespace. registerPoint(PointDesc{ ID: PointContainerStart, @@ -286,3 +287,14 @@ func init() { ContextFields: defaultContextFields, }) } + +var initOnce sync.Once + +// Initialize initializes the Points available in the system. +// Must be called prior to using any of them. +func Initialize() { + initOnce.Do(func() { + genericInit() + archInit() + }) +} diff --git a/pkg/sentry/seccheck/metadata_amd64.go b/pkg/sentry/seccheck/metadata_amd64.go index a4bcc4015..bcbc8bd49 100644 --- a/pkg/sentry/seccheck/metadata_amd64.go +++ b/pkg/sentry/seccheck/metadata_amd64.go @@ -17,9 +17,9 @@ package seccheck -// init registers syscall trace points metadata. +// archInit registers syscall trace points metadata. // Keep them sorted by syscall number. -func init() { +func archInit() { addSyscallPoint(0, "read", []FieldDesc{ { ID: FieldSyscallPath, diff --git a/pkg/sentry/seccheck/metadata_arm64.go b/pkg/sentry/seccheck/metadata_arm64.go index 0e349dd93..08c9c1bdc 100644 --- a/pkg/sentry/seccheck/metadata_arm64.go +++ b/pkg/sentry/seccheck/metadata_arm64.go @@ -17,9 +17,9 @@ package seccheck -// init registers syscall trace points metadata. +// archInit registers syscall trace points metadata. // Keep them sorted by syscall number. -func init() { +func archInit() { addSyscallPoint(19, "eventfd2", nil) addSyscallPoint(23, "dup", []FieldDesc{ { diff --git a/pkg/sentry/seccheck/seccheck_test.go b/pkg/sentry/seccheck/seccheck_test.go index 975d6a38b..b10a1f887 100644 --- a/pkg/sentry/seccheck/seccheck_test.go +++ b/pkg/sentry/seccheck/seccheck_test.go @@ -16,6 +16,7 @@ package seccheck import ( "errors" + "os" "testing" "gvisor.dev/gvisor/pkg/context" @@ -23,13 +24,6 @@ import ( pb "gvisor.dev/gvisor/pkg/sentry/seccheck/points/points_go_proto" ) -func init() { - RegisterSink(SinkDesc{ - Name: "test-sink", - New: newTestSink, - }) -} - type testSink struct { SinkDefaults @@ -275,3 +269,13 @@ func TestFieldMask(t *testing.T) { t.Errorf("FieldMask must not contain %v: %+v", want, fd) } } + +func TestMain(m *testing.M) { + + RegisterSink(SinkDesc{ + Name: "test-sink", + New: newTestSink, + }) + Initialize() + os.Exit(m.Run()) +} diff --git a/pkg/sentry/syscalls/linux/linux64_test.go b/pkg/sentry/syscalls/linux/linux64_test.go index ff6eceab9..b7b285209 100644 --- a/pkg/sentry/syscalls/linux/linux64_test.go +++ b/pkg/sentry/syscalls/linux/linux64_test.go @@ -16,6 +16,7 @@ package linux import ( "fmt" + "os" "reflect" "runtime" "strings" @@ -100,3 +101,8 @@ func TestSeccheckSyscalls(t *testing.T) { }) } } + +func TestMain(m *testing.M) { + seccheck.Initialize() + os.Exit(m.Run()) +} diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index aaab3849a..c93953b8d 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -243,6 +243,9 @@ const startingStdioFD = 256 func New(args Args) (*Loader, error) { stopProfiling := profile.Start(args.ProfileOpts) + // Initialize seccheck points. + seccheck.Initialize() + // We initialize the rand package now to make sure /dev/urandom is pre-opened // on kernels that do not support getrandom(2). if err := rand.Init(); err != nil { diff --git a/runsc/cmd/trace/create_test.go b/runsc/cmd/trace/create_test.go index e7befb101..756a38559 100644 --- a/runsc/cmd/trace/create_test.go +++ b/runsc/cmd/trace/create_test.go @@ -77,3 +77,8 @@ func TestConfigFile(t *testing.T) { }) } } + +func TestMain(m *testing.M) { + seccheck.Initialize() + os.Exit(m.Run()) +} diff --git a/runsc/cmd/trace/trace.go b/runsc/cmd/trace/trace.go index 842fc6325..fe3540e7e 100644 --- a/runsc/cmd/trace/trace.go +++ b/runsc/cmd/trace/trace.go @@ -20,6 +20,7 @@ import ( "context" "github.com/google/subcommands" + "gvisor.dev/gvisor/pkg/sentry/seccheck" "gvisor.dev/gvisor/runsc/flag" ) @@ -54,6 +55,7 @@ func (*Trace) SetFlags(f *flag.FlagSet) {} // Execute implements subcommands.Command. func (*Trace) Execute(ctx context.Context, f *flag.FlagSet, args ...any) subcommands.ExitStatus { + seccheck.Initialize() return createCommander(f).Execute(ctx, args...) }