Skip to content

Commit

Permalink
libct: fix shared pidns detection
Browse files Browse the repository at this point in the history
When someone is using libcontainer to start and kill containers from a
long lived process (i.e. the same process creates and removes the
container), initProcess.wait method is used, which has a kludge to work
around killing containers that do not have their own PID namespace.

The code that checks for own PID namespace is not entirely correct.
To be exact, it does not set sharePidns flag when the host/caller PID
namespace is implicitly used. As a result, the above mentioned kludge
does not work.

Fix the issue, add a test case (which fails without the fix).

Signed-off-by: Kir Kolyshkin <[email protected]>
  • Loading branch information
kolyshkin committed May 13, 2023
1 parent 8384744 commit bd44b32
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 7 deletions.
12 changes: 12 additions & 0 deletions libcontainer/configs/namespaces_syscall.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,15 @@ func (n *Namespaces) CloneFlags() uintptr {
}
return uintptr(flag)
}

// Private tells whether the namespace of type t is configured as private
// (i.e. it exists and is not shared).
func (n Namespaces) Private(t NamespaceType) bool {
for _, v := range n {
if v.Type == t {
return v.Path == ""
}
}
// Not found, so implicitly sharing a parent namespace.
return false
}
3 changes: 1 addition & 2 deletions libcontainer/container_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -549,7 +549,6 @@ func (c *Container) newInitProcess(p *Process, cmd *exec.Cmd, messageSockPair, l
nsMaps[ns.Type] = ns.Path
}
}
_, sharePidns := nsMaps[configs.NEWPID]
data, err := c.bootstrapData(c.config.Namespaces.CloneFlags(), nsMaps, initStandard)
if err != nil {
return nil, err
Expand Down Expand Up @@ -594,7 +593,7 @@ func (c *Container) newInitProcess(p *Process, cmd *exec.Cmd, messageSockPair, l
container: c,
process: p,
bootstrapData: data,
sharePidns: sharePidns,
sharePidns: !c.config.Namespaces.Private(configs.NEWPID),
}
c.initProcess = init
return init, nil
Expand Down
20 changes: 15 additions & 5 deletions libcontainer/integration/exec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1355,16 +1355,26 @@ func TestPIDHost(t *testing.T) {
}
}

func TestPIDHostInitProcessWait(t *testing.T) {
func TestHostPidnsInitKill(t *testing.T) {
config := newTemplateConfig(t, nil)
// Implicitly use host pid ns.
config.Namespaces.Remove(configs.NEWPID)
testPidnsInitKill(t, config)
}

func TestSharedPidnsInitKill(t *testing.T) {
config := newTemplateConfig(t, nil)
// Explicitly use host pid ns.
config.Namespaces.Add(configs.NEWPID, "/proc/1/ns/pid")
testPidnsInitKill(t, config)
}

func testPidnsInitKill(t *testing.T, config *configs.Config) {
if testing.Short() {
return
}

pidns := "/proc/1/ns/pid"

// Run a container with two long-running processes.
config := newTemplateConfig(t, nil)
config.Namespaces.Add(configs.NEWPID, pidns)
container, err := newContainer(t, config)
ok(t, err)
defer func() {
Expand Down

0 comments on commit bd44b32

Please sign in to comment.