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

[release/1.1 backport] update runc to 2b18fe1d885ee5083ef9f0838fee39b62d653e30 #3083

Merged
merged 4 commits into from
Mar 12, 2019

Conversation

thaJeztah
Copy link
Member

backport of #3081

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:

Signed-off-by: Sebastiaan van Stijn github@gone.nl
(cherry picked from commit b8d40b3)
Signed-off-by: Sebastiaan van Stijn github@gone.nl

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>
@@ -1,8 +1,9 @@
# OCI runtime-spec. When updating this, make sure you use a version tag rather
# than a commit ID so it's much more obvious what version of the spec we are
# using.
github.com/opencontainers/runtime-spec 5684b8af48c1ac3b1451fa499724e30e3c20a294
github.com/opencontainers/runtime-spec 29686dbc5559d93fb1ef402eeda3e35c38d75af4
Copy link
Member Author

Choose a reason for hiding this comment

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

do we want the vendored version to match this one?

Copy link
Member

Choose a reason for hiding this comment

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

I think it makes sense to use the runtime-spec that matches runc; IMO

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I'll update this PR, and update that as well, thanks!

@thaJeztah
Copy link
Member Author

how tightly coupled other dependencies are to the version of runc; going through history when the runtime-spec was updated (to double-check if local changes are needed for specific bumps), I arrived at #2500 as the first bump since the 1.1 release; that one also updates containerd/go-runc (and some other dependencies). I can update those as well, but wasn't sure if I should

@codecov-io
Copy link

codecov-io commented Mar 7, 2019

Codecov Report

Merging #3083 into release/1.1 will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##           release/1.1    #3083   +/-   ##
============================================
  Coverage        49.07%   49.07%           
============================================
  Files               85       85           
  Lines             7598     7598           
============================================
  Hits              3729     3729           
  Misses            3194     3194           
  Partials           675      675
Flag Coverage Δ
#linux 49.07% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 52bfc9f...21abff9. Read the comment docs.

@thaJeztah
Copy link
Member Author

how tightly coupled other dependencies are to the version of runc; going through history when the runtime-spec was updated (to double-check if local changes are needed for specific bumps), I arrived at #2500 as the first bump since the 1.1 release; that one also updates containerd/go-runc (and some other dependencies). I can update those as well, but wasn't sure if I should

@crosbymichael could you help with this? I was discussing with @estesp but we both weren't sure if those should be updated here as well

@crosbymichael
Copy link
Member

match them

@cyphar
Copy link
Contributor

cyphar commented Mar 8, 2019

You probably don't want to backport this yet -- I just realised the bind-mount approach isn't fool-proof. I'm working on a follow-up patch that should be ready soon.

@thaJeztah
Copy link
Member Author

Thanks, I just saw your comment; let me mark this one WIP

@thaJeztah thaJeztah changed the title [release/1.1 backport] update runc to 2b18fe1d885ee5083ef9f0838fee39b62d653e30 [WIP][release/1.1 backport] update runc to 2b18fe1d885ee5083ef9f0838fee39b62d653e30 Mar 8, 2019
@cyphar
Copy link
Contributor

cyphar commented Mar 10, 2019

You can drop the WIP -- I've closed opencontainers/runc#2006 after deciding that running CAP_SYS_ADMIN (in a non-userns container with AppArmor disabled) was always unsafe and it makes no sense to block a working fix based on that.

@estesp
Copy link
Member

estesp commented Mar 11, 2019

Hey @thaJeztah; sounds like we can go ahead with this, but I guess at this point you should do the vendor update per @crosbymichael's comment about keeping them in sync. Thanks!

@thaJeztah
Copy link
Member Author

Ahm yes, didn't finish that yet; let me make some time to finish the vendoring 🤗

crosbymichael and others added 3 commits March 11, 2019 22:49
Signed-off-by: Michael Crosby <crosbymichael@gmail.com>
(cherry picked from commit 5a0b040)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: Madhan Raj Mookkandy <madhanm@microsoft.com>
(cherry picked from commit 744d93e)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: John Howard <jhoward@microsoft.com>
(cherry picked from commit 98766e8)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
@thaJeztah
Copy link
Member Author

added #2500, #2644, and #2990

@thaJeztah thaJeztah changed the title [WIP][release/1.1 backport] update runc to 2b18fe1d885ee5083ef9f0838fee39b62d653e30 [release/1.1 backport] update runc to 2b18fe1d885ee5083ef9f0838fee39b62d653e30 Mar 11, 2019
@crosbymichael
Copy link
Member

LGTM

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

@estesp estesp merged commit 8ecb055 into containerd:release/1.1 Mar 12, 2019
@thaJeztah thaJeztah deleted the 1.1_backport_bump_runc branch March 12, 2019 18:34
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

6 participants