diff --git a/virtcontainers/hyperstart_agent.go b/virtcontainers/hyperstart_agent.go index 6c8202577..648acd506 100644 --- a/virtcontainers/hyperstart_agent.go +++ b/virtcontainers/hyperstart_agent.go @@ -739,12 +739,17 @@ func (h *hyper) register() error { } defer h.disconnect() + console, err := h.sandbox.hypervisor.getSandboxConsole(h.sandbox.id) + if err != nil { + return err + } + registerVMOptions := &proxyClient.RegisterVMOptions{ - Console: h.sandbox.hypervisor.getSandboxConsole(h.sandbox.id), + Console: console, NumIOStreams: 0, } - _, err := h.client.RegisterVM(h.sandbox.id, h.sockets[0].HostPath, + _, err = h.client.RegisterVM(h.sandbox.id, h.sockets[0].HostPath, h.sockets[1].HostPath, registerVMOptions) return err } diff --git a/virtcontainers/hypervisor.go b/virtcontainers/hypervisor.go index 3dd3a44a6..2e0907ca8 100644 --- a/virtcontainers/hypervisor.go +++ b/virtcontainers/hypervisor.go @@ -501,6 +501,6 @@ type hypervisor interface { addDevice(devInfo interface{}, devType deviceType) error hotplugAddDevice(devInfo interface{}, devType deviceType) error hotplugRemoveDevice(devInfo interface{}, devType deviceType) error - getSandboxConsole(sandboxID string) string + getSandboxConsole(sandboxID string) (string, error) capabilities() capabilities } diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 4e835200f..4deb2c6f3 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -33,21 +33,21 @@ import ( ) var ( - defaultKataSockPathTemplate = "%s/%s/kata.sock" - defaultKataChannel = "agent.channel.0" - defaultKataDeviceID = "channel0" - defaultKataID = "charch0" - errorMissingProxy = errors.New("Missing proxy pointer") - errorMissingOCISpec = errors.New("Missing OCI specification") - kataHostSharedDir = "/run/kata-containers/shared/sandboxes/" - kataGuestSharedDir = "/run/kata-containers/shared/containers/" - mountGuest9pTag = "kataShared" - type9pFs = "9p" - vsockSocketScheme = "vsock" - kata9pDevType = "9p" - kataBlkDevType = "blk" - kataSCSIDevType = "scsi" - sharedDir9pOptions = []string{"trans=virtio,version=9p2000.L", "nodev"} + defaultKataSocketName = "kata.sock" + defaultKataChannel = "agent.channel.0" + defaultKataDeviceID = "channel0" + defaultKataID = "charch0" + errorMissingProxy = errors.New("Missing proxy pointer") + errorMissingOCISpec = errors.New("Missing OCI specification") + kataHostSharedDir = "/run/kata-containers/shared/sandboxes/" + kataGuestSharedDir = "/run/kata-containers/shared/containers/" + mountGuest9pTag = "kataShared" + type9pFs = "9p" + vsockSocketScheme = "vsock" + kata9pDevType = "9p" + kataBlkDevType = "blk" + kataSCSIDevType = "scsi" + sharedDir9pOptions = []string{"trans=virtio,version=9p2000.L", "nodev"} ) // KataAgentConfig is a structure storing information needed @@ -114,10 +114,15 @@ func (k *kataAgent) generateVMSocket(sandbox *Sandbox, c KataAgentConfig) error cid, port, err := parseVSOCKAddr(c.GRPCSocket) if err != nil { // We need to generate a host UNIX socket path for the emulated serial port. + kataSock, err := utils.BuildSocketPath(runStoragePath, sandbox.id, defaultKataSocketName) + if err != nil { + return err + } + k.vmSocket = Socket{ DeviceID: defaultKataDeviceID, ID: defaultKataID, - HostPath: fmt.Sprintf(defaultKataSockPathTemplate, runStoragePath, sandbox.id), + HostPath: kataSock, Name: defaultKataChannel, } } else { diff --git a/virtcontainers/kata_builtin_proxy.go b/virtcontainers/kata_builtin_proxy.go index 9beef08c6..f5f67743b 100644 --- a/virtcontainers/kata_builtin_proxy.go +++ b/virtcontainers/kata_builtin_proxy.go @@ -30,8 +30,12 @@ func (p *kataBuiltInProxy) start(sandbox *Sandbox, params proxyParams) (int, str } p.sandboxID = sandbox.id - console := sandbox.hypervisor.getSandboxConsole(sandbox.id) - err := p.watchConsole(consoleProtoUnix, console, params.logger) + console, err := sandbox.hypervisor.getSandboxConsole(sandbox.id) + if err != nil { + return -1, "", err + } + + err = p.watchConsole(consoleProtoUnix, console, params.logger) if err != nil { return -1, "", err } diff --git a/virtcontainers/kata_proxy.go b/virtcontainers/kata_proxy.go index 93cb3b0ff..608b1d6f0 100644 --- a/virtcontainers/kata_proxy.go +++ b/virtcontainers/kata_proxy.go @@ -41,7 +41,12 @@ func (p *kataProxy) start(sandbox *Sandbox, params proxyParams) (int, string, er args := []string{config.Path, "-listen-socket", proxyURL, "-mux-socket", params.agentURL} if config.Debug { args = append(args, "-log", "debug") - args = append(args, "-agent-logs-socket", sandbox.hypervisor.getSandboxConsole(sandbox.id)) + console, err := sandbox.hypervisor.getSandboxConsole(sandbox.id) + if err != nil { + return -1, "", err + } + + args = append(args, "-agent-logs-socket", console) } cmd := exec.Command(args[0], args[1:]...) diff --git a/virtcontainers/mock_hypervisor.go b/virtcontainers/mock_hypervisor.go index 15bc8e9b5..563a395ca 100644 --- a/virtcontainers/mock_hypervisor.go +++ b/virtcontainers/mock_hypervisor.go @@ -57,6 +57,6 @@ func (m *mockHypervisor) hotplugRemoveDevice(devInfo interface{}, devType device return nil } -func (m *mockHypervisor) getSandboxConsole(sandboxID string) string { - return "" +func (m *mockHypervisor) getSandboxConsole(sandboxID string) (string, error) { + return "", nil } diff --git a/virtcontainers/mock_hypervisor_test.go b/virtcontainers/mock_hypervisor_test.go index 746b472fa..86a859035 100644 --- a/virtcontainers/mock_hypervisor_test.go +++ b/virtcontainers/mock_hypervisor_test.go @@ -87,7 +87,12 @@ func TestMockHypervisorGetSandboxConsole(t *testing.T) { expected := "" - if result := m.getSandboxConsole("testSandboxID"); result != expected { + result, err := m.getSandboxConsole("testSandboxID") + if err != nil { + t.Fatal(err) + } + + if result != expected { t.Fatalf("Got %s\nExpecting %s", result, expected) } } diff --git a/virtcontainers/qemu.go b/virtcontainers/qemu.go index dc0e25112..95ae5c826 100644 --- a/virtcontainers/qemu.go +++ b/virtcontainers/qemu.go @@ -7,6 +7,7 @@ package virtcontainers import ( "context" + "errors" "fmt" "os" "path/filepath" @@ -60,8 +61,6 @@ type qemu struct { const qmpCapErrMsg = "Failed to negoatiate QMP capabilities" -const qmpSockPathSizeLimit = 107 - const defaultConsole = "console.sock" // agnostic list of kernel parameters @@ -221,20 +220,49 @@ func (q *qemu) memoryTopology(sandboxConfig SandboxConfig) (govmmQemu.Memory, er } func (q *qemu) qmpSocketPath(socketName string) (string, error) { + if socketName == "" { + return "", errors.New("need socket name") + } + parentDirPath := filepath.Join(runStoragePath, q.sandbox.id) - if len(parentDirPath) > qmpSockPathSizeLimit { - return "", fmt.Errorf("Parent directory path %q is too long "+ - "(%d characters), could not add any path for the QMP socket", - parentDirPath, len(parentDirPath)) + + dir, err := utils.BuildSocketPath(parentDirPath) + if err != nil { + return "", err } - path := fmt.Sprintf("%s/%s-%s", parentDirPath, socketName, q.state.UUID) + name := fmt.Sprintf("%s-%s", socketName, q.state.UUID) - if len(path) > qmpSockPathSizeLimit { - return path[:qmpSockPathSizeLimit], nil + path, err := utils.BuildSocketPath(dir, name) + if err == nil { + return path, nil } - return path, nil + // The socket path is too long so truncate up to a minimum length. + + // The minimum path length we're prepared to use (based on current + // values) + const minNameLen = 12 + + dirLen := len(dir) + + // '-1' is for the addition of a path separator + availableNameLen := utils.MaxSocketPathLen - dirLen - 1 + + if availableNameLen < minNameLen { + return "", fmt.Errorf("QMP socket name cannot be shortened: %v", name) + } + + new := name[:availableNameLen] + + q.Logger().WithFields(logrus.Fields{ + "original-name": name, + "new-name": new, + }).Warnf("shortening QMP socket name") + + name = new + + return utils.BuildSocketPath(dir, name) } func (q *qemu) getQemuMachine(sandboxConfig SandboxConfig) (govmmQemu.Machine, error) { @@ -362,7 +390,12 @@ func (q *qemu) createSandbox(sandboxConfig SandboxConfig) error { devices = q.arch.appendBridges(devices, q.state.Bridges) devices = q.arch.append9PVolumes(devices, sandboxConfig.Volumes) - devices = q.arch.appendConsole(devices, q.getSandboxConsole(sandboxConfig.ID)) + console, err := q.getSandboxConsole(sandboxConfig.ID) + if err != nil { + return err + } + + devices = q.arch.appendConsole(devices, console) if initrdPath == "" { devices, err = q.appendImage(devices) @@ -874,6 +907,6 @@ func (q *qemu) addDevice(devInfo interface{}, devType deviceType) error { // getSandboxConsole builds the path of the console where we can read // logs coming from the sandbox. -func (q *qemu) getSandboxConsole(sandboxID string) string { - return filepath.Join(runStoragePath, sandboxID, defaultConsole) +func (q *qemu) getSandboxConsole(sandboxID string) (string, error) { + return utils.BuildSocketPath(runStoragePath, sandboxID, defaultConsole) } diff --git a/virtcontainers/qemu_test.go b/virtcontainers/qemu_test.go index b06124bbf..14751db5e 100644 --- a/virtcontainers/qemu_test.go +++ b/virtcontainers/qemu_test.go @@ -254,7 +254,12 @@ func TestQemuGetSandboxConsole(t *testing.T) { sandboxID := "testSandboxID" expected := filepath.Join(runStoragePath, sandboxID, defaultConsole) - if result := q.getSandboxConsole(sandboxID); result != expected { + result, err := q.getSandboxConsole(sandboxID) + if err != nil { + t.Fatal(err) + } + + if result != expected { t.Fatalf("Got %s\nExpecting %s", result, expected) } } diff --git a/virtcontainers/utils/utils.go b/virtcontainers/utils/utils.go index 06b392931..15eab237d 100644 --- a/virtcontainers/utils/utils.go +++ b/virtcontainers/utils/utils.go @@ -7,15 +7,22 @@ package utils import ( "crypto/rand" + "errors" "fmt" "os" "os/exec" + "path/filepath" ) const cpBinaryName = "cp" const fileMode0755 = os.FileMode(0755) +// MaxSocketPathLen is the effective maximum Unix domain socket length. +// +// See unix(7). +const MaxSocketPathLen = 107 + // FileCopy copys files from srcPath to dstPath func FileCopy(srcPath, dstPath string) error { if srcPath == "" { @@ -174,3 +181,22 @@ func MakeNameID(namedType, id string, maxLen int) string { return nameID } + +// BuildSocketPath concatenates the provided elements into a path and returns +// it. If the resulting path is longer than the maximum permitted socket path +// on Linux, it will return an error. +func BuildSocketPath(elements ...string) (string, error) { + result := filepath.Join(elements...) + + if result == "" { + return "", errors.New("empty path") + } + + l := len(result) + + if l > MaxSocketPathLen { + return "", fmt.Errorf("path too long (got %v, max %v): %s", l, MaxSocketPathLen, result) + } + + return result, nil +} diff --git a/virtcontainers/utils/utils_test.go b/virtcontainers/utils/utils_test.go index 5369b70b4..f738c235c 100644 --- a/virtcontainers/utils/utils_test.go +++ b/virtcontainers/utils/utils_test.go @@ -8,7 +8,9 @@ package utils import ( "io/ioutil" "os" + "path/filepath" "reflect" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -247,5 +249,48 @@ func TestGetSCSIAddress(t *testing.T) { scsiAddr, err := GetSCSIAddress(test.index) assert.Nil(t, err) assert.Equal(t, scsiAddr, test.expectedSCSIAddress) + + } +} + +func TestBuildSocketPath(t *testing.T) { + assert := assert.New(t) + + type testData struct { + elems []string + valid bool + expected string + } + + longPath := strings.Repeat("/a", 106/2) + longestPath := longPath + "a" + pathTooLong := filepath.Join(longestPath, "x") + + data := []testData{ + {[]string{""}, false, ""}, + + {[]string{"a"}, true, "a"}, + {[]string{"/a"}, true, "/a"}, + {[]string{"a", "b", "c"}, true, "a/b/c"}, + {[]string{"a", "/b", "c"}, true, "a/b/c"}, + {[]string{"/a", "b", "c"}, true, "/a/b/c"}, + {[]string{"/a", "/b", "/c"}, true, "/a/b/c"}, + + {[]string{longPath}, true, longPath}, + {[]string{longestPath}, true, longestPath}, + {[]string{pathTooLong}, false, ""}, + } + + for i, d := range data { + result, err := BuildSocketPath(d.elems...) + + if d.valid { + assert.NoErrorf(err, "test %d, data %+v", i, d) + } else { + assert.Errorf(err, "test %d, data %+v", i, d) + } + + assert.NotNil(result) + assert.Equal(d.expected, result) } } diff --git a/virtcontainers/virtcontainers_test.go b/virtcontainers/virtcontainers_test.go index 838abc613..86432b6da 100644 --- a/virtcontainers/virtcontainers_test.go +++ b/virtcontainers/virtcontainers_test.go @@ -75,7 +75,7 @@ func TestMain(m *testing.M) { } SetLogger(logger) - testDir, err = ioutil.TempDir("", "virtcontainers-tmp-") + testDir, err = ioutil.TempDir("", "vc-tmp-") if err != nil { panic(err) }