From 607be0585fdc659ec3c043c989a8a6f86fcc14db Mon Sep 17 00:00:00 2001 From: praveensastry Date: Tue, 6 Aug 2019 01:15:48 +1000 Subject: [PATCH 1/4] Add option to configure reference leak checking --- pkg/refs/refcounter.go | 14 ++++++++++++++ runsc/boot/config.go | 19 +++++++++++++++++++ runsc/boot/loader.go | 8 +++----- runsc/main.go | 7 +++++++ 4 files changed, 43 insertions(+), 5 deletions(-) diff --git a/pkg/refs/refcounter.go b/pkg/refs/refcounter.go index 6ecbace9e..417f4a2d6 100644 --- a/pkg/refs/refcounter.go +++ b/pkg/refs/refcounter.go @@ -231,6 +231,20 @@ const ( LeaksLogTraces ) +// String returns LeakMode's string representation. +func (l LeakMode) String() string { + switch l { + case NoLeakChecking: + return "NoLeakChecking" + case LeaksLogWarning: + return "LeaksLogWarning" + case LeaksLogTraces: + return "LeaksLogTraces" + default: + panic(fmt.Sprintf("Invalid leakmode: %d", l)) + } +} + // leakMode stores the current mode for the reference leak checker. // // Values must be one of the LeakMode values. diff --git a/runsc/boot/config.go b/runsc/boot/config.go index 7ae0dd05d..139eb1cce 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -19,6 +19,7 @@ import ( "strconv" "strings" + "gvisor.dev/gvisor/pkg/refs" "gvisor.dev/gvisor/pkg/sentry/watchdog" ) @@ -112,6 +113,20 @@ func MakeWatchdogAction(s string) (watchdog.Action, error) { } } +// MakeRefsLeakMode converts type from string +func MakeRefsLeakMode(s string) (refs.LeakMode, error) { + switch strings.ToLower(s) { + case "nocheck": + return refs.NoLeakChecking, nil + case "warning": + return refs.LeaksLogWarning, nil + case "traces": + return refs.LeaksLogTraces, nil + default: + return 0, fmt.Errorf("invalid refs leakmode %q", s) + } +} + // Config holds configuration that is not part of the runtime spec. type Config struct { // RootDir is the runtime root directory. @@ -201,6 +216,9 @@ type Config struct { // AlsoLogToStderr allows to send log messages to stderr. AlsoLogToStderr bool + + // ReferenceLeakMode sets reference leak check mode + ReferenceLeakMode refs.LeakMode } // ToFlags returns a slice of flags that correspond to the given Config. @@ -227,6 +245,7 @@ func (c *Config) ToFlags() []string { "--num-network-channels=" + strconv.Itoa(c.NumNetworkChannels), "--rootless=" + strconv.FormatBool(c.Rootless), "--alsologtostderr=" + strconv.FormatBool(c.AlsoLogToStderr), + "--refs-leak-mode=" + c.ReferenceLeakMode.String(), } if c.TestOnlyAllowRunAsCurrentUserWithoutChroot { // Only include if set since it is never to be used by users. diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 50cac0433..2fce800ae 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -191,6 +191,9 @@ func New(args Args) (*Loader, error) { return nil, fmt.Errorf("setting up memory usage: %v", err) } + // Sets the refs leak check mode + refs.SetLeakMode(args.Conf.ReferenceLeakMode) + // Create kernel and platform. p, err := createPlatform(args.Conf, args.Device) if err != nil { @@ -1040,8 +1043,3 @@ func (l *Loader) threadGroupFromIDLocked(key execID) (*kernel.ThreadGroup, *host } return ep.tg, ep.tty, nil } - -func init() { - // TODO(gvisor.dev/issue/365): Make this configurable. - refs.SetLeakMode(refs.NoLeakChecking) -} diff --git a/runsc/main.go b/runsc/main.go index 5823819f4..a10138049 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -73,6 +73,7 @@ var ( netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") + referenceLeakMode = flag.String("refs-leak-mode", "nocheck", "sets reference leak check mode: nocheck (default), warning, traces.") // Test flags, not to be used outside tests, ever. testOnlyAllowRunAsCurrentUserWithoutChroot = flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") @@ -168,6 +169,11 @@ func main() { cmd.Fatalf("num_network_channels must be > 0, got: %d", *numNetworkChannels) } + refsLeakMode, err := boot.MakeRefsLeakMode(*referenceLeakMode) + if err != nil { + cmd.Fatalf("%v", err) + } + // Create a new Config from the flags. conf := &boot.Config{ RootDir: *rootDir, @@ -191,6 +197,7 @@ func main() { NumNetworkChannels: *numNetworkChannels, Rootless: *rootless, AlsoLogToStderr: *alsoLogToStderr, + ReferenceLeakMode: refsLeakMode, TestOnlyAllowRunAsCurrentUserWithoutChroot: *testOnlyAllowRunAsCurrentUserWithoutChroot, } From 8d89c0d92b3839eed0839b1a9bc7666e6261d972 Mon Sep 17 00:00:00 2001 From: praveensastry Date: Tue, 6 Aug 2019 11:57:50 +1000 Subject: [PATCH 2/4] Remove traces option for ref leak mode --- runsc/boot/config.go | 6 ++---- runsc/boot/loader.go | 6 +++--- runsc/main.go | 2 +- 3 files changed, 6 insertions(+), 8 deletions(-) diff --git a/runsc/boot/config.go b/runsc/boot/config.go index 139eb1cce..4276a4cc4 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -116,12 +116,10 @@ func MakeWatchdogAction(s string) (watchdog.Action, error) { // MakeRefsLeakMode converts type from string func MakeRefsLeakMode(s string) (refs.LeakMode, error) { switch strings.ToLower(s) { - case "nocheck": + case "disabled": return refs.NoLeakChecking, nil case "warning": return refs.LeaksLogWarning, nil - case "traces": - return refs.LeaksLogTraces, nil default: return 0, fmt.Errorf("invalid refs leakmode %q", s) } @@ -245,7 +243,7 @@ func (c *Config) ToFlags() []string { "--num-network-channels=" + strconv.Itoa(c.NumNetworkChannels), "--rootless=" + strconv.FormatBool(c.Rootless), "--alsologtostderr=" + strconv.FormatBool(c.AlsoLogToStderr), - "--refs-leak-mode=" + c.ReferenceLeakMode.String(), + "--ref-leak-mode=" + c.ReferenceLeakMode.String(), } if c.TestOnlyAllowRunAsCurrentUserWithoutChroot { // Only include if set since it is never to be used by users. diff --git a/runsc/boot/loader.go b/runsc/boot/loader.go index 2fce800ae..65ac67dbf 100644 --- a/runsc/boot/loader.go +++ b/runsc/boot/loader.go @@ -181,6 +181,9 @@ type Args struct { // New initializes a new kernel loader configured by spec. // New also handles setting up a kernel for restoring a container. func New(args Args) (*Loader, error) { + // Sets the reference leak check mode + refs.SetLeakMode(args.Conf.ReferenceLeakMode) + // 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 { @@ -191,9 +194,6 @@ func New(args Args) (*Loader, error) { return nil, fmt.Errorf("setting up memory usage: %v", err) } - // Sets the refs leak check mode - refs.SetLeakMode(args.Conf.ReferenceLeakMode) - // Create kernel and platform. p, err := createPlatform(args.Conf, args.Device) if err != nil { diff --git a/runsc/main.go b/runsc/main.go index a10138049..8857b96ac 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -73,7 +73,7 @@ var ( netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") - referenceLeakMode = flag.String("refs-leak-mode", "nocheck", "sets reference leak check mode: nocheck (default), warning, traces.") + referenceLeakMode = flag.String("ref-leak-mode", "disabled", "sets reference leak check mode: disabled (default), warning.") // Test flags, not to be used outside tests, ever. testOnlyAllowRunAsCurrentUserWithoutChroot = flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") From 73985c6545d887644d8aa4f0e00491cc903501c7 Mon Sep 17 00:00:00 2001 From: praveensastry Date: Fri, 9 Aug 2019 17:13:06 +1000 Subject: [PATCH 3/4] Fix the Stringer for leak mode --- pkg/refs/refcounter.go | 6 +++--- runsc/boot/config.go | 2 ++ runsc/main.go | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pkg/refs/refcounter.go b/pkg/refs/refcounter.go index 417f4a2d6..7fe42d8ff 100644 --- a/pkg/refs/refcounter.go +++ b/pkg/refs/refcounter.go @@ -235,11 +235,11 @@ const ( func (l LeakMode) String() string { switch l { case NoLeakChecking: - return "NoLeakChecking" + return "disabled" case LeaksLogWarning: - return "LeaksLogWarning" + return "warning" case LeaksLogTraces: - return "LeaksLogTraces" + return "traces" default: panic(fmt.Sprintf("Invalid leakmode: %d", l)) } diff --git a/runsc/boot/config.go b/runsc/boot/config.go index 4276a4cc4..3c0f72e9f 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -120,6 +120,8 @@ func MakeRefsLeakMode(s string) (refs.LeakMode, error) { return refs.NoLeakChecking, nil case "warning": return refs.LeaksLogWarning, nil + case "traces": + return refs.LeaksLogTraces, nil default: return 0, fmt.Errorf("invalid refs leakmode %q", s) } diff --git a/runsc/main.go b/runsc/main.go index 8857b96ac..1b7c1c4b7 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -73,7 +73,7 @@ var ( netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") - referenceLeakMode = flag.String("ref-leak-mode", "disabled", "sets reference leak check mode: disabled (default), warning.") + referenceLeakMode = flag.String("ref-leak-mode", "disabled", "sets reference leak check mode: disabled (default), warning, traces.") // Test flags, not to be used outside tests, ever. testOnlyAllowRunAsCurrentUserWithoutChroot = flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.") From 7672eaae25eebad650e71ba790e1585736866ccc Mon Sep 17 00:00:00 2001 From: praveensastry Date: Thu, 22 Aug 2019 22:52:43 +1000 Subject: [PATCH 4/4] Add log prefix for better clarity --- pkg/refs/refcounter.go | 4 ++-- runsc/boot/config.go | 4 ++-- runsc/main.go | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pkg/refs/refcounter.go b/pkg/refs/refcounter.go index 7fe42d8ff..8c3e3d5ab 100644 --- a/pkg/refs/refcounter.go +++ b/pkg/refs/refcounter.go @@ -237,9 +237,9 @@ func (l LeakMode) String() string { case NoLeakChecking: return "disabled" case LeaksLogWarning: - return "warning" + return "log-names" case LeaksLogTraces: - return "traces" + return "log-traces" default: panic(fmt.Sprintf("Invalid leakmode: %d", l)) } diff --git a/runsc/boot/config.go b/runsc/boot/config.go index 3c0f72e9f..6a742f349 100644 --- a/runsc/boot/config.go +++ b/runsc/boot/config.go @@ -118,9 +118,9 @@ func MakeRefsLeakMode(s string) (refs.LeakMode, error) { switch strings.ToLower(s) { case "disabled": return refs.NoLeakChecking, nil - case "warning": + case "log-names": return refs.LeaksLogWarning, nil - case "traces": + case "log-traces": return refs.LeaksLogTraces, nil default: return 0, fmt.Errorf("invalid refs leakmode %q", s) diff --git a/runsc/main.go b/runsc/main.go index 1b7c1c4b7..58e7dd8f3 100644 --- a/runsc/main.go +++ b/runsc/main.go @@ -73,7 +73,7 @@ var ( netRaw = flag.Bool("net-raw", false, "enable raw sockets. When false, raw sockets are disabled by removing CAP_NET_RAW from containers (`runsc exec` will still be able to utilize raw sockets). Raw sockets allow malicious containers to craft packets and potentially attack the network.") numNetworkChannels = flag.Int("num-network-channels", 1, "number of underlying channels(FDs) to use for network link endpoints.") rootless = flag.Bool("rootless", false, "it allows the sandbox to be started with a user that is not root. Sandbox and Gofer processes may run with same privileges as current user.") - referenceLeakMode = flag.String("ref-leak-mode", "disabled", "sets reference leak check mode: disabled (default), warning, traces.") + referenceLeakMode = flag.String("ref-leak-mode", "disabled", "sets reference leak check mode: disabled (default), log-names, log-traces.") // Test flags, not to be used outside tests, ever. testOnlyAllowRunAsCurrentUserWithoutChroot = flag.Bool("TESTONLY-unsafe-nonroot", false, "TEST ONLY; do not ever use! This skips many security measures that isolate the host from the sandbox.")