From 5ffcc1f799e31eba3a95d7e2f251ee111656520c Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Mon, 13 Jun 2022 15:02:23 -0700 Subject: [PATCH] Don't leak network namespaces PiperOrigin-RevId: 454707336 --- pkg/sentry/fsimpl/testutil/kernel.go | 1 + pkg/sentry/inet/BUILD | 14 ++++++++++++++ pkg/sentry/inet/inet.go | 3 +++ pkg/sentry/inet/namespace.go | 16 +++++++++++++++- pkg/sentry/inet/test_stack.go | 4 ++++ pkg/sentry/kernel/kernel.go | 2 ++ pkg/sentry/kernel/task_clone.go | 10 ++++++++++ pkg/sentry/kernel/task_exit.go | 2 ++ pkg/sentry/kernel/task_start.go | 1 + pkg/sentry/socket/hostinet/stack.go | 4 ++++ pkg/sentry/socket/netstack/stack.go | 5 +++++ 11 files changed, 61 insertions(+), 1 deletion(-) diff --git a/pkg/sentry/fsimpl/testutil/kernel.go b/pkg/sentry/fsimpl/testutil/kernel.go index 2df2501ee..e0fd2ff39 100644 --- a/pkg/sentry/fsimpl/testutil/kernel.go +++ b/pkg/sentry/fsimpl/testutil/kernel.go @@ -147,6 +147,7 @@ func CreateTask(ctx context.Context, name string, tc *kernel.ThreadGroup, mntns FDTable: k.NewFDTable(), UserCounters: k.GetUserCounters(creds.RealKUID), } + config.NetworkNamespace.IncRef() t, err := k.TaskSet().NewTask(ctx, config) if err != nil { config.ThreadGroup.Release(ctx) diff --git a/pkg/sentry/inet/BUILD b/pkg/sentry/inet/BUILD index 3bd141b89..156addaf2 100644 --- a/pkg/sentry/inet/BUILD +++ b/pkg/sentry/inet/BUILD @@ -6,6 +6,17 @@ package( licenses = ["notice"], ) +go_template_instance( + name = "namespace_refs", + out = "namespace_refs.go", + package = "inet", + prefix = "namespace", + template = "//pkg/refsvfs2:refs_template", + types = { + "T": "Namespace", + }, +) + go_template_instance( name = "atomicptr_netns", out = "atomicptr_netns_unsafe.go", @@ -24,11 +35,14 @@ go_library( "context.go", "inet.go", "namespace.go", + "namespace_refs.go", "test_stack.go", ], deps = [ "//pkg/abi/linux", + "//pkg/atomicbitops", "//pkg/context", + "//pkg/refsvfs2", "//pkg/tcpip", "//pkg/tcpip/stack", ], diff --git a/pkg/sentry/inet/inet.go b/pkg/sentry/inet/inet.go index b80e07679..5daa76b1f 100644 --- a/pkg/sentry/inet/inet.go +++ b/pkg/sentry/inet/inet.go @@ -85,6 +85,9 @@ type Stack interface { // Resume restarts the network stack after restore. Resume() + // Destroy the network stack. + Destroy() + // RegisteredEndpoints returns all endpoints which are currently registered. RegisteredEndpoints() []stack.TransportEndpoint diff --git a/pkg/sentry/inet/namespace.go b/pkg/sentry/inet/namespace.go index 029af3025..e66dbaa9f 100644 --- a/pkg/sentry/inet/namespace.go +++ b/pkg/sentry/inet/namespace.go @@ -18,6 +18,8 @@ package inet // // +stateify savable type Namespace struct { + namespaceRefs + // stack is the network stack implementation of this network namespace. stack Stack `state:"nosave"` @@ -36,11 +38,13 @@ type Namespace struct { // allowing new network namespaces to be created. If creator is nil, no // networking will function if the network is namespaced. func NewRootNamespace(stack Stack, creator NetworkStackCreator) *Namespace { - return &Namespace{ + n := &Namespace{ stack: stack, creator: creator, isRoot: true, } + n.InitRefs() + return n } // NewNamespace creates a new network namespace from the root. @@ -49,9 +53,19 @@ func NewNamespace(root *Namespace) *Namespace { creator: root.creator, } n.init() + n.InitRefs() return n } +// DecRef decrements the Namespace's refcount. +func (n *Namespace) DecRef() { + n.namespaceRefs.DecRef(func() { + if s := n.Stack(); s != nil { + s.Destroy() + } + }) +} + // Stack returns the network stack of n. Stack may return nil if no network // stack is configured. func (n *Namespace) Stack() Stack { diff --git a/pkg/sentry/inet/test_stack.go b/pkg/sentry/inet/test_stack.go index fef7391b9..e04f2c144 100644 --- a/pkg/sentry/inet/test_stack.go +++ b/pkg/sentry/inet/test_stack.go @@ -50,6 +50,10 @@ func (s *TestStack) Interfaces() map[int32]Interface { return s.InterfacesMap } +// Destroy implements Stack. +func (s *TestStack) Destroy() { +} + // RemoveInterface implements Stack. func (s *TestStack) RemoveInterface(idx int32) error { delete(s.InterfacesMap, idx) diff --git a/pkg/sentry/kernel/kernel.go b/pkg/sentry/kernel/kernel.go index c2973adcc..7ec524d2a 100644 --- a/pkg/sentry/kernel/kernel.go +++ b/pkg/sentry/kernel/kernel.go @@ -1079,6 +1079,7 @@ func (k *Kernel) CreateProcess(args CreateProcessArgs) (*ThreadGroup, ThreadID, ContainerID: args.ContainerID, UserCounters: k.GetUserCounters(args.Credentials.RealKUID), } + config.NetworkNamespace.IncRef() t, err := k.tasks.NewTask(ctx, config) if err != nil { return nil, 0, err @@ -1853,6 +1854,7 @@ func (k *Kernel) Release() { } k.timekeeper.Destroy() k.vdso.Release(ctx) + k.RootNetworkNamespace().DecRef() } // PopulateNewCgroupHierarchy moves all tasks into a newly created cgroup diff --git a/pkg/sentry/kernel/task_clone.go b/pkg/sentry/kernel/task_clone.go index ca3684829..d5b3b181b 100644 --- a/pkg/sentry/kernel/task_clone.go +++ b/pkg/sentry/kernel/task_clone.go @@ -117,7 +117,12 @@ func (t *Task) Clone(args *linux.CloneArgs) (ThreadID, *SyscallControl, error) { netns := t.NetworkNamespace() if args.Flags&linux.CLONE_NEWNET != 0 { netns = inet.NewNamespace(netns) + } else { + netns.IncRef() } + cu.Add(func() { + netns.DecRef() + }) // TODO(b/63601033): Implement CLONE_NEWNS. mntnsVFS2 := t.mountNamespaceVFS2 @@ -454,11 +459,13 @@ func (t *Task) Unshare(flags int32) error { } t.mu.Lock() // Can't defer unlock: DecRefs must occur without holding t.mu. + var oldNETNS *inet.Namespace if flags&linux.CLONE_NEWNET != 0 { if !haveCapSysAdmin { t.mu.Unlock() return linuxerr.EPERM } + oldNETNS = t.netns.Load() t.netns.Store(inet.NewNamespace(t.netns.Load())) } if flags&linux.CLONE_NEWUTS != 0 { @@ -498,6 +505,9 @@ func (t *Task) Unshare(flags int32) error { if oldIPCNS != nil { oldIPCNS.DecRef(t) } + if oldNETNS != nil { + oldNETNS.DecRef() + } if oldFDTable != nil { oldFDTable.DecRef(t) } diff --git a/pkg/sentry/kernel/task_exit.go b/pkg/sentry/kernel/task_exit.go index 717c2b29f..f0eebda40 100644 --- a/pkg/sentry/kernel/task_exit.go +++ b/pkg/sentry/kernel/task_exit.go @@ -272,11 +272,13 @@ func (*runExitMain) execute(t *Task) taskRunState { mntns := t.mountNamespaceVFS2 t.mountNamespaceVFS2 = nil ipcns := t.ipcns + netns := t.NetworkNamespace() t.mu.Unlock() if mntns != nil { mntns.DecRef(t) } ipcns.DecRef(t) + netns.DecRef() // If this is the last task to exit from the thread group, release the // thread group's resources. diff --git a/pkg/sentry/kernel/task_start.go b/pkg/sentry/kernel/task_start.go index 2904398ad..b7497563b 100644 --- a/pkg/sentry/kernel/task_start.go +++ b/pkg/sentry/kernel/task_start.go @@ -115,6 +115,7 @@ func (ts *TaskSet) NewTask(ctx context.Context, cfg *TaskConfig) (*Task, error) cfg.FSContext.DecRef(ctx) cfg.FDTable.DecRef(ctx) cfg.IPCNamespace.DecRef(ctx) + cfg.NetworkNamespace.DecRef() if cfg.MountNamespaceVFS2 != nil { cfg.MountNamespaceVFS2.DecRef(ctx) } diff --git a/pkg/sentry/socket/hostinet/stack.go b/pkg/sentry/socket/hostinet/stack.go index 3e176d80c..f64827bd6 100644 --- a/pkg/sentry/socket/hostinet/stack.go +++ b/pkg/sentry/socket/hostinet/stack.go @@ -65,6 +65,10 @@ type Stack struct { netSNMPFile *os.File } +// Destroy implements inet.Stack.Destroy. +func (*Stack) Destroy() { +} + // NewStack returns an empty Stack containing no configuration. func NewStack() *Stack { return &Stack{ diff --git a/pkg/sentry/socket/netstack/stack.go b/pkg/sentry/socket/netstack/stack.go index 36f377edf..bf2b0741a 100644 --- a/pkg/sentry/socket/netstack/stack.go +++ b/pkg/sentry/socket/netstack/stack.go @@ -37,6 +37,11 @@ type Stack struct { Stack *stack.Stack `state:"manual"` } +// Destroy implements inet.Stack.Destroy. +func (s *Stack) Destroy() { + s.Stack.Close() +} + // SupportsIPv6 implements Stack.SupportsIPv6. func (s *Stack) SupportsIPv6() bool { return s.Stack.CheckNetworkProtocol(ipv6.ProtocolNumber)