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

oci: WithDefaultUnixDevices(): remove tun/tap from the default devices #6923

Merged
merged 1 commit into from May 12, 2022

Conversation

thaJeztah
Copy link
Member

A container should not have access to tun/tap device, unless it is explicitly
specified in configuration.

This device was already removed from docker's default, and runc's default;

Per the commit message in runc, this should also fix these messages;

Apr 26 03:46:56 foo.bar systemd1: Couldn't stat device /dev/char/10:200: No such file or directory

coming from systemd on every container start, when the systemd cgroup driver
is used, and the system runs an old (< v240) version of systemd
(the message was presumably eliminated by 1).

A container should not have access to tun/tap device, unless it is explicitly
specified in configuration.

This device was already removed from docker's default, and runc's default;

- opencontainers/runc@2ce40b6
- https://github.com/moby/moby//commit/9c4570a958df42d1ad19364b1a8da55b891d850a

Per the commit message in runc, this should also fix these messages;

> Apr 26 03:46:56 foo.bar systemd[1]: Couldn't stat device /dev/char/10:200: No such file or directory

coming from systemd on every container start, when the systemd cgroup driver
is used, and the system runs an old (< v240) version of systemd
(the message was presumably eliminated by [1]).

[1]: systemd/systemd@d5aecba

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

@kolyshkin @AkihiroSuda PTAL

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

@thaJeztah
Copy link
Member Author

thaJeztah commented May 11, 2022

Was having a chat with @tianon, and he raised the question "how stable are these fixed numbers guaranteed to be"?

Curious; would it be an idea (and is there a good way?) to dynamically find the major/minor numbers (with a sync.Once) to propagate this list? Perhaps you have ideas on that @kolyshkin ?

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@kolyshkin
Copy link
Contributor

"how stable are these fixed numbers guaranteed to be"?

Practically, they are set in stone. Not that there is any standard saying that /dev/net/tun is 10:200 or /dev/null is 1:3, but I bet these will never ever change.

@fuweid
Copy link
Member

fuweid commented May 12, 2022

"how stable are these fixed numbers guaranteed to be"?

Practically, they are set in stone. Not that there is any standard saying that /dev/net/tun is 10:200 or /dev/null is 1:3, but I bet these will never ever change.

no standard but 10:200 show in the https://www.kernel.org/doc/Documentation/networking/tuntap.txt doc 😂

@estesp estesp merged commit 9aa6725 into containerd:main May 12, 2022
@thaJeztah thaJeztah deleted the no_tun branch May 12, 2022 14:22
@thaJeztah
Copy link
Member Author

Practically, they are set in stone. Not that there is any standard saying that /dev/net/tun is 10:200 or /dev/null is 1:3, but I bet these will never ever change.

no standard but 10:200 show in the https://www.kernel.org/doc/Documentation/networking/tuntap.txt doc

Yes, that was roughly my expectation; "unlikely" to change (famous last words! 😂); was wondering if that's documented somewhere (and if there would be a place to document it, or to "formalise" it).

I guess if they ever decide to change that, we'll notice that pretty fast.

@jdef
Copy link

jdef commented Aug 4, 2022

kindly request that this is backported to release/1.5 and/or release/1.6

@fuweid
Copy link
Member

fuweid commented Aug 5, 2022

kindly request that this is backported to release/1.5 and/or release/1.6

Go ahead please cc @thaJeztah

@thaJeztah
Copy link
Member Author

Opened #7267 and #7268

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked/1.5.x PR commits are cherry-picked into release/1.5 branch cherry-picked/1.6.x PR commits are cherry-picked into release/1.6 branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants