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.

Sign up for GitHub

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

Jump to bottom

[1.0] Fix failure with read-only /dev in spec #3277

Merged
merged 3 commits into from
Nov 29, 2021
Merged

[1.0] Fix failure with read-only /dev in spec #3277

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3640499
libct/rootfs: consolidate utils imports
kailun-qin Jun 30, 2021
e802cfa
test/int/mount.bats: refer to github issue
kolyshkin Nov 12, 2021
cbb2367
runc run: fix ro /dev
kolyshkin Nov 12, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 13 additions & 19 deletions libcontainer/rootfs_linux.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import (
"github.com/opencontainers/runc/libcontainer/devices"
"github.com/opencontainers/runc/libcontainer/userns"
"github.com/opencontainers/runc/libcontainer/utils"
libcontainerUtils "github.com/opencontainers/runc/libcontainer/utils"
"github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/selinux/go-selinux/label"
"github.com/sirupsen/logrus"
Expand All @@ -42,7 +41,7 @@ type mountConfig struct {
// needsSetupDev returns true if /dev needs to be set up.
func needsSetupDev(config *configs.Config) bool {
for _, m := range config.Mounts {
if m.Device == "bind" && libcontainerUtils.CleanPath(m.Destination) == "/dev" {
if m.Device == "bind" && utils.CleanPath(m.Destination) == "/dev" {
return false
}
}
Expand Down Expand Up @@ -154,15 +153,16 @@ func prepareRootfs(pipe io.ReadWriter, iConfig *initConfig) (err error) {
// finalizeRootfs sets anything to ro if necessary. You must call
// prepareRootfs first.
func finalizeRootfs(config *configs.Config) (err error) {
// remount dev as ro if specified
// All tmpfs mounts and /dev were previously mounted as rw
// by mountPropagate. Remount them read-only as requested.
for _, m := range config.Mounts {
if libcontainerUtils.CleanPath(m.Destination) == "/dev" {
if m.Flags&unix.MS_RDONLY == unix.MS_RDONLY {
if err := remountReadonly(m); err != nil {
return newSystemErrorWithCausef(err, "remounting %q as readonly", m.Destination)
}
if m.Flags&unix.MS_RDONLY != unix.MS_RDONLY {
continue
}
if m.Device == "tmpfs" || utils.CleanPath(m.Destination) == "/dev" {
if err := remountReadonly(m); err != nil {
return newSystemErrorWithCausef(err, "remounting %q as readonly", m.Destination)
}
break
}
}

Expand Down Expand Up @@ -432,12 +432,6 @@ func mountToRootfs(m *configs.Mount, c *mountConfig) error {
return err
}
}
// Initially mounted rw in mountPropagate, remount to ro if flag set.
if m.Flags&unix.MS_RDONLY != 0 {
if err := remount(m, rootfs); err != nil {
return err
}
}
return nil
case "bind":
if err := prepareBindMount(m, rootfs); err != nil {
Expand Down Expand Up @@ -1047,10 +1041,10 @@ func mountPropagate(m *configs.Mount, rootfs string, mountLabel string) error {
flags = m.Flags
)
// Delay mounting the filesystem read-only if we need to do further
// operations on it. We need to set up files in "/dev" and tmpfs mounts may
// need to be chmod-ed after mounting. The mount will be remounted ro later
// in finalizeRootfs() if necessary.
if libcontainerUtils.CleanPath(m.Destination) == "/dev" || m.Device == "tmpfs" {
// operations on it. We need to set up files in "/dev", and other tmpfs
// mounts may need to be chmod-ed after mounting. These mounts will be
// remounted ro later in finalizeRootfs(), if necessary.
if m.Device == "tmpfs" || utils.CleanPath(m.Destination) == "/dev" {
flags &= ^unix.MS_RDONLY
}

Expand Down
11 changes: 11 additions & 0 deletions tests/integration/mounts.bats
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ function teardown() {
[[ "${lines[0]}" == *'/tmp/bind/config.json'* ]]
}

# https://github.com/opencontainers/runc/issues/2246
@test "runc run [ro tmpfs mount]" {
update_config ' .mounts += [{
source: "tmpfs",
Expand All @@ -37,6 +38,16 @@ function teardown() {
[[ "${lines[0]}" == *'ro,'* ]]
}

# https://github.com/opencontainers/runc/issues/3248
@test "runc run [ro /dev mount]" {
update_config ' .mounts |= map((select(.destination == "/dev") | .options += ["ro"]) // .)
| .process.args |= ["grep", "^tmpfs /dev", "/proc/mounts"]'

runc run test_busybox
[ "$status" -eq 0 ]
[[ "${lines[0]}" == *'ro,'* ]]
}

# https://github.com/opencontainers/runc/issues/2683
@test "runc run [tmpfs mount with absolute symlink]" {
# in container, /conf -> /real/conf
Expand Down