From 6a5847c61dc3aa77ed2fecf08a7f4bd64244ed9b Mon Sep 17 00:00:00 2001 From: Swen Schillig Date: Fri, 15 Feb 2019 14:34:05 +0100 Subject: [PATCH 1/2] 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 Reviewed-by: Christof Schmitt Reviewed-by: Martin Schwenke (cherry picked from commit fa8e69ac9538980c441b7fbefe0979027ecc8eac) --- ctdb/common/ctdb_io.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/ctdb/common/ctdb_io.c b/ctdb/common/ctdb_io.c index d86540762ea..c16eb7f67b7 100644 --- a/ctdb/common/ctdb_io.c +++ b/ctdb/common/ctdb_io.c @@ -164,6 +164,7 @@ static void queue_io_read(struct ctdb_queue *queue) { int num_ready = 0; uint32_t pkt_size = 0; + uint32_t start_offset; ssize_t nread; uint8_t *data; @@ -226,7 +227,17 @@ buffer_shift: } data_read: - num_ready = MIN(num_ready, queue->buffer.size - queue->buffer.length); + start_offset = queue->buffer.length + queue->buffer.offset; + if (start_offset < queue->buffer.length) { + DBG_ERR("Buffer overflow\n"); + goto failed; + } + if (start_offset > queue->buffer.size) { + DBG_ERR("Buffer overflow\n"); + goto failed; + } + + num_ready = MIN(num_ready, queue->buffer.size - start_offset); if (num_ready > 0) { nread = sys_read(queue->fd, -- 2.20.1 From bc7a411cb327a5fa456543c2bceaeadb7d3924d4 Mon Sep 17 00:00:00 2001 From: Christof Schmitt Date: Tue, 19 Feb 2019 13:59:05 -0700 Subject: [PATCH 2/2] ctdb-tests: Add test for ctdb_io.c BUG: https://bugzilla.samba.org/show_bug.cgi?id=13791 Signed-off-by: Christof Schmitt Reviewed-by: Martin Schwenke Autobuild-User(master): Martin Schwenke Autobuild-Date(master): Fri Feb 22 03:51:37 CET 2019 on sn-devel-144 (cherry picked from commit 92a90524373a0348c1912d5019254dd18c07e207) --- 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..5a2f81538a1 --- /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 . +*/ + +#include "replace.h" +#include "system/filesys.h" + +#include + +#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, test1_req_len) == 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 \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..c2f1956a916 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 tdb 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.20.1