From d19b12d3fe5ce4a2abb1bdc7df39527f8c2a80c1 Mon Sep 17 00:00:00 2001
From: Jakob Borg <jakob@kastelo.net>
Date: Mon, 11 Nov 2019 09:36:31 +0100
Subject: [PATCH] lib/protocol: Buffer allocation when compressing (fixes
 #6146) (#6147)

We incorrectly gave a too small buffer to lz4.Compress, causing it to
allocate in some cases (when the data actually becomes larger when
compressed). This then panicked when passed to the buffer pool.

This ensures a buffer that is large enough, and adds tripwires closer to
the source in case this ever pops up again. There is a test that
exercises the issue.
---
 lib/protocol/protocol.go      | 18 ++++++++++++------
 lib/protocol/protocol_test.go | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+), 6 deletions(-)

diff --git a/lib/protocol/protocol.go b/lib/protocol/protocol.go
index 59991732..7ffae79a 100644
--- a/lib/protocol/protocol.go
+++ b/lib/protocol/protocol.go
@@ -959,14 +959,17 @@ func (c *rawConnection) Statistics() Statistics {
 
 func (c *rawConnection) lz4Compress(src []byte) ([]byte, error) {
 	var err error
-	buf := BufferPool.Get(len(src))
-	buf, err = lz4.Encode(buf, src)
+	buf := BufferPool.Get(lz4.CompressBound(len(src)))
+	compressed, err := lz4.Encode(buf, src)
 	if err != nil {
 		return nil, err
 	}
+	if &compressed[0] != &buf[0] {
+		panic("bug: lz4.Compress allocated, which it must not (should use buffer pool)")
+	}
 
-	binary.BigEndian.PutUint32(buf, binary.LittleEndian.Uint32(buf))
-	return buf, nil
+	binary.BigEndian.PutUint32(compressed, binary.LittleEndian.Uint32(compressed))
+	return compressed, nil
 }
 
 func (c *rawConnection) lz4Decompress(src []byte) ([]byte, error) {
@@ -974,9 +977,12 @@ func (c *rawConnection) lz4Decompress(src []byte) ([]byte, error) {
 	binary.LittleEndian.PutUint32(src, size)
 	var err error
 	buf := BufferPool.Get(int(size))
-	buf, err = lz4.Decode(buf, src)
+	decoded, err := lz4.Decode(buf, src)
 	if err != nil {
 		return nil, err
 	}
-	return buf, nil
+	if &decoded[0] != &buf[0] {
+		panic("bug: lz4.Decode allocated, which it must not (should use buffer pool)")
+	}
+	return decoded, nil
 }
diff --git a/lib/protocol/protocol_test.go b/lib/protocol/protocol_test.go
index 6075d719..4f973c61 100644
--- a/lib/protocol/protocol_test.go
+++ b/lib/protocol/protocol_test.go
@@ -434,6 +434,38 @@ func TestLZ4Compression(t *testing.T) {
 	}
 }
 
+func TestStressLZ4CompressGrows(t *testing.T) {
+	c := new(rawConnection)
+	success := 0
+	for i := 0; i < 100; i++ {
+		// Create a slize that is precisely one min block size, fill it with
+		// random data. This shouldn't compress at all, so will in fact
+		// become larger when LZ4 does its thing.
+		data := make([]byte, MinBlockSize)
+		if _, err := rand.Reader.Read(data); err != nil {
+			t.Fatal("randomness failure")
+		}
+
+		comp, err := c.lz4Compress(data)
+		if err != nil {
+			t.Fatal("unexpected compression error: ", err)
+		}
+		if len(comp) < len(data) {
+			// data size should grow. We must have been really unlucky in
+			// the random generation, try again.
+			continue
+		}
+
+		// Putting it into the buffer pool shouldn't panic because the block
+		// should come from there to begin with.
+		BufferPool.Put(comp)
+		success++
+	}
+	if success == 0 {
+		t.Fatal("unable to find data that grows when compressed")
+	}
+}
+
 func TestCheckFilename(t *testing.T) {
 	cases := []struct {
 		name string