Fix mounting extra block device

The current implementation doesn't work for extra block device mounting. An error is returned "rpc
error: code = Unknown desc = failed to create VM: failed to patch drive mount stub: failed to expose
patched drive contents to jail: file exists". For more contexts, read this PR: firecracker-microvm#522

This commit is for fixing the above issue.

Signed-off-by: Royce Zhao <[email protected]>

Author: RoyceDavison

internal/fsutil.go

@@ -21,6 +21,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"testing"
 
 	"github.com/pkg/errors"
@@ -33,7 +34,7 @@ func CreateFSImg(ctx context.Context, t *testing.T, fsType string, testFiles ...
 	t.Helper()
 
 	switch fsType {
-	case "ext3", "ext4":
+	case "ext4":
 		return createTestExtImg(ctx, t, fsType, testFiles...)
 	default:
 		require.FailNowf(t, "unsupported fs type %q", fsType)
@@ -72,6 +73,48 @@ func createTestExtImg(ctx context.Context, t *testing.T, extName string, testFil
 	return imgFile.Name()
 }
 
+// BlockDeviceFile represents a block device
+type BlockDeviceFile struct {
+	Subpath string
+	UID     int
+	GID     int
+	Dev     int // major number
+}
+
+// CreateBlockDevice creates a block device, or block special file for testing
+func CreateBlockDevice(ctx context.Context, t *testing.T, fsType string, testFile BlockDeviceFile) string {
+	t.Helper()
+
+	switch fsType {
+	case "ext4":
+		return createTestBlockDevice(ctx, t, fsType, testFile)
+	default:
+		require.FailNowf(t, "unsupported fs type %q", fsType)
+		return ""
+	}
+}
+
+func createTestBlockDevice(ctx context.Context, t *testing.T, extName string, testFile BlockDeviceFile) string {
+	t.Helper()
+
+	deviceFile := filepath.Join("/tmp/blockExample", testFile.Subpath)
+	err := os.MkdirAll(filepath.Dir(deviceFile), 0750)
+	require.NoError(t, err, "failed to mkdir for the block device")
+
+	err = syscall.Mknod(deviceFile, syscall.S_IFBLK, testFile.Dev)
+	require.NoError(t, err, "failed to create a new block device")
+
+	err = os.Chmod(deviceFile, 0600)
+	require.NoError(t, err, "failed to change file mode for the new created block device")
+
+	err = os.Chown(deviceFile, testFile.UID, testFile.GID)
+	require.NoError(t, err, "failed to grant permission to the new created block device")
+
+	output, err := exec.CommandContext(ctx, "mkfs."+extName, "-v", deviceFile).CombinedOutput()
+	require.NoErrorf(t, err, "failed to create ext img, command output:%s \n", string(output))
+	return deviceFile
+}
+
 // MountInfo holds data parsed from a line of /proc/mounts
 type MountInfo struct {
 	SourcePath string

runtime/jailer_integ_test.go

@@ -35,6 +35,11 @@ import (
 	"github.com/firecracker-microvm/firecracker-containerd/runtime/firecrackeroci"
 )
 
+const (
+	jailerUID = 300001
+	jailerGID = 300001
+)
+
 func TestJailer_Isolated(t *testing.T) {
 	prepareIntegTest(t)
 	t.Run("Without Jailer", func(t *testing.T) {
@@ -44,15 +49,35 @@ func TestJailer_Isolated(t *testing.T) {
 	t.Run("With Jailer", func(t *testing.T) {
 		t.Parallel()
 		testJailer(t, &proto.JailerConfig{
-			UID: 300001,
-			GID: 300001,
+			UID: jailerUID,
+			GID: jailerGID,
 		})
 	})
 	t.Run("With Jailer and bind-mount", func(t *testing.T) {
 		t.Parallel()
 		testJailer(t, &proto.JailerConfig{
-			UID:               300001,
-			GID:               300001,
+			UID:               jailerUID,
+			GID:               jailerGID,
+			DriveExposePolicy: proto.DriveExposePolicy_BIND,
+		})
+	})
+}
+
+func TestAttachBlockDevice_Isolated(t *testing.T) {
+	prepareIntegTest(t)
+	t.Run("Without Jailer", func(t *testing.T) {
+		testAttachBlockDevice(t, nil)
+	})
+	t.Run("With Jailer", func(t *testing.T) {
+		testAttachBlockDevice(t, &proto.JailerConfig{
+			UID: jailerUID,
+			GID: jailerGID,
+		})
+	})
+	t.Run("With Jailer and bind-mount", func(t *testing.T) {
+		testAttachBlockDevice(t, &proto.JailerConfig{
+			UID:               jailerUID,
+			GID:               jailerGID,
 			DriveExposePolicy: proto.DriveExposePolicy_BIND,
 		})
 	})
@@ -170,3 +195,110 @@ func TestJailerCPUSet_Isolated(t *testing.T) {
 	}
 	testJailer(t, config)
 }
+
+func testAttachBlockDevice(t *testing.T, jailerConfig *proto.JailerConfig) {
+	require := require.New(t)
+	client, err := containerd.New(containerdSockPath, containerd.WithDefaultRuntime(firecrackerRuntime))
+	require.NoError(err, "unable to create client to containerd service at %s, is containerd running?", containerdSockPath)
+	defer client.Close()
+
+	ctx := namespaces.WithNamespace(context.Background(), "default")
+
+	image, err := alpineImage(ctx, client, defaultSnapshotterName)
+	require.NoError(err, "failed to get alpine image")
+
+	fcClient, err := newFCControlClient(containerdSockPath)
+	require.NoError(err)
+
+	vmID := testNameToVMID(t.Name())
+
+	// default BlockDeviceFile setup, will change UID & GID if a jailerConfig is passed
+	deviceFile := internal.BlockDeviceFile{
+		Subpath: "dir/block1",
+		UID:     0,
+		GID:     0,
+		Dev:     259, // 259 is the major number for blkext which is one type of block devices
+	}
+
+	if jailerConfig != nil {
+		// cover "With Jailer" & "With Jailer and bind-mount" test cases
+		deviceFile.UID = int(jailerConfig.UID)
+		deviceFile.GID = int(jailerConfig.GID)
+	}
+	additionalBlockDevice := internal.CreateBlockDevice(ctx, t, "ext4", deviceFile)
+	blockExampleDir := filepath.Dir(additionalBlockDevice)
+	defer os.RemoveAll(filepath.Dir(blockExampleDir)) // clean up
+
+	request := proto.CreateVMRequest{
+		VMID:         vmID,
+		JailerConfig: jailerConfig,
+		DriveMounts: []*proto.FirecrackerDriveMount{
+			{HostPath: additionalBlockDevice, VMPath: "/home/driveMount", FilesystemType: "ext4"},
+		},
+	}
+
+	// If the drive files are bind-mounted, the files must be readable from the jailer's user.
+	if jailerConfig != nil && jailerConfig.DriveExposePolicy == proto.DriveExposePolicy_BIND {
+		f, err := ioutil.TempFile("", fsSafeTestName(t)+"_rootfs")
+		require.NoError(err)
+		defer f.Close()
+
+		dst := f.Name()
+
+		// Copy the root drive before chown, since the file is used by other tests.
+		err = copyFile(defaultRuntimeConfig.RootDrive, dst, 0400)
+		require.NoErrorf(err, "failed to copy a rootfs as %q", dst)
+
+		err = os.Chown(dst, int(jailerConfig.UID), int(jailerConfig.GID))
+		require.NoError(err, "failed to chown %q", dst)
+
+		request.RootDrive = &proto.FirecrackerRootDrive{HostPath: dst}
+
+		// The additional drive file is only used by this test.
+		err = os.Chown(additionalBlockDevice, int(jailerConfig.UID), int(jailerConfig.GID))
+		require.NoError(err, "failed to chown %q", additionalBlockDevice)
+	}
+
+	_, err = fcClient.CreateVM(ctx, &request)
+	require.NoError(err)
+
+	// create a container to test bind mount block device into the container
+	c, err := client.NewContainer(ctx,
+		vmID+"-container",
+		containerd.WithSnapshotter(defaultSnapshotterName),
+		containerd.WithNewSnapshot(vmID+"-snapshot", image),
+		containerd.WithNewSpec(
+			oci.WithProcessArgs(
+				"/bin/sh", "-c", "echo heyhey && cd /mnt/blockDeviceTest",
+			),
+			firecrackeroci.WithVMID(vmID),
+			oci.WithMounts([]specs.Mount{{
+				Source:      "/home/driveMount",
+				Destination: "/mnt/blockDeviceTest",
+				Options:     []string{"bind"},
+			}}),
+		),
+	)
+	require.NoError(err)
+
+	stdout := startAndWaitTask(ctx, t, c)
+	require.Equal("heyhey\n", stdout)
+
+	stat, err := os.Stat(filepath.Join(shimBaseDir(), "default#"+vmID))
+	require.NoError(err)
+	assert.True(t, stat.IsDir())
+
+	err = c.Delete(ctx, containerd.WithSnapshotCleanup)
+	require.NoError(err, "failed to delete a container-block-device")
+
+	_, err = fcClient.StopVM(ctx, &proto.StopVMRequest{VMID: vmID})
+	require.NoError(err)
+
+	_, err = os.Stat(filepath.Join(shimBaseDir(), "default#"+vmID))
+	assert.Error(t, err)
+	assert.True(t, os.IsNotExist(err))
+
+	shimContents, err := ioutil.ReadDir(shimBaseDir())
+	require.NoError(err)
+	assert.Len(t, shimContents, 0)
+}

runtime/runc_jailer.go

@@ -138,10 +138,18 @@ func newRuncJailer(
 
 func (j *runcJailer) prepareBindMounts(mounts []*proto.FirecrackerDriveMount) error {
 	for _, m := range mounts {
-		err := j.bindMountFileToJail(m.HostPath, filepath.Join(j.RootPath(), m.HostPath))
-		if err != nil {
+		stat := syscall.Stat_t{}
+		if err := syscall.Stat(m.HostPath, &stat); err != nil {
 			return err
 		}
+		// Only bindMount regular file.
+		// To avoid duplicate files, for block device, we will use system call to create device file for it.
+		if stat.Mode&syscall.S_IFMT == syscall.S_IFREG {
+			err := j.bindMountFileToJail(m.HostPath, filepath.Join(j.RootPath(), m.HostPath))
+			if err != nil {
+				return err
+			}
+		}
 	}
 	return nil
 }

runtime/service_integ_test.go

@@ -1007,7 +1007,7 @@ func TestDriveMount_Isolated(t *testing.T) {
 		},
 		{
 			VMPath:         "/mnt",
-			FilesystemType: "ext3",
+			FilesystemType: "ext4",
 			// don't specify "ro" to validate it's automatically set via "IsWritable: false"
 			VMMountOptions: []string{"relatime"},
 			ContainerPath:  "/bar",