PATCH: ctdb: buffer write beyond limits

[Martin Schwenke]

On Tue, 19 Feb 2019 16:17:30 -0700, Christof Schmitt via
samba-technical <samba-technical at lists.samba.org> wrote:

> 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.

Thanks for that.  That looks really good.  I always thought we would
have problems writing unit tests for that code because it uses struct
ctdb_context, which drags in the world, but this is nice!

Ironically, the test refuses to compile with CFLAGS="-O3" and
--picky-developer:

[318/380] Compiling ctdb/tests/src/ctdb_io_test.c
In file included from ../../tests/src/ctdb_io_test.c:23:
../../tests/src/ctdb_io_test.c: In function ‘test1_callback’:
../../tests/src/ctdb_io_test.c:70:9: error: ‘__builtin_memcmp_eq’ reading 12 bytes from a region of size 9 [-Werror=stringop-overflow=]
  assert(memcmp(data  + sizeof(len), test1_req, len) == 0);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

That is, the test reads off the end of the buffer...  :-)

The fixup below seems to do the job:

diff --git a/ctdb/tests/src/ctdb_io_test.c b/ctdb/tests/src/ctdb_io_test.c
index 22bd2f4930f..5a2f81538a1 100644
--- a/ctdb/tests/src/ctdb_io_test.c
+++ b/ctdb/tests/src/ctdb_io_test.c
@@ -67,7 +67,7 @@ static void test1_callback(uint8_t *data, size_t length, void *private_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);
+       assert(memcmp(data  + sizeof(len), test1_req, test1_req_len) == 0);
 }
 
 static void test1(void)

Thanks...

peace & happiness,
martin