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

Fix race in runc exec #1812

Merged
merged 1 commit into from
Jun 4, 2018
Merged

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Jun 1, 2018

There is a race in runc exec when the init process stops just before
the check for the container status. It is then wrongly assumed that
we are trying to start an init process instead of an exec process.

Signed-off-by: Mrunal Patel mrunalp@gmail.com

@mrunalp
Copy link
Contributor Author

mrunalp commented Jun 1, 2018

@crosbymichael @cyphar @dqminh PTAL

@mrunalp
Copy link
Contributor Author

mrunalp commented Jun 1, 2018

This is triggered by healthchecks in Dockerfile and kubernetes readiness/liveness probes. Credit to Ulrich Obergfell for pinpointing this issue.

@mrunalp mrunalp force-pushed the fix_exec_race branch 3 times, most recently from c692f2c to ee36781 Compare June 1, 2018 23:05
There is a race in runc exec when the init process stops just before
the check for the container status. It is then wrongly assumed that
we are trying to start an init process instead of an exec process.

This commit add an Init field to libcontainer Process to distinguish
between init and exec processes to prevent this race.

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
Copy link
Contributor

@dqminh dqminh left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -140,6 +140,7 @@ func execProcess(context *cli.Context) (int, error) {
detach: detach,
pidFile: context.String("pid-file"),
action: CT_ACT_RUN,
init: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: should not need this.

@dqminh
Copy link
Contributor

dqminh commented Jun 2, 2018

LGTM

Approved with PullApprove

@cyphar
Copy link
Member

cyphar commented Jun 4, 2018

LGTM.

Approved with PullApprove

@cyphar cyphar merged commit bd3c4f8 into opencontainers:master Jun 4, 2018
cyphar added a commit that referenced this pull request Jun 4, 2018
  Fix race in runc exec

LGTMs: @dqminh @cyphar
Closes #1812
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.

3 participants