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

network: disallow CNI networks with user namespaces #8960

Merged

Conversation

giuseppe
Copy link
Member

it solves a segfault when running as rootless a command like:

$ podman run --uidmap 0:0:1 --net foo --rm fedora true
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x5629bccc407c]

goroutine 1 [running]:
panic(0x5629bd3d39e0, 0x5629be0ab8e0)
/usr/lib/golang/src/runtime/panic.go:1064 +0x545 fp=0xc0004592c0 sp=0xc0004591f8 pc=0x5629bbd35d85
runtime.panicmem(...)
/usr/lib/golang/src/runtime/panic.go:212
runtime.sigpanic()
/usr/lib/golang/src/runtime/signal_unix.go:742 +0x413 fp=0xc0004592f0 sp=0xc0004592c0 pc=0x5629bbd4cd33
github.com/containers/podman/libpod.(*Runtime).setupRootlessNetNS(0xc0003fe9c0, 0xc0003d74a0, 0x0, 0x0)
/builddir/build/BUILD/podman-2.2.1/_build/src/github.com/containers/podman/libpod/networking_linux.go:238 +0xdc fp=0xc000459338 sp=0xc0004592f0 pc=0x5629bccc407c
github.com/containers/podman/libpod.(*Container).completeNetworkSetup(0xc0003d74a0, 0x0, 0x0)
/builddir/build/BUILD/podman-2.2.1/_build/src/github.com/containers/podman/libpod/container_internal.go:965 +0xb72 fp=0xc0004594d8 sp=0xc000459338 pc=0x5629bcc81732

[.....]

Signed-off-by: Giuseppe Scrivano [email protected]

@openshift-ci-robot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: giuseppe

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci-robot openshift-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 13, 2021
@@ -639,6 +639,22 @@ var _ = Describe("Podman run networking", func() {
Expect(create.ExitCode()).To(BeZero())
})

It("podman run in custom CNI network with --uidmap", func() {
netName := "podmantestnetwork2"
Copy link
Member

Choose a reason for hiding this comment

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

Please use stringid.GenerateNonCryptoID() for the network name. The static network names are causing a lot of flakes.

@giuseppe giuseppe force-pushed the bridge-no-post-config branch from 3978eaf to 9585fbe Compare January 13, 2021 11:55
@rhatdan
Copy link
Member

rhatdan commented Jan 13, 2021

LGTM but make @Luap99 change.

@rhatdan
Copy link
Member

rhatdan commented Jan 13, 2021

@containers/podman-maintainers PTAL

@mheon
Copy link
Member

mheon commented Jan 13, 2021

LGTM, but your test is failing on rootless.

@giuseppe giuseppe force-pushed the bridge-no-post-config branch from d7d2ba9 to 5e8f08e Compare January 13, 2021 15:32
@giuseppe giuseppe changed the title network: always configure CNI networks before startup network: disallow CNI networks with user namespaces Jan 13, 2021
@giuseppe
Copy link
Member Author

it was passing locally as I had a modified containers.conf file :/

I don't think we can (easily) support this use case, the user namespace doesn't own the CNI namespace, so the netns configuration should be performed through the infra container after the container is created.

@AkihiroSuda do you have better ideas on how it can be performed?

it solves a segfault when running as rootless a command like:

$ podman run --uidmap 0:0:1 --net foo --rm fedora true
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x30 pc=0x5629bccc407c]

goroutine 1 [running]:
panic(0x5629bd3d39e0, 0x5629be0ab8e0)
	/usr/lib/golang/src/runtime/panic.go:1064 +0x545 fp=0xc0004592c0 sp=0xc0004591f8 pc=0x5629bbd35d85
runtime.panicmem(...)
	/usr/lib/golang/src/runtime/panic.go:212
runtime.sigpanic()
	/usr/lib/golang/src/runtime/signal_unix.go:742 +0x413 fp=0xc0004592f0 sp=0xc0004592c0 pc=0x5629bbd4cd33
github.com/containers/podman/libpod.(*Runtime).setupRootlessNetNS(0xc0003fe9c0, 0xc0003d74a0, 0x0, 0x0)
	/builddir/build/BUILD/podman-2.2.1/_build/src/github.com/containers/podman/libpod/networking_linux.go:238 +0xdc fp=0xc000459338 sp=0xc0004592f0 pc=0x5629bccc407c
github.com/containers/podman/libpod.(*Container).completeNetworkSetup(0xc0003d74a0, 0x0, 0x0)
	/builddir/build/BUILD/podman-2.2.1/_build/src/github.com/containers/podman/libpod/container_internal.go:965 +0xb72 fp=0xc0004594d8 sp=0xc000459338 pc=0x5629bcc81732

[.....]

Signed-off-by: Giuseppe Scrivano <[email protected]>
@giuseppe giuseppe force-pushed the bridge-no-post-config branch from 5e8f08e to ee68466 Compare January 13, 2021 16:05
@umohnani8
Copy link
Member

LGTM, tests seem to be mainly green

@rhatdan
Copy link
Member

rhatdan commented Jan 13, 2021

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Jan 13, 2021
@mheon
Copy link
Member

mheon commented Jan 13, 2021

/hold
One failure

@openshift-ci-robot openshift-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2021
@mheon
Copy link
Member

mheon commented Jan 13, 2021

Looks like a flake, already restarted. Remove hold when all green.

@rhatdan
Copy link
Member

rhatdan commented Jan 13, 2021

/hold cancel

@openshift-ci-robot openshift-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 13, 2021
@TomSweeneyRedHat
Copy link
Member

LGTM

@TomSweeneyRedHat
Copy link
Member

All happy green test buttons.

@openshift-merge-robot openshift-merge-robot merged commit bbff9c8 into containers:master Jan 13, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 23, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants