Merge pull request #5156 from fengwang666/uid-reuse-bug

Non-root hypervisor uid reuse bug

src/runtime/pkg/containerd-shim-v2/create.go

@@ -26,6 +26,7 @@ import (
 	"github.com/kata-containers/kata-containers/src/runtime/virtcontainers/pkg/rootless"
 	"github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
 
 	// only register the proto type
 	crioption "github.com/containerd/containerd/pkg/runtimeoptions/v1"
@@ -136,7 +137,7 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con
 		katautils.HandleFactory(ctx, vci, s.config)
 		rootless.SetRootless(s.config.HypervisorConfig.Rootless)
 		if rootless.IsRootless() {
-			if err := configureNonRootHypervisor(s.config); err != nil {
+			if err := configureNonRootHypervisor(s.config, r.ID); err != nil {
 				return nil, err
 			}
 		}
@@ -303,13 +304,17 @@ func doMount(mounts []*containerd_types.Mount, rootfs string) error {
 	return nil
 }
 
-func configureNonRootHypervisor(runtimeConfig *oci.RuntimeConfig) error {
+func configureNonRootHypervisor(runtimeConfig *oci.RuntimeConfig, sandboxId string) error {
 	userName, err := utils.CreateVmmUser()
 	if err != nil {
 		return err
 	}
 	defer func() {
 		if err != nil {
+			shimLog.WithFields(logrus.Fields{
+				"user_name":  userName,
+				"sandbox_id": sandboxId,
+			}).WithError(err).Warn("configure non root hypervisor failed, delete the user")
 			if err2 := utils.RemoveVmmUser(userName); err2 != nil {
 				shimLog.WithField("userName", userName).WithError(err).Warn("failed to remove user")
 			}
@@ -330,7 +335,14 @@ func configureNonRootHypervisor(runtimeConfig *oci.RuntimeConfig) error {
 		return err
 	}
 	runtimeConfig.HypervisorConfig.Uid = uint32(uid)
+	runtimeConfig.HypervisorConfig.User = userName
 	runtimeConfig.HypervisorConfig.Gid = uint32(gid)
+	shimLog.WithFields(logrus.Fields{
+		"user_name":  userName,
+		"uid":        uid,
+		"gid":        gid,
+		"sandbox_id": sandboxId,
+	}).Debug("successfully created a non root user for the hypervisor")
 
 	userTmpDir := path.Join("/run/user/", fmt.Sprint(uid))
 	_, err = os.Stat(userTmpDir)

src/runtime/virtcontainers/hypervisor.go

@@ -380,6 +380,9 @@ type HypervisorConfig struct {
 	// BlockiDeviceAIO specifies the I/O API to be used.
 	BlockDeviceAIO string
 
+	// The user maps to the uid.
+	User string
+
 	// KernelParams are additional guest kernel parameters.
 	KernelParams []Param
 

src/runtime/virtcontainers/qemu.go

@@ -110,6 +110,8 @@ type qemu struct {
 	nvdimmCount int
 
 	stopped bool
+
+	mu sync.Mutex
 }
 
 const (
@@ -678,7 +680,7 @@ func (q *qemu) checkBpfEnabled() {
 			q.Logger().WithError(err).Warningf("failed to get bpf_jit_enable status")
 			return
 		}
-		enabled, err := strconv.Atoi(string(out))
+		enabled, err := strconv.Atoi(strings.TrimSpace(string(out)))
 		if err != nil {
 			q.Logger().WithError(err).Warningf("failed to convert bpf_jit_enable status to integer")
 			return
@@ -968,6 +970,8 @@ func (q *qemu) waitVM(ctx context.Context, timeout int) error {
 
 // StopVM will stop the Sandbox's VM.
 func (q *qemu) StopVM(ctx context.Context, waitOnly bool) error {
+	q.mu.Lock()
+	defer q.mu.Unlock()
 	span, _ := katatrace.Trace(ctx, q.Logger(), "StopVM", qemuTracingTags, map[string]string{"sandbox_id": q.id})
 	defer span.End()
 
@@ -1059,15 +1063,27 @@ func (q *qemu) cleanupVM() error {
 	}
 
 	if rootless.IsRootless() {
-		u, err := user.LookupId(strconv.Itoa(int(q.config.Uid)))
-		if err != nil {
-			q.Logger().WithError(err).WithField("uid", q.config.Uid).Warn("failed to find the user")
+		if _, err := user.Lookup(q.config.User); err != nil {
+			q.Logger().WithError(err).WithFields(
+				logrus.Fields{
+					"user": q.config.User,
+					"uid":  q.config.Uid,
+				}).Warn("failed to find the user, it might have been removed")
 			return nil
 		}
 
-		if err := pkgUtils.RemoveVmmUser(u.Username); err != nil {
-			q.Logger().WithError(err).WithField("user", u.Username).Warn("failed to delete the user")
+		if err := pkgUtils.RemoveVmmUser(q.config.User); err != nil {
+			q.Logger().WithError(err).WithFields(
+				logrus.Fields{
+					"user": q.config.User,
+					"uid":  q.config.Uid,
+				}).Warn("failed to delete the user")
 		}
+		q.Logger().WithFields(
+			logrus.Fields{
+				"user": q.config.User,
+				"uid":  q.config.Uid,
+			}).Debug("successfully removed the non root user")
 	}
 
 	return nil