Skip to content

Commit

Permalink
Don't leak network namespaces
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 454707336
  • Loading branch information
avagin authored and gvisor-bot committed Jun 13, 2022
1 parent 2610918 commit 5ffcc1f
Show file tree
Hide file tree
Showing 11 changed files with 61 additions and 1 deletion.
1 change: 1 addition & 0 deletions pkg/sentry/fsimpl/testutil/kernel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions pkg/sentry/inet/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
],
Expand Down
3 changes: 3 additions & 0 deletions pkg/sentry/inet/inet.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
16 changes: 15 additions & 1 deletion pkg/sentry/inet/namespace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"`

Expand All @@ -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.
Expand All @@ -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 {
Expand Down
4 changes: 4 additions & 0 deletions pkg/sentry/inet/test_stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions pkg/sentry/kernel/kernel.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
10 changes: 10 additions & 0 deletions pkg/sentry/kernel/task_clone.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 2 additions & 0 deletions pkg/sentry/kernel/task_exit.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions pkg/sentry/kernel/task_start.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down
4 changes: 4 additions & 0 deletions pkg/sentry/socket/hostinet/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down
5 changes: 5 additions & 0 deletions pkg/sentry/socket/netstack/stack.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 5ffcc1f

Please sign in to comment.