bonding: fix race condition in bonding_store_slaves_active

nikolay@redhat.com · davem330 · commit e196c0e57990 · 2012-11-29T13:13:15.000-05:00
Race between bonding_store_slaves_active() and slave manipulation
 functions. The bond_for_each_slave use in bonding_store_slaves_active()
 is not protected by any synchronization mechanism.
 NULL pointer dereference is easy to reach.
 Fixed by acquiring the bond-&gt;lock for the slave walk.

 v2: Make description text &lt; 75 columns

Signed-off-by: Nikolay Aleksandrov &lt;nikolay@redhat.com&gt;
Signed-off-by: Jay Vosburgh &lt;fubar@us.ibm.com&gt;
Signed-off-by: David S. Miller &lt;davem@davemloft.net&gt;
diff --git a/drivers/net/bonding/bond_sysfs.c b/drivers/net/bonding/bond_sysfs.c
@@ -1568,6 +1568,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
 		goto out;
 	}
 
+	read_lock(&bond->lock);
 	bond_for_each_slave(bond, slave, i) {
 		if (!bond_is_active_slave(slave)) {
 			if (new_value)
@@ -1576,6 +1577,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,
 				slave->inactive = 1;
 		}
 	}
+	read_unlock(&bond->lock);
 out:
 	return ret;
 }

Original file line number	Diff line number	Diff line change
`@@ -1568,6 +1568,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,`
`1568`	`1568`	`goto out;`
`1569`	`1569`	`}`
`1570`	`1570`
	`1571`	`+ read_lock(&bond->lock);`
`1571`	`1572`	`bond_for_each_slave(bond, slave, i) {`
`1572`	`1573`	`if (!bond_is_active_slave(slave)) {`
`1573`	`1574`	`if (new_value)`
`@@ -1576,6 +1577,7 @@ static ssize_t bonding_store_slaves_active(struct device *d,`
`1576`	`1577`	`slave->inactive = 1;`
`1577`	`1578`	`}`
`1578`	`1579`	`}`
	`1580`	`+ read_unlock(&bond->lock);`
`1579`	`1581`	`out:`
`1580`	`1582`	`return ret;`
`1581`	`1583`	`}`