Ver código fonte

ssh: fix locking in channel.Write

Since a lock is retaken before sync.Cond.Wait returns, this could
deadlock when the for loop attempts to take the lock again. (Reported
by sanjay.m.)

theirWindow was used outside of the lock, therefore concurrent writers
could overrun the window.

theirWindow was never updated to reflect the data written.

R=dave, balasanjay
CC=golang-dev
https://golang.org/cl/5671084
Adam Langley 13 anos atrás
pai
commit
7f524f2468
1 arquivos alterados com 30 adições e 9 exclusões
  1. 30 9
      ssh/channel.go

+ 30 - 9
ssh/channel.go

@@ -224,22 +224,43 @@ func (c *channel) Read(data []byte) (n int, err error) {
 	panic("unreachable")
 	panic("unreachable")
 }
 }
 
 
-func (c *channel) Write(data []byte) (n int, err error) {
-	for len(data) > 0 {
-		c.lock.Lock()
+// getWindowSpace takes, at most, max bytes of space from the peer's window. It
+// returns the number of bytes actually reserved.
+func (c *channel) getWindowSpace(max uint32) (uint32, error) {
+	c.lock.Lock()
+	defer c.lock.Unlock()
+
+	for {
 		if c.dead || c.weClosed {
 		if c.dead || c.weClosed {
 			return 0, io.EOF
 			return 0, io.EOF
 		}
 		}
 
 
-		if c.theirWindow == 0 {
-			c.cond.Wait()
-			continue
+		if c.theirWindow > 0 {
+			break
+		}
+
+		c.cond.Wait()
+	}
+
+	taken := c.theirWindow
+	if taken > max {
+		taken = max
+	}
+
+	c.theirWindow -= taken
+	return taken, nil
+}
+
+func (c *channel) Write(data []byte) (n int, err error) {
+	for len(data) > 0 {
+		var space uint32
+		if space, err = c.getWindowSpace(uint32(len(data))); err != nil {
+			return 0, err
 		}
 		}
-		c.lock.Unlock()
 
 
 		todo := data
 		todo := data
-		if uint32(len(todo)) > c.theirWindow {
-			todo = todo[:c.theirWindow]
+		if uint32(len(todo)) > space {
+			todo = todo[:space]
 		}
 		}
 
 
 		packet := make([]byte, 1+4+4+len(todo))
 		packet := make([]byte, 1+4+4+len(todo))