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

[1.1] Fixes inability to use /dev/null when inside a container #3620

Merged
merged 1 commit into from
Oct 21, 2022

Conversation

evanphx
Copy link
Contributor

@evanphx evanphx commented Oct 2, 2022

The original code depended on the origin filesystem to have /dev/{block,char} populated. This is done by udev normally and while is very common non-containerized systemd installs, it's very easy to start systemd in a container created by runc itself and not have /dev/{block,char} populated. When this occurs, the following error output is observed:

$ docker run hello-world
docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error reopening /dev/null inside container: open /dev/null: operation not permitted: unknown.

/dev/null can't be opened because it was not added to the deviceAllowList, as there was no /dev/char directory. The change here utilizes the fact that when sysfs in in use, there is a /sys/dev/{block,char} that is kernel maintained that we can check.

I'd also recommend having a base allow list that includes /dev/null (1:3) since runc implicitly depends on it.

@AkihiroSuda
Copy link
Member

What distro needs this fix?

@evanphx
Copy link
Contributor Author

evanphx commented Oct 3, 2022

It's not a specific distro that needs the fix, it was more of a one-off. I was experimenting with running systemd inside a containerd managed container, with docker installed inside the that container (so, nested containerds). The installation of systemd didn't pull in udev (it was ubuntu) and thusly /dev/{block,char} were not created.

@evanphx evanphx force-pushed the b-remove-udev-dep branch 3 times, most recently from f3a00d3 to a3e87da Compare October 3, 2022 17:47
@thaJeztah
Copy link
Member

Looks like this PR was opened against the 1.1 branch; the standard flow is to open against main, and then cherry-pick backports; could you change the PR to tarter "main"?

@evanphx
Copy link
Contributor Author

evanphx commented Oct 3, 2022

@thaJeztah The code on main has been reorganized so the patch wouldn't be easily backported. I'm happy to open an additional PR with the fix for main.

evanphx added a commit to lab47/runc that referenced this pull request Oct 3, 2022
This is a forward port of opencontainers#3620

The original code depended on the origin filesystem to have
/dev/{block,char} populated. This is done by udev normally and while is
very common non-containerized systemd installs, it's very easy to start
systemd in a container created by runc itself and not have
/dev/{block,char} populated. When this occurs, the following error
output is observed:

$ docker run hello-world
docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error reopening /dev/null inside container: open /dev/null: operation not permitted: unknown.

/dev/null can't be opened because it was not added to the
deviceAllowList, as there was no /dev/char directory. The change here
utilizes the fact that when sysfs in in use, there is a
/sys/dev/{block,char} that is kernel maintained that we can check.

Signed-off-by: Evan Phoenix <evan@phx.io>
@evanphx
Copy link
Contributor Author

evanphx commented Oct 3, 2022

@thaJeztah Went ahead and did the forward port: #3623

@AkihiroSuda AkihiroSuda changed the title Fixes inability to use /dev/null when inside a container [1.1] Fixes inability to use /dev/null when inside a container Oct 3, 2022
@AkihiroSuda AkihiroSuda marked this pull request as draft October 3, 2022 23:21
@thaJeztah
Copy link
Member

Thanks! Just making sure that we have all fixes in main (otherwise we may forget porting, and end up with regressions if a release is cut from the main branch 😅)

@kolyshkin
Copy link
Contributor

I suggest we discuss this in #3623 first.

evanphx added a commit to lab47/runc that referenced this pull request Oct 8, 2022
This is a forward port of opencontainers#3620

The original code depended on the origin filesystem to have
/dev/{block,char} populated. This is done by udev normally and while is
very common non-containerized systemd installs, it's very easy to start
systemd in a container created by runc itself and not have
/dev/{block,char} populated. When this occurs, the following error
output is observed:

$ docker run hello-world
docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error reopening /dev/null inside container: open /dev/null: operation not permitted: unknown.

/dev/null can't be opened because it was not added to the
deviceAllowList, as there was no /dev/char directory. The change here
utilizes the fact that when sysfs in in use, there is a
/sys/dev/{block,char} that is kernel maintained that we can check.

Signed-off-by: Evan Phoenix <evan@phx.io>
evanphx added a commit to lab47/runc that referenced this pull request Oct 8, 2022
This is a forward port of opencontainers#3620

The original code depended on the origin filesystem to have
/dev/{block,char} populated. This is done by udev normally and while is
very common non-containerized systemd installs, it's very easy to start
systemd in a container created by runc itself and not have
/dev/{block,char} populated. When this occurs, the following error
output is observed:

$ docker run hello-world
docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error reopening /dev/null inside container: open /dev/null: operation not permitted: unknown.

/dev/null can't be opened because it was not added to the
deviceAllowList, as there was no /dev/char directory. The change here
utilizes the fact that when sysfs in in use, there is a
/sys/dev/{block,char} that is kernel maintained that we can check.

Signed-off-by: Evan Phoenix <evan@phx.io>
@kolyshkin
Copy link
Contributor

#3623 is merged, please backport

@kolyshkin kolyshkin added this to the 1.1.5 milestone Oct 12, 2022
@evanphx evanphx marked this pull request as ready for review October 13, 2022 02:59
@evanphx
Copy link
Contributor Author

evanphx commented Oct 13, 2022

@kolyshkin ready to go!

@kolyshkin
Copy link
Contributor

CI centos-stream-8 timed out; restarted.

kolyshkin
kolyshkin previously approved these changes Oct 17, 2022
Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

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

Please use git cherry-pick -x for tracking the main commit

This is a forward port of opencontainers#3620

The original code depended on the origin filesystem to have
/dev/{block,char} populated. This is done by udev normally and while is
very common non-containerized systemd installs, it's very easy to start
systemd in a container created by runc itself and not have
/dev/{block,char} populated. When this occurs, the following error
output is observed:

$ docker run hello-world
docker: Error response from daemon: failed to create shim task: OCI runtime create failed: runc create failed: unable to start container process: error during container init: error reopening /dev/null inside container: open /dev/null: operation not permitted: unknown.

/dev/null can't be opened because it was not added to the
deviceAllowList, as there was no /dev/char directory. The change here
utilizes the fact that when sysfs in in use, there is a
/sys/dev/{block,char} that is kernel maintained that we can check.

Signed-off-by: Evan Phoenix <evan@phx.io>
(cherry picked from commit 462e719)
@evanphx
Copy link
Contributor Author

evanphx commented Oct 20, 2022

@AkihiroSuda No problem, done!

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

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

LGTM

@mprasil
Copy link

mprasil commented Mar 30, 2023

It's not a specific distro that needs the fix, it was more of a one-off.

Just a follow up on this. @evanphx's fix is actually useful for systems running on top of LXD. (possibly in certain configuration, although I hit this with very standard setup IMO) So this is probably more common than you'd expect.

I am certainly very grateful for this fix. 🙏

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.

None yet

5 participants