[lxc-devel] [lxd/master] Avoid race condition in network fill function

[stgraber on Github]

A non-text attachment was scrubbed...
Name: not available
Type: text/x-mailbox
Size: 354 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20161127/74a98fd0/attachment.bin>
-------------- next part --------------
From 345c9909a2a0c6535b6a477a5a78a9b73fb6b2ca Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?St=C3=A9phane=20Graber?= <stgraber at ubuntu.com>
Date: Sat, 26 Nov 2016 21:50:53 -0500
Subject: [PATCH] Avoid race condition in network fill function
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Stéphane Graber <stgraber at ubuntu.com>
---
 lxd/container_lxc.go | 72 ++++++++++++++++++++++++++++++----------------------
 lxd/db_containers.go |  9 +++++++
 2 files changed, 51 insertions(+), 30 deletions(-)

diff --git a/lxd/container_lxc.go b/lxd/container_lxc.go
index fe416be..82a91d3 100644
--- a/lxd/container_lxc.go
+++ b/lxd/container_lxc.go
@@ -5254,6 +5254,26 @@ func (c *containerLXC) fillNetworkDevice(name string, m shared.Device) (shared.D
 		}
 	}
 
+	updateKey := func(key string, value string) error {
+		tx, err := dbBegin(c.daemon.db)
+		if err != nil {
+			return err
+		}
+
+		err = dbContainerConfigInsert(tx, c.id, map[string]string{key: value})
+		if err != nil {
+			tx.Rollback()
+			return err
+		}
+
+		err = txCommit(tx)
+		if err != nil {
+			return err
+		}
+
+		return nil
+	}
+
 	// Fill in the MAC address
 	if m["nictype"] != "physical" && m["hwaddr"] == "" {
 		configKey := fmt.Sprintf("volatile.%s.hwaddr", name)
@@ -5265,24 +5285,20 @@ func (c *containerLXC) fillNetworkDevice(name string, m shared.Device) (shared.D
 				return nil, err
 			}
 
-			c.localConfig[configKey] = volatileHwaddr
-			c.expandedConfig[configKey] = volatileHwaddr
-
 			// Update the database
-			tx, err := dbBegin(c.daemon.db)
+			err = updateKey(configKey, volatileHwaddr)
 			if err != nil {
-				return nil, err
-			}
-
-			err = dbContainerConfigInsert(tx, c.id, map[string]string{configKey: volatileHwaddr})
-			if err != nil {
-				tx.Rollback()
-				return nil, err
-			}
+				// Check if something else filled it in behind our back
+				value, err1 := dbContainerConfigGet(c.daemon.db, c.id, configKey)
+				if err1 != nil || value == "" {
+					return nil, err
+				}
 
-			err = txCommit(tx)
-			if err != nil {
-				return nil, err
+				c.localConfig[configKey] = value
+				c.expandedConfig[configKey] = value
+			} else {
+				c.localConfig[configKey] = volatileHwaddr
+				c.expandedConfig[configKey] = volatileHwaddr
 			}
 		}
 		newDevice["hwaddr"] = volatileHwaddr
@@ -5299,24 +5315,20 @@ func (c *containerLXC) fillNetworkDevice(name string, m shared.Device) (shared.D
 				return nil, err
 			}
 
-			c.localConfig[configKey] = volatileName
-			c.expandedConfig[configKey] = volatileName
-
 			// Update the database
-			tx, err := dbBegin(c.daemon.db)
-			if err != nil {
-				return nil, err
-			}
-
-			err = dbContainerConfigInsert(tx, c.id, map[string]string{configKey: volatileName})
+			err = updateKey(configKey, volatileName)
 			if err != nil {
-				tx.Rollback()
-				return nil, err
-			}
+				// Check if something else filled it in behind our back
+				value, err1 := dbContainerConfigGet(c.daemon.db, c.id, configKey)
+				if err1 != nil || value == "" {
+					return nil, err
+				}
 
-			err = txCommit(tx)
-			if err != nil {
-				return nil, err
+				c.localConfig[configKey] = value
+				c.expandedConfig[configKey] = value
+			} else {
+				c.localConfig[configKey] = volatileName
+				c.expandedConfig[configKey] = volatileName
 			}
 		}
 		newDevice["name"] = volatileName
diff --git a/lxd/db_containers.go b/lxd/db_containers.go
index 7cc9a3e..eec6e9c 100644
--- a/lxd/db_containers.go
+++ b/lxd/db_containers.go
@@ -224,6 +224,15 @@ func dbContainerConfigInsert(tx *sql.Tx, id int, config map[string]string) error
 	return nil
 }
 
+func dbContainerConfigGet(db *sql.DB, id int, key string) (string, error) {
+	q := "SELECT value FROM containers_config WHERE container_id=? AND key=?"
+	value := ""
+	arg1 := []interface{}{id, key}
+	arg2 := []interface{}{&value}
+	err := dbQueryRowScan(db, q, arg1, arg2)
+	return value, err
+}
+
 func dbContainerConfigRemove(db *sql.DB, id int, name string) error {
 	_, err := dbExec(db, "DELETE FROM containers_config WHERE key=? AND container_id=?", name, id)
 	return err