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

[3.2.3-rhel] seccomp: propagate defaultErrnoRet #11039

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Jul 25, 2021

(same as #11033 but for v3.2.3-rhel branch)

This fixes the issue of not propagating defaultErrnoRet from
system's seccomp.json to container's config.json, which can
result in redundant seccomp rules and inability to start a container
when using some runtimes (notably runc up to and including 1.0.1).

The fix is to vendor in containers/common with the fixed pkg/seccomp
which no longer ignores defaultErrnoRet.

Fixes: #11031
Fixes: bugzilla.redhat.com/1987049

Proposed changelog entry

Bugfixes:
 * Fixed a bug of ignoring the `defaultErrnoRet` field in system's seccomp.json,
   which may cause an "error adding seccomp filter rule for syscall" error
   and a failure to start a container when using `--runtime=runc` (#11031).

This fixes the issue of not propagating defaultErrnoRet from
system's seccomp.json to container's config.json, which can
result in redundant seccomp rules and inability to start a container
when using some runtimes (notably runc up to and including 1.0.1).

The fix is to vendor in containers/common with the fixed pkg/seccomp
which no longer ignores defaultErrnoRet.

Brought to you by:

	go get github.com/containers/[email protected]
	make vendor

Signed-off-by: Kir Kolyshkin <[email protected]>
@mheon
Copy link
Member

mheon commented Jul 25, 2021 via email

@kolyshkin
Copy link
Contributor Author

Do we have a new-enough c/common in 8.4.0.2 to require this? If so, I think
we definitely need a BZ spun up to get this landed.

I assume we don't. As long as the vendored version is used for building podman in rhel (which I believe is how things are), we do not care about what version of containers-common rpm is installed.

@vrothberg
Copy link
Member

Yes, we need a BZ to merge any PR into the -rhel branch (also to start rebuilding packages). @kolyshkin, would you open one?

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 26, 2021
@kolyshkin
Copy link
Contributor Author

Yes, we need a BZ to merge any PR into the -rhel branch (also to start rebuilding packages). @kolyshkin, would you open one?

Done, please see bugzilla.redhat.com/1987049

Description updated.

Copy link
Member

@vrothberg vrothberg left a comment

Choose a reason for hiding this comment

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

/lgtm

Let's wait for an ACK in the BZ before merge. Thank you, Kir!

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jul 29, 2021
@rhatdan
Copy link
Member

rhatdan commented Jul 29, 2021

/approve

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jul 29, 2021

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kolyshkin, rhatdan

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jul 29, 2021
@rhatdan
Copy link
Member

rhatdan commented Jul 29, 2021

/hold cancel
Looks like we have acks.

@openshift-ci openshift-ci bot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 29, 2021
@openshift-merge-robot openshift-merge-robot merged commit 47dc7d7 into containers:v3.2.3-rhel Jul 29, 2021
@github-actions github-actions bot added the locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments. label Sep 22, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. locked - please file new issue/PR Assist humans wanting to comment on an old issue or PR with locked comments.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants