Skip to content

Commit

Permalink
rootlessnetns: fix cleanup for errors in Run
Browse files Browse the repository at this point in the history
The Run() function is used to run long running command in the netns,
namly podman unshare --rootless-netns used that. As such the function
actually unlocks for the main command as otherwise a user could hold the
lock forever effectively causing deadlocks.

Now because we unlock the ref count might change during that time. And
just because we create the netns doesn't mean there are now other users
of the netns. Therefore the cleanup in runInner() was wrong in that
case causing problems for other running containers.

To fix this make sure we do not cleanup in the Run() case unless the
count is 0.

Signed-off-by: Paul Holzinger <pholzing@redhat.com>
  • Loading branch information
Luap99 committed Aug 8, 2024
1 parent c11b369 commit e32811a
Showing 1 changed file with 8 additions and 9 deletions.
17 changes: 8 additions & 9 deletions libnetwork/internal/rootlessnetns/netns_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -512,14 +512,14 @@ func (n *Netns) mountCNIVarDir() error {
return nil
}

func (n *Netns) runInner(toRun func() error) (err error) {
func (n *Netns) runInner(toRun func() error, cleanup bool) (err error) {
nsRef, newNs, err := n.getOrCreateNetns()
if err != nil {
return err
}
defer nsRef.Close()
// If a new netns was created make sure to clean it up again on an error to not leak it.
if newNs {
// If a new netns was created make sure to clean it up again on an error to not leak it if requested.
if newNs && cleanup {
defer func() {
if err != nil {
if err := n.cleanup(); err != nil {
Expand Down Expand Up @@ -555,7 +555,7 @@ func (n *Netns) runInner(toRun func() error) (err error) {
}

func (n *Netns) Setup(nets int, toRun func() error) error {
err := n.runInner(toRun)
err := n.runInner(toRun, true)
if err != nil {
return err
}
Expand All @@ -564,7 +564,7 @@ func (n *Netns) Setup(nets int, toRun func() error) error {
}

func (n *Netns) Teardown(nets int, toRun func() error) error {
err := n.runInner(toRun)
err := n.runInner(toRun, true)
if err != nil {
return err
}
Expand Down Expand Up @@ -601,7 +601,7 @@ func (n *Netns) Run(lock *lockfile.LockFile, toRun func() error) error {
return err
}

inErr := n.runInner(inner)
inErr := n.runInner(inner, false)
// make sure to always reset the ref counter afterwards
count, err := refCount(n.dir, -1)
if err != nil {
Expand All @@ -611,9 +611,8 @@ func (n *Netns) Run(lock *lockfile.LockFile, toRun func() error) error {
logrus.Errorf("Failed to decrement ref count: %v", err)
return inErr
}
// runInner() already cleans up the netns when it created a new one on errors
// so we only need to do that if there was no error.
if inErr == nil && count == 0 {

if count == 0 {
err = n.cleanup()
if err != nil {
return wrapError("cleanup", err)
Expand Down

0 comments on commit e32811a

Please sign in to comment.