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

Reexec causes infinite loop, if built without CGO_ENABLED #1435

Open
hesch opened this issue Nov 17, 2022 · 9 comments
Open

Reexec causes infinite loop, if built without CGO_ENABLED #1435

hesch opened this issue Nov 17, 2022 · 9 comments

Comments

@hesch
Copy link

hesch commented Nov 17, 2022

Recently I opened this issue containers/buildah#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:

// #cgo CFLAGS: -Wall
// extern void _containers_unshare(void);
// static void __attribute__((constructor)) init(void) {
// _containers_unshare();
// }
import "C"
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)

@rhatdan
Copy link
Member

rhatdan commented Nov 18, 2022

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

@hesch
Copy link
Author

hesch commented Nov 18, 2022

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.

@giuseppe
Copy link
Member

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
Copy link
Member

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

@giuseppe
Copy link
Member

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

@hesch
Copy link
Author

hesch commented Nov 18, 2022

@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?

@giuseppe
Copy link
Member

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

@rhatdan
Copy link
Member

rhatdan commented Nov 28, 2022

@hesch @giuseppe Interested in opening a PR?

@hesch
Copy link
Author

hesch commented Nov 30, 2022

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants