Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nsexec: spring cleaning #3953

Closed
wants to merge 11 commits into from
9 changes: 9 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,15 @@ jobs:
with:
bats-version: 1.9.0

- name: procfs mount
run: |
# Get the list of mounts to help with debugging.
cat /proc/self/mounts
# Create a procfs mount that is not masked, to ensure that container
# procfs mounts will succeed.
sudo mkdir -p /tmp/.procfs-stashed-mount
sudo unshare -pf mount -t proc -o subset=pid proc /tmp/.procfs-stashed-mount
Comment on lines +81 to +88
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know why this has become necessary with this PR (the "problem" commit is the /proc/self/exe cloning change) but this solves the issue and this is a CI-weirdness issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cyphar Does that mean that runc won't work as-is inside "your usual" GHA, or is it only for the sake of testing?


- name: unit test
if: matrix.rootless != 'rootless'
run: sudo -E PATH="$PATH" -- make TESTFLAGS="${{ matrix.race }}" localunittest
Expand Down
4 changes: 2 additions & 2 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -105,7 +105,7 @@ unittest: runcimage
-t --privileged --rm \
-v /lib/modules:/lib/modules:ro \
-v $(CURDIR):/go/src/$(PROJECT) \
$(RUNC_IMAGE) make localunittest TESTFLAGS=$(TESTFLAGS)
$(RUNC_IMAGE) make localunittest TESTFLAGS="$(TESTFLAGS)"

localunittest: all
$(GO) test -timeout 3m -tags "$(BUILDTAGS)" $(TESTFLAGS) -v ./...
Expand All @@ -115,7 +115,7 @@ integration: runcimage
-t --privileged --rm \
-v /lib/modules:/lib/modules:ro \
-v $(CURDIR):/go/src/$(PROJECT) \
$(RUNC_IMAGE) make localintegration TESTPATH=$(TESTPATH)
$(RUNC_IMAGE) make localintegration TESTPATH="$(TESTPATH)"

localintegration: all
bats -t tests/integration$(TESTPATH)
Expand Down
4 changes: 2 additions & 2 deletions contrib/cmd/recvtty/recvtty.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func handleSingle(path string, noStdin bool) error {
defer socket.Close()

// Get the master file descriptor from runC.
master, err := utils.RecvFd(socket)
master, err := utils.RecvFile(socket)
if err != nil {
return err
}
Expand Down Expand Up @@ -171,7 +171,7 @@ func handleNull(path string) error {
defer socket.Close()

// Get the master file descriptor from runC.
master, err := utils.RecvFd(socket)
master, err := utils.RecvFile(socket)
if err != nil {
return
}
Expand Down
12 changes: 10 additions & 2 deletions libcontainer/configs/mount_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,14 @@ type Mount struct {
// Source path for the mount.
Source string `json:"source"`

// SourceFd is a open_tree(2)-style file descriptor created with the new
// mount API. If non-nil, this indicates that SourceFd is a configured
// mountfd that should be used for the mount into the container.
//
// Note that you cannot use /proc/self/fd/$n-style paths with
// open_tree(2)-style file descriptors.
SourceFd *int

// Destination path for the mount inside the container.
Destination string `json:"destination"`

Expand Down Expand Up @@ -34,13 +42,13 @@ type Mount struct {
// Note that, the underlying filesystem should support this feature to be
// used.
// Every mount point could have its own mapping.
UIDMappings []IDMap `json:"uidMappings,omitempty"`
UIDMappings []IDMap `json:"uid_mappings,omitempty"`

// GIDMappings is used to changing file group owners w/o calling chown.
// Note that, the underlying filesystem should support this feature to be
// used.
// Every mount point could have its own mapping.
GIDMappings []IDMap `json:"gidMappings,omitempty"`
GIDMappings []IDMap `json:"gid_mappings,omitempty"`
}

func (m *Mount) IsBind() bool {
Expand Down
38 changes: 2 additions & 36 deletions libcontainer/configs/validate/validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -259,24 +259,11 @@ func checkIDMapMounts(config *configs.Config, m *configs.Mount) error {
}

if !m.IsBind() {
return fmt.Errorf("gidMappings/uidMappings is supported only for mounts with the option 'bind'")
return fmt.Errorf("id-mapped mounts are only supported for bind-mounts")
}
if config.RootlessEUID {
return fmt.Errorf("gidMappings/uidMappings is not supported when runc is being launched with EUID != 0, needs CAP_SYS_ADMIN on the runc parent's user namespace")
return fmt.Errorf("id-mapped mounts are not supported for rootless containers")
}
if len(config.UIDMappings) == 0 || len(config.GIDMappings) == 0 {
return fmt.Errorf("not yet supported to use gidMappings/uidMappings in a mount without also using a user namespace")
}
if !sameMapping(config.UIDMappings, m.UIDMappings) {
return fmt.Errorf("not yet supported for the mount uidMappings to be different than user namespace uidMapping")
}
if !sameMapping(config.GIDMappings, m.GIDMappings) {
return fmt.Errorf("not yet supported for the mount gidMappings to be different than user namespace gidMapping")
}
if !filepath.IsAbs(m.Source) {
return fmt.Errorf("mount source not absolute")
}

return nil
}

Expand All @@ -298,27 +285,6 @@ func mounts(config *configs.Config) error {
return nil
}

// sameMapping checks if the mappings are the same. If the mappings are the same
// but in different order, it returns false.
func sameMapping(a, b []configs.IDMap) bool {
if len(a) != len(b) {
return false
}

for i := range a {
if a[i].ContainerID != b[i].ContainerID {
return false
}
if a[i].HostID != b[i].HostID {
return false
}
if a[i].Size != b[i].Size {
return false
}
}
return true
}

func isHostNetNS(path string) (bool, error) {
const currentProcessNetns = "/proc/self/ns/net"

Expand Down
81 changes: 30 additions & 51 deletions libcontainer/configs/validate/validator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -436,129 +436,108 @@ func TestValidateIDMapMounts(t *testing.T) {
},
},
{
name: "idmap mount without userns mappings",
isErr: true,
config: &configs.Config{
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts with different userns and mount mappings",
name: "idmap mounts without abs dest path",
isErr: true,
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Destination: "/abs/path/",
Destination: "./rel/path/",
Flags: unix.MS_BIND,
UIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts with different userns and mount mappings",
isErr: true,
name: "simple idmap mount",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Source: "/another-abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts without abs source path",
isErr: true,
name: "idmap mount with more flags",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "./rel/path/",
Source: "/another-abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
Flags: unix.MS_BIND | unix.MS_RDONLY,
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mounts without abs dest path",
isErr: true,
name: "idmap mount without userns mappings",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/abs/path/",
Destination: "./rel/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: mapping,
},
},
},
},

{
name: "simple idmap mount",
name: "idmap mounts with different userns and mount mappings",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/another-abs/path/",
Source: "/abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND,
UIDMappings: mapping,
UIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
GIDMappings: mapping,
},
},
},
},
{
name: "idmap mount with more flags",
name: "idmap mounts with different userns and mount mappings",
config: &configs.Config{
UIDMappings: mapping,
GIDMappings: mapping,
Mounts: []*configs.Mount{
{
Source: "/another-abs/path/",
Source: "/abs/path/",
Destination: "/abs/path/",
Flags: unix.MS_BIND | unix.MS_RDONLY,
Flags: unix.MS_BIND,
UIDMappings: mapping,
GIDMappings: mapping,
GIDMappings: []configs.IDMap{
{
ContainerID: 10,
HostID: 10,
Size: 1,
},
},
},
},
},
Expand Down
Loading