libct/cg/sd: fix dbus connection leak (alternative)

libcontainer/cgroups/systemd/common.go

@@ -384,15 +384,23 @@ func setUnitProperties(cm *dbusConnManager, name string, properties ...systemdDb
 	})
 }
 
+func getManagerProperty(cm *dbusConnManager, name string) (string, error) {
+	str := ""
+	err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) error {
+		var err error
+		str, err = c.GetManagerProperty(name)
+		return err
+	})
+	if err != nil {
+		return "", err
+	}
+	return strconv.Unquote(str)
+}
+
 func systemdVersion(cm *dbusConnManager) int {
 	versionOnce.Do(func() {
 		version = -1
-		var verStr string
-		err := cm.retryOnDisconnect(func(c *systemdDbus.Conn) error {
-			var err error
-			verStr, err = c.GetManagerProperty("Version")
-			return err
-		})
+		verStr, err := getManagerProperty(cm, "Version")
 		if err == nil {
 			version, err = systemdVersionAtoi(verStr)
 		}
@@ -407,11 +415,11 @@ func systemdVersion(cm *dbusConnManager) int {
 
 func systemdVersionAtoi(verStr string) (int, error) {
 	// verStr should be of the form:
-	// "v245.4-1.fc32", "245", "v245-1.fc32", "245-1.fc32"
-	// all the input strings include quotes, and the output int should be 245
-	// thus, we unconditionally remove the `"v`
-	// and then match on the first integer we can grab
-	re := regexp.MustCompile(`"?v?([0-9]+)`)
+	// "v245.4-1.fc32", "245", "v245-1.fc32", "245-1.fc32" (without quotes).
+	// The result for all of the above should be 245.
+	// Thus, we unconditionally remove the "v" prefix
+	// and then match on the first integer we can grab.
+	re := regexp.MustCompile(`v?([0-9]+)`)
 	matches := re.FindStringSubmatch(verStr)
 	if len(matches) < 2 {
 		return 0, errors.Errorf("can't parse version %s: incorrect number of matches %v", verStr, matches)

libcontainer/cgroups/systemd/dbus.go

@@ -10,50 +10,56 @@ import (
 	dbus "github.com/godbus/dbus/v5"
 )
 
+var (
+	dbusC        *systemdDbus.Conn
+	dbusMu       sync.RWMutex
+	dbusInited   bool
+	dbusRootless bool
+)
+
 type dbusConnManager struct {
-	conn     *systemdDbus.Conn
-	rootless bool
-	sync.RWMutex
 }
 
 // newDbusConnManager initializes systemd dbus connection manager.
 func newDbusConnManager(rootless bool) *dbusConnManager {
-	return &dbusConnManager{
-		rootless: rootless,
+	if dbusInited && rootless != dbusRootless {
+		panic("can't have both root and rootless dbus")
 	}
+	dbusRootless = rootless
+	return &dbusConnManager{}
 }
 
 // getConnection lazily initializes and returns systemd dbus connection.
 func (d *dbusConnManager) getConnection() (*systemdDbus.Conn, error) {
-	// In the case where d.conn != nil
+	// In the case where dbusC != nil
 	// Use the read lock the first time to ensure
 	// that Conn can be acquired at the same time.
-	d.RLock()
-	if conn := d.conn; conn != nil {
-		d.RUnlock()
+	dbusMu.RLock()
+	if conn := dbusC; conn != nil {
+		dbusMu.RUnlock()
 		return conn, nil
 	}
-	d.RUnlock()
+	dbusMu.RUnlock()
 
-	// In the case where d.conn == nil
+	// In the case where dbusC == nil
 	// Use write lock to ensure that only one
 	// will be created
-	d.Lock()
-	defer d.Unlock()
-	if conn := d.conn; conn != nil {
+	dbusMu.Lock()
+	defer dbusMu.Unlock()
+	if conn := dbusC; conn != nil {
 		return conn, nil
 	}
 
 	conn, err := d.newConnection()
 	if err != nil {
 		return nil, err
 	}
-	d.conn = conn
+	dbusC = conn
 	return conn, nil
 }
 
 func (d *dbusConnManager) newConnection() (*systemdDbus.Conn, error) {
-	if d.rootless {
+	if dbusRootless {
 		return newUserSystemdDbus()
 	}
 	return systemdDbus.NewWithContext(context.TODO())
@@ -62,11 +68,11 @@ func (d *dbusConnManager) newConnection() (*systemdDbus.Conn, error) {
 // resetConnection resets the connection to its initial state
 // (so it can be reconnected if necessary).
 func (d *dbusConnManager) resetConnection(conn *systemdDbus.Conn) {
-	d.Lock()
-	defer d.Unlock()
-	if d.conn != nil && d.conn == conn {
-		d.conn.Close()
-		d.conn = nil
+	dbusMu.Lock()
+	defer dbusMu.Unlock()
+	if dbusC != nil && dbusC == conn {
+		dbusC.Close()
+		dbusC = nil
 	}
 }
 

libcontainer/cgroups/systemd/v2.go

@@ -337,16 +337,8 @@ func (m *unifiedManager) getSliceFull() (string, error) {
 	}
 
 	if m.rootless {
-		dbusConnection, err := m.dbus.getConnection()
-		if err != nil {
-			return "", err
-		}
-		// managerCGQuoted is typically "/user.slice/user-${uid}.slice/user@${uid}.service" including the quote symbols
-		managerCGQuoted, err := dbusConnection.GetManagerProperty("ControlGroup")
-		if err != nil {
-			return "", err
-		}
-		managerCG, err := strconv.Unquote(managerCGQuoted)
+		// managerCG is typically "/user.slice/user-${uid}.slice/user@${uid}.service".
+		managerCG, err := getManagerProperty(m.dbus, "ControlGroup")
 		if err != nil {
 			return "", err
 		}

libcontainer/integration/exec_test.go

@@ -1917,6 +1917,14 @@ func TestCGROUPHost(t *testing.T) {
 }
 
 func TestFdLeaks(t *testing.T) {
+	testFdLeaks(t, false)
+}
+
+func TestFdLeaksSystemd(t *testing.T) {
+	testFdLeaks(t, true)
+}
+
+func testFdLeaks(t *testing.T, systemd bool) {
 	if testing.Short() {
 		return
 	}
@@ -1933,7 +1941,10 @@ func TestFdLeaks(t *testing.T) {
 	_, err = pfd.Seek(0, 0)
 	ok(t, err)
 
-	config := newTemplateConfig(t, &tParam{rootfs: rootfs})
+	config := newTemplateConfig(t, &tParam{
+		rootfs:  rootfs,
+		systemd: systemd,
+	})
 	buffers, exitCode, err := runContainer(t, config, "", "true")
 	ok(t, err)