PATCH: ctdb: buffer write beyond limits

Christof Schmitt cs at samba.org
Tue Feb 19 23:17:30 UTC 2019


On Tue, Feb 19, 2019 at 02:02:31PM -0700, Christof Schmitt via samba-technical wrote:
> Maybe we can get this in another day? See the attached patch for the
> start of a test of the queue code. Could we extend that to trigger the
> problem and then verify that it is fixed with your patch?

The initial patch was missing the wscript update. Here is an extended
version that shows the memory corruption without your fix. I would
suggest to add the fix with a test now to fix the 4.10 release and then
we can work towards adding more tests for the queueing code.

Christof
-------------- next part --------------
From 2641f1189af23cc17162f41e154f42d267b6e008 Mon Sep 17 00:00:00 2001
From: Christof Schmitt <cs at samba.org>
Date: Tue, 19 Feb 2019 13:59:05 -0700
Subject: [PATCH] ctdb-tests: Add test for ctdb_io.c

Signed-off-by: Christof Schmitt <cs at samba.org>
---
 ctdb/tests/cunit/ctdb_io_test_001.sh |   8 ++
 ctdb/tests/src/ctdb_io_test.c        | 196 +++++++++++++++++++++++++++
 ctdb/wscript                         |   6 +
 3 files changed, 210 insertions(+)
 create mode 100755 ctdb/tests/cunit/ctdb_io_test_001.sh
 create mode 100644 ctdb/tests/src/ctdb_io_test.c

diff --git a/ctdb/tests/cunit/ctdb_io_test_001.sh b/ctdb/tests/cunit/ctdb_io_test_001.sh
new file mode 100755
index 00000000000..15842ea8e17
--- /dev/null
+++ b/ctdb/tests/cunit/ctdb_io_test_001.sh
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+. "${TEST_SCRIPTS_DIR}/unit.sh"
+
+ok_null
+
+unit_test ctdb_io_test 1
+unit_test ctdb_io_test 2
diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
new file mode 100644
index 00000000000..22bd2f4930f
--- /dev/null
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -0,0 +1,196 @@
+/*
+   ctdb_io tests
+
+   Copyright (C) Christof Schmitt 2019
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program; if not, see <http://www.gnu.org/licenses/>.
+*/
+
+#include "replace.h"
+#include "system/filesys.h"
+
+#include <assert.h>
+
+#include "common/ctdb_io.c"
+
+void ctdb_set_error(struct ctdb_context *ctdb, const char *fmt, ...)
+{
+	va_list ap;
+	va_start(ap, fmt);
+	vprintf(fmt, ap);
+	assert(false);
+}
+
+static void test_setup(ctdb_queue_cb_fn_t cb,
+		       int *pfd,
+		       struct ctdb_context **pctdb)
+{
+	int pipefd[2], ret;
+	struct ctdb_context *ctdb;
+	struct ctdb_queue *queue;
+
+	ret = pipe(pipefd);
+	assert(ret == 0);
+
+	ctdb = talloc_zero(NULL, struct ctdb_context);
+	assert(ctdb != NULL);
+
+	ctdb->ev = tevent_context_init(NULL);
+
+	queue = ctdb_queue_setup(ctdb, ctdb, pipefd[0], 0, cb,
+				 NULL, "test queue");
+	assert(queue != NULL);
+
+	*pctdb = ctdb;
+	*pfd = pipefd[1];
+}
+
+static const size_t test1_req_len = 8;
+static const char *test1_req = "abcdefgh";
+
+static void test1_callback(uint8_t *data, size_t length, void *private_data)
+{
+	uint32_t len;
+
+	len = *(uint32_t *)data;
+	assert(len == sizeof(uint32_t) + test1_req_len);
+
+	assert(length == sizeof(uint32_t) + test1_req_len);
+	assert(memcmp(data  + sizeof(len), test1_req, length) == 0);
+}
+
+static void test1(void)
+{
+	struct ctdb_context *ctdb;
+	int fd, ret;
+	uint32_t pkt_size;
+
+	test_setup(test1_callback, &fd, &ctdb);
+
+	pkt_size = sizeof(uint32_t) + test1_req_len;
+	ret = write(fd, &pkt_size, sizeof(pkt_size));
+	assert(ret == sizeof(pkt_size));
+
+	ret = write(fd, test1_req, test1_req_len);
+	assert(ret == test1_req_len);
+
+	tevent_loop_once(ctdb->ev);
+
+	TALLOC_FREE(ctdb);
+}
+
+static const size_t test2_req_len[] = { 900, 24, 600 };
+
+static int test2_cb_num = 0;
+
+static void test2_callback(uint8_t *data, size_t length, void *private_data)
+{
+	uint32_t len;
+
+	len = *(uint32_t *)data;
+	assert(len == sizeof(uint32_t) + test2_req_len[test2_cb_num]);
+	assert(length == sizeof(uint32_t) + test2_req_len[test2_cb_num]);
+
+	test2_cb_num++;
+}
+
+static void test2(void)
+{
+	struct ctdb_context *ctdb;
+	int fd, ret, i;
+	uint32_t pkt_size;
+	char req[1024] = { 0 };
+
+	for (i = 0; i < sizeof(req); i++) {
+		req[i] = i % CHAR_MAX;
+	}
+
+	test_setup(test2_callback, &fd, &ctdb);
+
+	/*
+	 * request 0
+	 */
+
+	pkt_size = sizeof(uint32_t) + test2_req_len[0];
+	ret = write(fd, &pkt_size, sizeof(pkt_size));
+	assert(ret == sizeof(pkt_size));
+
+	ret = write(fd, req, test2_req_len[0]);
+	assert(ret == test2_req_len[0]);
+
+	/*
+	 * request 1
+	 */
+	pkt_size = sizeof(uint32_t) + test2_req_len[1];
+	ret = write(fd, &pkt_size, sizeof(pkt_size));
+	assert(ret == sizeof(pkt_size));
+
+	/*
+	 * Omit the last byte to avoid buffer processing.
+	 */
+	ret = write(fd, req, test2_req_len[1] - 1);
+	assert(ret == test2_req_len[1] - 1);
+
+	tevent_loop_once(ctdb->ev);
+
+	/*
+	 * Write the missing byte now.
+	 */
+	ret = write(fd, &req[test2_req_len[1] - 1], 1);
+	assert(ret == 1);
+
+	/*
+	 * request 2
+	 */
+	pkt_size = sizeof(uint32_t) + test2_req_len[2];
+	ret = write(fd, &pkt_size, sizeof(pkt_size));
+	assert(ret == sizeof(pkt_size));
+
+	ret = write(fd, req, test2_req_len[2]);
+	assert(ret == test2_req_len[2]);
+
+	tevent_loop_once(ctdb->ev);
+	tevent_loop_once(ctdb->ev);
+
+	assert(test2_cb_num == 2);
+
+	TALLOC_FREE(ctdb);
+}
+
+int main(int argc, const char **argv)
+{
+	int num;
+
+	if (argc != 2) {
+		fprintf(stderr, "%s <testnum>\n", argv[0]);
+		exit(1);
+	}
+
+
+	num = atoi(argv[1]);
+	switch (num) {
+	case 1:
+		test1();
+		break;
+
+	case 2:
+		test2();
+		break;
+
+	default:
+		fprintf(stderr, "Unknown test number %s\n", argv[1]);
+	}
+
+	return 0;
+}
diff --git a/ctdb/wscript b/ctdb/wscript
index 30b09d6dc16..96fad7a07c0 100644
--- a/ctdb/wscript
+++ b/ctdb/wscript
@@ -943,6 +943,12 @@ def build(bld):
                              LIBASYNC_REQ samba-util sys_rw''',
                      install_path='${CTDB_TEST_LIBEXECDIR}')
 
+    bld.SAMBA_BINARY('ctdb_io_test',
+                     source='tests/src/ctdb_io_test.c',
+                     deps='''talloc tevent samba-util sys_rw''',
+                     install_path='${CTDB_TEST_LIBEXECDIR}')
+
+
     bld.SAMBA_SUBSYSTEM('ctdb-protocol-tests-basic',
                         source=bld.SUBDIR('tests/src',
                                           'protocol_common_basic.c'),
-- 
2.17.0



More information about the samba-technical mailing list