Skip to content
This repository has been archived by the owner on Jun 11, 2020. It is now read-only.

[17.06] backport: Avoid race when opening exec fifo #4

Closed
wants to merge 7 commits into from
Closed

[17.06] backport: Avoid race when opening exec fifo #4

wants to merge 7 commits into from

Conversation

andrewhsu
Copy link

backport:

with cherry-pick:

git cherry-pick -s -x 8d3e6c9 5c0af14

no conflicts

williammartin and others added 2 commits March 12, 2018 20:56
When starting a container with `runc start` or `runc run`, the stub
process (runc[2:INIT]) opens a fifo for writing. Its parent runc process
will open the same fifo for reading. In this way, they synchronize.

If the stub process exits at the wrong time, the parent runc process
will block forever.

This can happen when racing 2 runc operations against each other: `runc
run/start`, and `runc delete`. It could also happen for other reasons,
e.g. the kernel's OOM killer may select the stub process.

This commit resolves this race by racing the opening of the exec fifo
from the runc parent process against the stub process exiting. If the
stub process exits before we open the fifo, we return an error.

Another solution is to wait on the stub process. However, it seems it
would require more refactoring to avoid calling wait multiple times on
the same process, which is an error.

Signed-off-by: Craig Furman <cfurman@pivotal.io>
(cherry picked from commit 8d3e6c9)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
Signed-off-by: Craig Furman <cfurman@pivotal.io>
(cherry picked from commit 5c0af14)
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
@tiborvass tiborvass changed the title WIP [17.06] backport: Avoid race when opening exec fifo [17.06] backport: Avoid race when opening exec fifo Mar 12, 2018
@tiborvass
Copy link

LGTM

andrewhsu and others added 5 commits March 13, 2018 00:29
Signed-off-by: Tibor Vass <tibor@docker.com>
Signed-off-by: Andrew Hsu <andrewhsu@docker.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
Signed-off-by: Tibor Vass <tibor@docker.com>
CRIU needs to load a few modules to checkpoint/resume containers.

opencontainers#1745
Signed-off-by: Andrei Vagin <avagin@virtuozzo.com>
And use it only in local tooling that is forwarding the pseudoterminal
master.  That way runC no longer has an opinion on the onlcr setting
for folks who are creating a terminal and detaching.  They'll use
--console-socket and can setup the pseudoterminal however they like
without runC having an opinion.  With this commit, the only cases
where runC still has applies SaneTerminal is when *it* is the process
consuming the master descriptor.

Signed-off-by: W. Trevor King <wking@tremily.us>
(cherry picked from commit 830c0d7)
Signed-off-by: Tibor Vass <tibor@docker.com>
@andrewhsu
Copy link
Author

This PR is not desired anymore.

@andrewhsu andrewhsu closed this Dec 3, 2018
@andrewhsu andrewhsu deleted the bonkers branch December 3, 2018 21:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants