X Tutup
Skip to content

Commit a560e5e

Browse files
AkihiroSudaAkihiroSuda
committed
mount: fix read-only bind (containerd#1368)
Signed-off-by: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
1 parent b4cc42d commit a560e5e

File tree

6 files changed

+112
-14
lines changed

6 files changed

+112
-14
lines changed

‎container_test.go‎

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -704,7 +704,12 @@ func TestContainerExecNoBinaryExists(t *testing.T) {
704704

705705
func TestUserNamespaces(t *testing.T) {
706706
t.Parallel()
707+
t.Run("WritableRootFS", func(t *testing.T) { testUserNamespaces(t, false) })
708+
// see #1373 and runc#1572
709+
t.Run("ReadonlyRootFS", func(t *testing.T) { testUserNamespaces(t, true) })
710+
}
707711

712+
func testUserNamespaces(t *testing.T, readonlyRootFS bool) {
708713
client, err := newClient(t, address)
709714
if err != nil {
710715
t.Fatal(err)
@@ -714,7 +719,7 @@ func TestUserNamespaces(t *testing.T) {
714719
var (
715720
image Image
716721
ctx, cancel = testContext()
717-
id = t.Name()
722+
id = strings.Replace(t.Name(), "/", "-", -1)
718723
)
719724
defer cancel()
720725

@@ -726,13 +731,17 @@ func TestUserNamespaces(t *testing.T) {
726731
}
727732
}
728733

729-
container, err := client.NewContainer(ctx, id,
730-
WithNewSpec(withImageConfig(image),
731-
withExitStatus(7),
732-
withUserNamespace(0, 1000, 10000),
733-
),
734-
withRemappedSnapshot(id, image, 1000, 1000),
735-
)
734+
opts := []NewContainerOpts{WithNewSpec(withImageConfig(image),
735+
withExitStatus(7),
736+
withUserNamespace(0, 1000, 10000),
737+
)}
738+
if readonlyRootFS {
739+
opts = append(opts, withRemappedSnapshotView(id, image, 1000, 1000))
740+
} else {
741+
opts = append(opts, withRemappedSnapshot(id, image, 1000, 1000))
742+
}
743+
744+
container, err := client.NewContainer(ctx, id, opts...)
736745
if err != nil {
737746
t.Error(err)
738747
return

‎helpers_unix_test.go‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ func withExecArgs(s *specs.Process, args ...string) {
4040
}
4141

4242
var (
43-
withUserNamespace = WithUserNamespace
44-
withRemappedSnapshot = WithRemappedSnapshot
45-
withNewSnapshot = WithNewSnapshot
46-
withImageConfig = WithImageConfig
43+
withUserNamespace = WithUserNamespace
44+
withRemappedSnapshot = WithRemappedSnapshot
45+
withRemappedSnapshotView = WithRemappedSnapshotView
46+
withNewSnapshot = WithNewSnapshot
47+
withImageConfig = WithImageConfig
4748
)

‎helpers_windows_test.go‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ func withRemappedSnapshot(id string, i Image, u, g uint32) NewContainerOpts {
6363
}
6464
}
6565

66+
func withRemappedSnapshotView(id string, i Image, u, g uint32) NewContainerOpts {
67+
return func(ctx context.Context, client *Client, c *containers.Container) error {
68+
return nil
69+
}
70+
}
71+
6672
func withNoop(_ context.Context, _ *Client, _ *containers.Container, _ *specs.Spec) error {
6773
return nil
6874
}

‎mount/mount_linux.go‎

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,36 @@ import (
88

99
func (m *Mount) Mount(target string) error {
1010
flags, data := parseMountOptions(m.Options)
11-
return unix.Mount(m.Source, target, m.Type, uintptr(flags), data)
11+
12+
// propagation types.
13+
const ptypes = unix.MS_SHARED | unix.MS_PRIVATE | unix.MS_SLAVE | unix.MS_UNBINDABLE
14+
15+
// Ensure propagation type change flags aren't included in other calls.
16+
oflags := flags &^ ptypes
17+
18+
// In the case of remounting with changed data (data != ""), need to call mount (moby/moby#34077).
19+
if flags&unix.MS_REMOUNT == 0 || data != "" {
20+
// Initial call applying all non-propagation flags for mount
21+
// or remount with changed data
22+
if err := unix.Mount(m.Source, target, m.Type, uintptr(oflags), data); err != nil {
23+
return err
24+
}
25+
}
26+
27+
if flags&ptypes != 0 {
28+
// Change the propagation type.
29+
const pflags = ptypes | unix.MS_REC | unix.MS_SILENT
30+
if err := unix.Mount("", target, "", uintptr(flags&pflags), ""); err != nil {
31+
return err
32+
}
33+
}
34+
35+
const broflags = unix.MS_BIND | unix.MS_RDONLY
36+
if oflags&broflags == broflags {
37+
// Remount the bind to apply read only.
38+
return unix.Mount("", target, "", uintptr(oflags|unix.MS_REMOUNT), "")
39+
}
40+
return nil
1241
}
1342

1443
func Unmount(mount string, flags int) error {

‎snapshot/testsuite/testsuite.go‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,8 @@ func SnapshotterSuite(t *testing.T, name string, snapshotterFn func(ctx context.
3838

3939
// Rename test still fails on some kernels with overlay
4040
//t.Run("Rename", makeTest(name, snapshotterFn, checkRename))
41+
42+
t.Run("ViewReadonly", makeTest(name, snapshotterFn, checkSnapshotterViewReadonly))
4143
}
4244

4345
func makeTest(name string, snapshotterFn func(ctx context.Context, root string) (snapshot.Snapshotter, func() error, error), fn func(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string)) func(t *testing.T) {
@@ -651,3 +653,40 @@ func checkRemove(ctx context.Context, t *testing.T, snapshotter snapshot.Snapsho
651653
t.Fatal(err)
652654
}
653655
}
656+
657+
// checkSnapshotterViewReadonly ensures a KindView snapshot to be mounted as a read-only filesystem.
658+
// This function is called only when WithTestViewReadonly is true.
659+
func checkSnapshotterViewReadonly(ctx context.Context, t *testing.T, snapshotter snapshot.Snapshotter, work string) {
660+
preparing := filepath.Join(work, "preparing")
661+
if _, err := snapshotter.Prepare(ctx, preparing, ""); err != nil {
662+
t.Fatal(err)
663+
}
664+
committed := filepath.Join(work, "commited")
665+
if err := snapshotter.Commit(ctx, committed, preparing); err != nil {
666+
t.Fatal(err)
667+
}
668+
view := filepath.Join(work, "view")
669+
m, err := snapshotter.View(ctx, view, committed)
670+
if err != nil {
671+
t.Fatal(err)
672+
}
673+
viewMountPoint := filepath.Join(work, "view-mount")
674+
if err := os.MkdirAll(viewMountPoint, 0777); err != nil {
675+
t.Fatal(err)
676+
}
677+
678+
// Just checking the option string of m is not enough, we need to test real mount. (#1368)
679+
if err := mount.MountAll(m, viewMountPoint); err != nil {
680+
t.Fatal(err)
681+
}
682+
683+
testfile := filepath.Join(viewMountPoint, "testfile")
684+
if err := ioutil.WriteFile(testfile, []byte("testcontent"), 0777); err != nil {
685+
t.Logf("write to %q failed with %v (EROFS is expected but can be other error code)", testfile, err)
686+
} else {
687+
t.Fatalf("write to %q should fail (EROFS) but did not fail", testfile)
688+
}
689+
testutil.Unmount(t, viewMountPoint)
690+
assert.NoError(t, snapshotter.Remove(ctx, view))
691+
assert.NoError(t, snapshotter.Remove(ctx, committed))
692+
}

‎spec_opts_unix.go‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,15 @@ func WithUserNamespace(container, host, size uint32) SpecOpts {
225225
// WithRemappedSnapshot creates a new snapshot and remaps the uid/gid for the
226226
// filesystem to be used by a container with user namespaces
227227
func WithRemappedSnapshot(id string, i Image, uid, gid uint32) NewContainerOpts {
228+
return withRemappedSnapshotBase(id, i, uid, gid, false)
229+
}
230+
231+
// WithRemappedSnapshotView is similar to WithRemappedSnapshot but rootfs is mounted as read-only.
232+
func WithRemappedSnapshotView(id string, i Image, uid, gid uint32) NewContainerOpts {
233+
return withRemappedSnapshotBase(id, i, uid, gid, true)
234+
}
235+
236+
func withRemappedSnapshotBase(id string, i Image, uid, gid uint32, readonly bool) NewContainerOpts {
228237
return func(ctx context.Context, client *Client, c *containers.Container) error {
229238
diffIDs, err := i.(*image).i.RootFS(ctx, client.ContentStore())
230239
if err != nil {
@@ -257,7 +266,12 @@ func WithRemappedSnapshot(id string, i Image, uid, gid uint32) NewContainerOpts
257266
if err := snapshotter.Commit(ctx, usernsID, usernsID+"-remap"); err != nil {
258267
return err
259268
}
260-
if _, err := snapshotter.Prepare(ctx, id, usernsID); err != nil {
269+
if readonly {
270+
_, err = snapshotter.View(ctx, id, usernsID)
271+
} else {
272+
_, err = snapshotter.Prepare(ctx, id, usernsID)
273+
}
274+
if err != nil {
261275
return err
262276
}
263277
c.RootFS = id

0 commit comments

Comments
 (0)
X Tutup