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

exec: expose --preserve-fds #1995

Merged
merged 1 commit into from
Feb 26, 2019

Conversation

giuseppe
Copy link
Member

The implementation is already there, we only need to add the CLI
option and pass it down.

Signed-off-by: Giuseppe Scrivano gscrivan@redhat.com

@cyphar
Copy link
Member

cyphar commented Feb 25, 2019

Seems reasonable, though I would like an integration test.

The implementation is already there, we only need to add the CLI
option and pass it down.

Signed-off-by: Giuseppe Scrivano <gscrivan@redhat.com>
@giuseppe
Copy link
Member Author

tests are passing now

@cyphar
Copy link
Member

cyphar commented Feb 25, 2019

LGTM.

Approved with PullApprove

@mrunalp
Copy link
Contributor

mrunalp commented Feb 25, 2019

LGTM

Approved with PullApprove

@mrunalp
Copy link
Contributor

mrunalp commented Feb 25, 2019

@caniszczyk pull-approve is taking a long time to detect LGTMs.

@mrunalp mrunalp merged commit f79e211 into opencontainers:master Feb 26, 2019
@giuseppe giuseppe deleted the exec-preserve-fds branch February 26, 2019 08:40
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the
increased memory-consumption introduced by the original patch,
RHEL 7.6 getting into a loop due to a kernel bug in those kernels,
and improve compatibility with older kernels.

changes included:

- opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc
- opencontainers/runc#1978 Remove detection for scope properties, which have always been broken
- opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition
- opencontainers/runc#1995 exec: expose --preserve-fds
- opencontainers/runc#2000 fix preserve-fds flag may cause runc hang
- opencontainers/runc#1968 Create bind mount mountpoints during restore
- opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the
increased memory-consumption introduced by the original patch,
RHEL 7.6 getting into a loop due to a kernel bug in those kernels,
and improve compatibility with older kernels.

changes included:

- opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc
- opencontainers/runc#1978 Remove detection for scope properties, which have always been broken
- opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition
- opencontainers/runc#1995 exec: expose --preserve-fds
- opencontainers/runc#2000 fix preserve-fds flag may cause runc hang
- opencontainers/runc#1968 Create bind mount mountpoints during restore
- opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b8d40b3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
thaJeztah added a commit to thaJeztah/containerd that referenced this pull request Mar 7, 2019
This includes an improved fix for CVE-2019-5736 to reduce the
increased memory-consumption introduced by the original patch,
RHEL 7.6 getting into a loop due to a kernel bug in those kernels,
and improve compatibility with older kernels.

changes included:

- opencontainers/runc#1973 Vendor opencontainers/runtime-spec 29686dbc
- opencontainers/runc#1978 Remove detection for scope properties, which have always been broken
- opencontainers/runc#1963 Vendor in go-criu and use it for CRIU's RPC definition
- opencontainers/runc#1995 exec: expose --preserve-fds
- opencontainers/runc#2000 fix preserve-fds flag may cause runc hang
- opencontainers/runc#1968 Create bind mount mountpoints during restore
- opencontainers/runc#1984 nsenter: cloned_binary: "memfd" cleanups

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
(cherry picked from commit b8d40b3)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
runc run -d --console-socket $CONSOLE_SOCKET test_busybox
[ "$status" -eq 0 ]

run bash -c "cat hello > preserve-fds.test; exec 3<preserve-fds.test; $RUNC --log /proc/self/fd/2 --root $ROOT exec --preserve-fds=1 test_busybox cat /proc/self/fd/3"
Copy link
Contributor

Choose a reason for hiding this comment

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

What I don't understand is why cat hello is not failing. There is no such file named hello.

Copy link
Member Author

Choose a reason for hiding this comment

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

I guess the error is just ignored but the file is still created by the shell. I guess it had to be echo but too long ago to remember :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean, yes, cat hello should fail and this is ignored by bash since it does not run with set -e.

What I don't understand is how the check below (that checks that $output contains hello works at all.

Copy link
Contributor

@kolyshkin kolyshkin Oct 22, 2020

Choose a reason for hiding this comment

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

@giuseppe pointed out that cat hello produces the message like cat: hello: no such file or directory, and thus the check for hello in the $output succeeds!

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

4 participants