PATCH: ctdb: buffer write beyond limits

[swen]

On Mon, 2019-02-18 at 15:57 -0800, Jeremy Allison wrote:
> On Mon, Feb 18, 2019 at 06:22:49PM +0100, swen via samba-technical
> wrote:
> > 
> This is *way* too complex to even understand as a comment.
> 
> Please just add the buffer overrun/overflow checks.
> 
> That way we *KNOW* it's safe and don't need to read
> "War and Peace" to understand.
> 
> NAK until then !

Next try...this time with explicit check.

Please review and push if happy

Cheers Swen
-------------- next part --------------
From 160f5f112921dfb2e23c18389d45e2e3204c653b Mon Sep 17 00:00:00 2001
From: Swen Schillig <swen at linux.ibm.com>
Date: Fri, 15 Feb 2019 14:34:05 +0100
Subject: [PATCH] ctdb: buffer write beyond limits

In order to calculate the number of bytes correctly which
are to be read into the buffer, the buffer.offset must be taken
into account.

This patch fixes a regression introduced by 382705f495dd.
BUG: https://bugzilla.samba.org/show_bug.cgi?id=13791

Signed-off-by: Swen Schillig <swen at linux.ibm.com>
---
 ctdb/common/ctdb_io.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/ctdb/common/ctdb_io.c b/ctdb/common/ctdb_io.c
index d86540762ea..559742d1dbe 100644
--- a/ctdb/common/ctdb_io.c
+++ b/ctdb/common/ctdb_io.c
@@ -226,19 +226,29 @@ buffer_shift:
 	}
 
 data_read:
-	num_ready = MIN(num_ready, queue->buffer.size - queue->buffer.length);
+	num_ready = MIN(num_ready,
+			queue->buffer.size -
+				(queue->buffer.length + queue->buffer.offset));
 
 	if (num_ready > 0) {
+		struct ctdb_buffer *cb = &queue->buffer;
+
+		/* this cannot happen but was wanted as an assurance */
+		if (unlikely(cb->size <
+			     (num_ready + cb->offset + cb->length))) {
+			DBG_ERR("Possible buffer overflow (%d)\n", num_ready);
+			goto failed;
+		}
+
 		nread = sys_read(queue->fd,
-				 queue->buffer.data +
-					queue->buffer.offset +
-					queue->buffer.length,
+				 cb->data + cb->offset + cb->length,
 				 num_ready);
+
 		if (nread <= 0) {
 			DEBUG(DEBUG_ERR, ("read error nread=%d\n", (int)nread));
 			goto failed;
 		}
-		queue->buffer.length += nread;
+		cb->length += nread;
 	}
 
 	queue_process(queue);
-- 
2.20.1

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190219/e3800a03/signature.sig>