storage icon indicating copy to clipboard operation
storage copied to clipboard

Reexec causes infinite loop, if built without CGO_ENABLED

Open hesch opened this issue 2 years ago • 9 comments

Recently I opened this issue https://github.com/containers/buildah/issues/4370 in the buildah repo and found the culprit. While building, I didn't specify CGO_ENABLED=1. This causes an infinite loop of forks, because this is never called: https://github.com/containers/storage/blob/89878da4a1548918d0f0a1f0ba535e68d7dd11f7/pkg/unshare/unshare_cgo.go#L6-L11 Is it possible to check for this while compiling and failing the build?

Steps to reproduce: main.go:

package main

import (
        "github.com/containers/storage/pkg/unshare"
        "github.com/containers/storage/pkg/reexec"
)

func main() {
        reexec.Init()
        unshare.MaybeReexecUsingUserNamespace(false)
}

Compile this with CGO_ENABLED=0 go build -o main main.go

=> Executing the binary leads to an infinite loop (and is akin to a fork bomb)

hesch avatar Nov 17 '22 16:11 hesch

@giuseppe @nalind Is there a simple way to make this a noop if CGO not available?

rhatdan avatar Nov 18 '22 10:11 rhatdan

I would prefer the build failing or an error message to be printed instead of it being a noop. If it is a noop something somewhere down the line will fail and the error message will not lead the user to the root cause of the problem.

hesch avatar Nov 18 '22 12:11 hesch

if the code in unshare_cgo.go is not used, then we have a version in Go in unshare_linux.go. We'd have to understand why it is not running and causing a new reexec

giuseppe avatar Nov 18 '22 13:11 giuseppe

@hesch isn't the code in unshare_linux.go not running in your case before the reexec happens?

giuseppe avatar Nov 18 '22 13:11 giuseppe

we could probably use go exec to create the user namespace instead of the child doing the unshare(CLONE_NEWUSER) but it is not a trivial fix. The easiest is probably to add a new file unshare_nocgo.go and set a variable there saying cgo is not available so MaybeReexecUsingUserNamespace can fail with an useful error message

giuseppe avatar Nov 18 '22 14:11 giuseppe

@giuseppe I was also pretty confused at first how this is even supposed to work. There exists no implementation in pure go. It needs the C-Code to call unshare(CLONE_NEWUSER). What I don't understand is why unshare_linux.go does not use Cmd.SysProcAttr.UnshareFlags to just do the unshare while the new process is cloned. Wouldn't that be much easier?

hesch avatar Nov 18 '22 14:11 hesch

@hesch I agree, that should be easier than creating the namespaces directly from the new process

giuseppe avatar Nov 25 '22 13:11 giuseppe

@hesch @giuseppe Interested in opening a PR?

rhatdan avatar Nov 28 '22 21:11 rhatdan

What would you say is the best solution here? We could

  1. Fail the build if cgo is not enabled as suggested initially This has the advantage of potentially fixing other bugs that occur if cgo is not available.
  2. Fix the code so that it no longer requires cgo Here we have two options: a. Just port the C code as is to go This should be compatible with all OSes? b. Implement a new solution using clone This will drop support for all systems without the clone syscall. I have no Idea if BSD and darwin implement this syscall.

hesch avatar Nov 30 '22 18:11 hesch