smb: client: compress: fix buffer overrun in lz77_compress()

@dst buffer is allocated with same size as @src, which, for good
compression cases, works fine.

However, when compression goes bad (e.g. random bytes payloads), the
compressed size can increase significantly, and even by stopping the
main loop at 7/8 of @slen, writing leftover literals could write past
the end of @dst because of LZ77 metadata.

To fix this, add lz77_compressed_alloc_size() helper to compute the
correct allocation size for @dst, accounting for metadata and worst
cast scenario (all literals).

While this is overprovisioning memory, it's not only correct, but also
allows lz77_compress() main loop to run without ever checking @dst
limits (i.e. a perf improvement).

Signed-off-by: Enzo Matsumiya <[email protected]>
Signed-off-by: Steve French <[email protected]>

Author: ematsumiya

fs/smb/client/compress.c

@@ -329,11 +329,7 @@ int smb_compress(struct TCP_Server_Info *server, struct smb_rqst *rq, compress_s
 		goto err_free;
 	}
 
-	/*
-	 * This is just overprovisioning, as the algorithm will error out if @dst reaches 7/8
-	 * of @slen.
-	 */
-	dlen = slen;
+	dlen = lz77_compressed_alloc_size(slen);
 	dst = kvzalloc(dlen, GFP_KERNEL);
 	if (!dst) {
 		ret = -ENOMEM;

fs/smb/client/compress/lz77.c

@@ -137,6 +137,10 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
 	long flag = 0;
 	u64 *htable;
 
+	/* This is probably a bug, so throw a warning. */
+	if (WARN_ON_ONCE(*dlen < lz77_compressed_alloc_size(slen)))
+		return -EINVAL;
+
 	srcp = src;
 	end = src + slen;
 	dstp = dst;
@@ -180,15 +184,6 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
 			continue;
 		}
 
-		/*
-		 * Bail out if @dstp reached >= 7/8 of @slen -- already compressed badly, not worth
-		 * going further.
-		 */
-		if (unlikely(dstp - dst >= slen - (slen >> 3))) {
-			*dlen = slen;
-			goto out;
-		}
-
 		dstp = lz77_write_match(dstp, &nib, dist, len);
 		srcp += len;
 
@@ -225,7 +220,6 @@ noinline int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen)
 	lz77_write32(flag_pos, flag);
 
 	*dlen = dstp - dst;
-out:
 	kvfree(htable);
 
 	if (*dlen < slen)

fs/smb/client/compress/lz77.h

@@ -11,5 +11,33 @@
 
 #include <linux/kernel.h>
 
+/**
+ * lz77_compressed_alloc_size() - Compute compressed buffer size.
+ * @size:	uncompressed (src) size
+ *
+ * Compute allocation size for the compressed buffer based on uncompressed size.
+ * Accounts for metadata and overprovision for the worst case scenario.
+ *
+ * LZ77 metadata is a 4-byte flag that is written:
+ * - on dst begin (pos 0)
+ * - every 32 literals or matches
+ * - on end-of-stream (possibly, if last write was another flag)
+ *
+ * Worst case scenario is an all-literal compression, which means:
+ * metadata bytes = 4 + ((@size / 32) * 4) + 4, or, simplified, (@size >> 3) + 8
+ *
+ * The worst case scenario rarely happens, but such overprovisioning also allows lz77_compress()
+ * main loop to run without ever bound checking dst, which is a huge perf improvement, while also
+ * being safe when compression goes bad.
+ *
+ * Return: required (*) allocation size for compressed buffer.
+ *
+ * (*) checked once in the beginning of lz77_compress()
+ */
+static __always_inline u32 lz77_compressed_alloc_size(const u32 size)
+{
+	return size + (size >> 3) + 8;
+}
+
 int lz77_compress(const void *src, u32 slen, void *dst, u32 *dlen);
 #endif /* _SMB_COMPRESS_LZ77_H */