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

setup-user: use github.com/moby/sys/user #134

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

neersighted
Copy link
Contributor

@neersighted neersighted commented Oct 11, 2023

Use the new github.com/moby/sys/user module (and clean up the dependency tree) to remove the runc dependency and reduce the possibility of future conflicts resulting from dependencies.

@neersighted neersighted force-pushed the dep_cleanup branch 2 times, most recently from a6a74a6 to 4b9f447 Compare October 11, 2023 19:09
Copy link
Owner

@tianon tianon left a comment

Choose a reason for hiding this comment

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

Looks good, thank you!

I'd love to get pedantic and make sure golang.org/x/sys is at the absolute lowest version it can reasonably be, but that feels like an unreasonable amount of digging to accomplish, so I think I'll just leave it alone for now. 😅

One thing I'd like to clarify though (I wish I could comment on the commit message itself 😂):

Since Go 1.16, [Go issue 1435][1] is solved, and the stdlib syscall implementations work on Linux. While they are a bit more flexible/heavier-weight than the implementations that were copied to libcontainer/system (working across all threads), we compile with Cgo, and using the libc wrappers should be just as suitable.

When you say "we compile with cgo", did you mean "if we compile with cgo" ? (We actually go out of our way to compile without cgo everywhere we build, so it'll only be distro builds like the ones in Debian that might use cgo. 😅)

@neersighted
Copy link
Contributor Author

Oh, shoot, I kept that commit message from runc. That should be dropped here.

@tianon
Copy link
Owner

tianon commented Oct 26, 2023

Ah, that makes more sense 😄

Would you like to update, or should I instead just squash and merge? (I have no strong preference either way)

Since Go 1.16, [Go issue 1435][1] is solved, and the stdlib syscall
implementations work on Linux.

  [1]: golang/go#1435

Signed-off-by: Bjorn Neergaard <[email protected]>
Prefer to use the latest syscall implementation, instead of the one that
was shipped with the Go compiler. As this was an indirect dependency,
this aligns all syscalls in the package to a common implementation.

Signed-off-by: Bjorn Neergaard <[email protected]>
Break the dependency on runc by using the new canonical source of the
`user` package at github.com/moby/sys.

Signed-off-by: Bjorn Neergaard <[email protected]>
@tianon tianon merged commit 99f2f75 into tianon:master Nov 2, 2023
1 check passed
@neersighted neersighted deleted the dep_cleanup branch November 2, 2023 22:10
@eshafaq1
Copy link

eshafaq1 commented Nov 3, 2023

TY @neersighted

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

Successfully merging this pull request may close these issues.

gosu binary Vuln with thirdparty github.com/opencontainers/runc (CVE-2023-27561)
3 participants