Concurrency fixes for *cap.Set atomicity.

Previously, the atomicity was not uniformly enforced.

Signed-off-by: Andrew G. Morgan <morgan@kernel.org>

4 files changed, 58 insertions, 36 deletions

diff --git a/cap/cap.go b/cap/cap.go
index 5bbfea3..10c24a2 100644
--- a/cap/cap.go
+++ b/cap/cap.go
@@ -352,6 +352,8 @@ func GetProc() *Set {
 	return c
 }
 
+// setProc uses syscaller to set process capabilities.  Note, c is
+// either private to or (read) locked by the caller.
 func (sc *syscaller) setProc(c *Set) error {
 	if c == nil || len(c.flat) == 0 {
 		return ErrBadSet
@@ -371,6 +373,8 @@ func (sc *syscaller) setProc(c *Set) error {
 func (c *Set) SetProc() error {
 	state, sc := scwStateSC()
 	defer scwSetState(launchBlocked, state, -1)
+	c.mu.RLock()
+	defer c.mu.RUnlock()
 	return sc.setProc(c)
 }
 
diff --git a/cap/file.go b/cap/file.go
index cfc171d..b433c9b 100644
--- a/cap/file.go
+++ b/cap/file.go
@@ -164,6 +164,8 @@ func (c *Set) GetNSOwner() (int, error) {
 	if magic < kv3 {
 		return 0, ErrBadMagic
 	}
+	c.mu.RLock()
+	defer c.mu.RUnlock()
 	return c.nsRoot, nil
 }
 
@@ -187,6 +189,9 @@ func (c *Set) SetNSOwner(uid int) {
 // attributes, the process is a little lossy with respect to effective
 // bits.
 func (c *Set) packFileCap() ([]byte, error) {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+
 	var magic uint32
 	switch words {
 	case 1:
@@ -255,8 +260,8 @@ func (c *Set) SetFd(file *os.File) error {
 		}
 		return nil
 	}
-	c.mu.Lock()
-	defer c.mu.Unlock()
+	c.mu.RLock()
+	defer c.mu.RUnlock()
 	d, err := c.packFileCap()
 	if err != nil {
 		return err
@@ -297,8 +302,8 @@ func (c *Set) SetFile(path string) error {
 		}
 		return nil
 	}
-	c.mu.Lock()
-	defer c.mu.Unlock()
+	c.mu.RLock()
+	defer c.mu.RUnlock()
 	d, err := c.packFileCap()
 	if err != nil {
 		return err
@@ -372,8 +377,8 @@ func (c *Set) Export() ([]byte, error) {
 	}
 	b := new(bytes.Buffer)
 	binary.Write(b, binary.LittleEndian, ExtMagic)
-	c.mu.Lock()
-	defer c.mu.Unlock()
+	c.mu.RLock()
+	defer c.mu.RUnlock()
 	var n = uint(0)
 	for i, f := range c.flat {
 		if nn := 4 * uint(i); nn+4 > n {
diff --git a/cap/flags.go b/cap/flags.go
index 57c0443..0463484 100644
--- a/cap/flags.go
+++ b/cap/flags.go
@@ -87,18 +87,19 @@ func (c *Set) FillFlag(to Flag, ref *Set, from Flag) error {
 		return ErrBadValue
 	}
 
-	// Avoid deadlock by copying to intermediate memory.
-	a := make([]uint32, len(ref.flat))
-	ref.mu.Lock()
-	for i := range ref.flat {
-		a[i] = ref.flat[i][from]
+	// Avoid deadlock by using a copy.
+	if c != ref {
+		var err error
+		ref, err = ref.Dup()
+		if err != nil {
+			return err
+		}
 	}
-	ref.mu.Unlock()
 
 	c.mu.Lock()
 	defer c.mu.Unlock()
 	for i := range c.flat {
-		c.flat[i][to] = a[i]
+		c.flat[i][to] = ref.flat[i][from]
 	}
 	return nil
 }
@@ -165,6 +166,40 @@ func (c *Set) ClearFlag(vec Flag) error {
 	return c.forceFlag(vec, false)
 }
 
+// Cf returns 0 if c and d are identical. A non-zero Diff value
+// captures a simple macroscopic summary of how they differ. The
+// (Diff).Has() function can be used to determine how the two
+// capability sets differ.
+func (c *Set) Cf(d *Set) (Diff, error) {
+	if c == nil || len(c.flat) == 0 || d == nil || len(d.flat) == 0 {
+		return 0, ErrBadSet
+	}
+	if c == d {
+		return 0, nil
+	}
+	d, err := d.Dup()
+	if err != nil {
+		return 0, err
+	}
+
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+
+	var cf Diff
+	for i := 0; i < words; i++ {
+		if c.flat[i][Effective]^d.flat[i][Effective] != 0 {
+			cf |= effectiveDiff
+		}
+		if c.flat[i][Permitted]^d.flat[i][Permitted] != 0 {
+			cf |= permittedDiff
+		}
+		if c.flat[i][Inheritable]^d.flat[i][Inheritable] != 0 {
+			cf |= inheritableDiff
+		}
+	}
+	return cf, nil
+}
+
 // Compare returns 0 if c and d are identical in content.
 //
 // Deprecated: Replace with (*Set).Cf().
@@ -199,29 +234,6 @@ func (c *Set) Compare(d *Set) (uint, error) {
 	return uint(u), err
 }
 
-// Cf returns 0 if c and d are identical. A non-zero Diff value
-// captures a simple macroscopic summary of how they differ. The
-// (Diff).Has() function can be used to determine how the two
-// capability sets differ.
-func (c *Set) Cf(d *Set) (Diff, error) {
-	if c == nil || len(c.flat) == 0 || d == nil || len(d.flat) == 0 {
-		return 0, ErrBadSet
-	}
-	var cf Diff
-	for i := 0; i < words; i++ {
-		if c.flat[i][Effective]^d.flat[i][Effective] != 0 {
-			cf |= effectiveDiff
-		}
-		if c.flat[i][Permitted]^d.flat[i][Permitted] != 0 {
-			cf |= permittedDiff
-		}
-		if c.flat[i][Inheritable]^d.flat[i][Inheritable] != 0 {
-			cf |= inheritableDiff
-		}
-	}
-	return cf, nil
-}
-
 // Differs processes the result of Compare and determines if the
 // Flag's components were different.
 //
diff --git a/cap/text.go b/cap/text.go
index cf11a2d..7c3f540 100644
--- a/cap/text.go
+++ b/cap/text.go
@@ -48,6 +48,7 @@ const (
 var combos = []string{"", "e", "p", "ep", "i", "ei", "ip", "eip"}
 
 // histo generates a histogram of flag state combinations.
+// Note: c is locked by or private to the caller.
 func (c *Set) histo(bins []int, patterns []uint, from, limit Value) uint {
 	for v := from; v < limit; v++ {
 		b := uint(v & 31)