[LTP] [PATCH v2 00/11] Introduce guarded buffers

[LTP] [PATCH v2 00/11] Introduce guarded buffers

[Cyril Hrubis]

Changes since v1:

* Removed debugging "freeing buffer %p" output and
  added a message that is printed first time tst_alloc() is called.

* Added the missing getpid() to the warning message
  from check_canary()

* Added call to atexit(tst_free_all) in the safe_fork()
  - now we check the canary in child processes as well

* Added documentation into the test-writing-guidelines.txt

* Separated the accept02.c changes from the patch that adds
  the guarded buffers library

Cyril Hrubis (11):
  lib: Add support for guarded buffers
  lib: Add a canary for guarded buffers
  doc: Add guarded buffers documentation
  syscalls/accept02: Make use of guarded buffers.
  syscalls/preadv01: Make use of guarded buffers.
  syscalls/accept4_01: Make use of guarded buffers.
  syscalls/add_key04: Make use of guarded buffers.
  syscalls/adjtimex: Make use of guarded buffers.
  syscalls/clock_getres01: Make use of guarded buffers.
  syscalls/clock_settime01: Make use of guarded buffers.
  syscalls/sendmmsg01: Make use of guarded buffers.

 doc/test-writing-guidelines.txt               |  68 +++++++++
 include/tst_buffers.h                         |  63 ++++++++
 include/tst_test.h                            |   6 +
 lib/newlib_tests/.gitignore                   |   1 +
 lib/newlib_tests/test_guarded_buf.c           |  92 +++++++++++
 lib/tst_buffers.c                             | 143 ++++++++++++++++++
 lib/tst_test.c                                |   8 +
 testcases/kernel/syscalls/accept/accept02.c   |  41 ++---
 .../kernel/syscalls/accept4/accept4_01.c      |  24 +--
 testcases/kernel/syscalls/add_key/add_key04.c |  13 +-
 .../kernel/syscalls/adjtimex/adjtimex01.c     |  23 +--
 .../kernel/syscalls/adjtimex/adjtimex02.c     |  39 ++---
 .../syscalls/clock_getres/clock_getres01.c    |  12 +-
 .../syscalls/clock_settime/clock_settime01.c  |  29 ++--
 testcases/kernel/syscalls/preadv/preadv01.c   |  10 +-
 .../kernel/syscalls/sendmmsg/sendmmsg01.c     |  68 ++++-----
 16 files changed, 528 insertions(+), 112 deletions(-)
 create mode 100644 include/tst_buffers.h
 create mode 100644 lib/newlib_tests/test_guarded_buf.c
 create mode 100644 lib/tst_buffers.c

-- 
2.21.0

[LTP] [PATCH v2 01/11] lib: Add support for guarded buffers

[Cyril Hrubis]

This commit adds a support for guarder buffers. Guarded buffer is a
buffer allocated so that there is PROT_NONE page immediatelly after the
end of the buffer i.e. any access after the buffer generates
SEGFAULT/EFAULT etc.

The library is hooked into the tst_test structure so that all you need
is to fill up an NULL terminated array of buffer pointers and sizes to
get the respective buffers allocated. This library supports allocating
memory in test runtime as well as well as allocating more complex
buffers, which currently are iovec vectors.

All buffers are freeed automatically at the end of the test.

Example usage looks like:

static struct foo *foo_ptr;
static struct iovec *iov;
static void *buf_ptr;
static char *id;
...

static void run(void)
{
	...

	foo_ptr->bar = 1;
	foo_ptr->buf = buf_ptr;

	...
}

static void setup(void)
{
	...

	id = tst_strdup(string);

	...
}

static struct tst_test test = {
	...
	.bufs = (struct tst_buffers []) {
		{&foo_ptr, .size = sizeof(*foo_ptr)},
		{&buf_ptr, .size = BUF_SIZE},
		{&iov, .iov_sizes = (int[]){128, 32, -1},
		{}
	}
};

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 include/tst_buffers.h               |  63 ++++++++++++++++
 include/tst_test.h                  |   6 ++
 lib/newlib_tests/.gitignore         |   1 +
 lib/newlib_tests/test_guarded_buf.c |  78 +++++++++++++++++++
 lib/tst_buffers.c                   | 111 ++++++++++++++++++++++++++++
 lib/tst_test.c                      |   5 ++
 6 files changed, 264 insertions(+)
 create mode 100644 include/tst_buffers.h
 create mode 100644 lib/newlib_tests/test_guarded_buf.c
 create mode 100644 lib/tst_buffers.c

diff --git a/include/tst_buffers.h b/include/tst_buffers.h
new file mode 100644
index 000000000..d19ac8cf0
--- /dev/null
+++ b/include/tst_buffers.h
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+#ifndef TST_BUFFERS_H__
+#define TST_BUFFERS_H__
+
+/*
+ * Buffer description consist of a pointer to a pointer and buffer type/size
+ * encoded as a different structure members.
+ *
+ * Only one of the size and iov_sizes can be set at a time.
+ */
+struct tst_buffers {
+	/*
+	 * This pointer points to a buffer pointer.
+	 */
+	void *ptr;
+	/*
+	 * Buffer size.
+	 */
+	size_t size;
+	/*
+	 * Array of iov buffer sizes terminated by -1.
+	 */
+	int *iov_sizes;
+};
+
+/*
+ * Allocates buffers based on the tst_buffers structure.
+ *
+ * @bufs NULL terminated array of test buffer descriptions.
+ *
+ * This is called from the test library if the tst_test->bufs pointer is set.
+ */
+void tst_buffers_alloc(struct tst_buffers bufs[]);
+
+/*
+ * strdup() that callls tst_alloc().
+ */
+char *tst_strdup(const char *str);
+
+/*
+ * Allocates size bytes, returns pointer to the allocated buffer.
+ */
+void *tst_alloc(size_t size);
+
+/*
+ * Allocates iovec structure including the buffers.
+ *
+ * @sizes -1 terminated array of buffer sizes.
+ */
+struct iovec *tst_iovec_alloc(int sizes[]);
+
+/*
+ * Frees all allocated buffers.
+ *
+ * This is called at the end of the test automatically.
+ */
+void tst_free_all(void);
+
+#endif	/* TST_BUFFERS_H__ */
diff --git a/include/tst_test.h b/include/tst_test.h
index 2e07ff16b..cdeaf6ad0 100644
--- a/include/tst_test.h
+++ b/include/tst_test.h
@@ -35,6 +35,7 @@
 #include "tst_path_has_mnt_flags.h"
 #include "tst_sys_conf.h"
 #include "tst_coredump.h"
+#include "tst_buffers.h"
 
 /*
  * Reports testcase result.
@@ -200,6 +201,11 @@ struct tst_test {
 	 * test.
 	 */
 	const char *const *needs_kconfigs;
+
+	/*
+	 * NULL-terminated array to be allocated buffers.
+	 */
+	struct tst_buffers *bufs;
 };
 
 /*
diff --git a/lib/newlib_tests/.gitignore b/lib/newlib_tests/.gitignore
index d92b89872..6788ddf90 100644
--- a/lib/newlib_tests/.gitignore
+++ b/lib/newlib_tests/.gitignore
@@ -26,3 +26,4 @@ test_exec
 test_exec_child
 test_kconfig
 variant
+test_guarded_buf
diff --git a/lib/newlib_tests/test_guarded_buf.c b/lib/newlib_tests/test_guarded_buf.c
new file mode 100644
index 000000000..2951dce23
--- /dev/null
+++ b/lib/newlib_tests/test_guarded_buf.c
@@ -0,0 +1,78 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+/*
+ * Test that acces after guarded buffer causes segfault.
+ */
+
+#include <stdlib.h>
+#include <sys/wait.h>
+#include "tst_test.h"
+
+#define BUF1_LEN 10
+#define BUF2_LEN 4096
+#define BUF3_LEN 12004
+
+static char *buf1;
+static char *buf2;
+static char *buf3;
+
+static void do_test(unsigned int n)
+{
+	int pid = SAFE_FORK();
+	int status;
+
+	if (!pid) {
+		switch (n) {
+		case 0:
+			buf1[BUF1_LEN - 1] = 0;
+		break;
+		case 1:
+			buf2[BUF2_LEN - 1] = 0;
+		break;
+		case 2:
+			buf3[BUF3_LEN - 1] = 0;
+		break;
+		case 3:
+			buf1[BUF1_LEN] = 0;
+		break;
+		case 4:
+			buf2[BUF2_LEN] = 0;
+		break;
+		case 5:
+			buf3[BUF3_LEN] = 0;
+		break;
+		}
+
+		exit(0);
+	}
+
+	SAFE_WAITPID(pid, &status, 0);
+
+	if (n < 3) {
+		if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
+			tst_res(TPASS, "Exitted normally");
+			return;
+		}
+	} else {
+		if (WIFSIGNALED(status) && WTERMSIG(status) == SIGSEGV) {
+			tst_res(TPASS, "Killed by SIGSEGV");
+			return;
+		}
+	}
+
+	tst_res(TFAIL, "Child %s", tst_strstatus(status));
+}
+
+static struct tst_test test = {
+	.forks_child = 1,
+	.test = do_test,
+	.tcnt = 6,
+	.bufs = (struct tst_buffers []) {
+		{&buf1, .size = BUF1_LEN},
+		{&buf2, .size = BUF2_LEN},
+		{&buf3, .size = BUF3_LEN},
+	}
+};
diff --git a/lib/tst_buffers.c b/lib/tst_buffers.c
new file mode 100644
index 000000000..4f3bbe68e
--- /dev/null
+++ b/lib/tst_buffers.c
@@ -0,0 +1,111 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (c) 2019 Cyril Hrubis <chrubis@suse.cz>
+ */
+
+#include <sys/mman.h>
+#include <stdlib.h>
+#define TST_NO_DEFAULT_MAIN
+#include "tst_test.h"
+
+struct map {
+	void *addr;
+	size_t size;
+	struct map *next;
+};
+
+static struct map *maps;
+
+void *tst_alloc(size_t size)
+{
+	size_t page_size = getpagesize();
+	unsigned int pages = (size / page_size) + !!(size % page_size) + 1;
+	void *ret;
+	struct map *map = SAFE_MALLOC(sizeof(struct map));
+	static int print_msg = 1;
+
+	if (print_msg) {
+		tst_res(TINFO, "Test is using guarded buffers");
+		print_msg = 0;
+	}
+
+	ret = SAFE_MMAP(NULL, page_size * pages, PROT_READ | PROT_WRITE,
+	                MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
+
+	mprotect(ret + (pages-1) * page_size, page_size, PROT_NONE);
+
+	map->addr = ret;
+	map->size = pages * page_size;
+	map->next = maps;
+	maps = map;
+
+	if (size % page_size)
+		ret += page_size - (size % page_size);
+
+	return ret;
+}
+
+static int count_iovec(int *sizes)
+{
+	int ret = 0;
+
+	while (sizes[ret++] != -1);
+
+	return ret - 1;
+}
+
+struct iovec *tst_iovec_alloc(int sizes[])
+{
+	int i, cnt = count_iovec(sizes);
+	struct iovec *iovec;
+
+	if (cnt <= 0)
+		return NULL;
+
+	iovec = tst_alloc(sizeof(struct iovec) * cnt);
+
+	for (i = 0; i < cnt; i++) {
+		if (sizes[i]) {
+			iovec[i].iov_base = tst_alloc(sizes[i]);
+			iovec[i].iov_len = sizes[i];
+		} else {
+			iovec[i].iov_base = NULL;
+			iovec[i].iov_base = 0;
+		}
+	}
+
+	return iovec;
+}
+
+void tst_buffers_alloc(struct tst_buffers bufs[])
+{
+	unsigned int i;
+
+	for (i = 0; bufs[i].ptr; i++) {
+		if (bufs[i].size)
+			*((void**)bufs[i].ptr) = tst_alloc(bufs[i].size);
+		else
+			*((void**)bufs[i].ptr) = tst_iovec_alloc(bufs[i].iov_sizes);
+	}
+}
+
+char *tst_strdup(const char *str)
+{
+	size_t len = strlen(str);
+	char *ret = tst_alloc(len + 1);
+	return strcpy(ret, str);
+}
+
+void tst_free_all(void)
+{
+	struct map *i = maps;
+
+	while (i) {
+		struct map *j = i;
+		SAFE_MUNMAP(i->addr, i->size);
+		i = i->next;
+		free(j);
+	}
+
+	maps = NULL;
+}
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 245e287fa..8dc71dbb3 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -283,6 +283,8 @@ static void do_test_cleanup(void)
 	if (tst_test->cleanup)
 		tst_test->cleanup();
 
+	tst_free_all();
+
 	tst_brk_handler = tst_vbrk_;
 }
 
@@ -802,6 +804,9 @@ static void do_setup(int argc, char *argv[])
 
 	setup_ipc();
 
+	if (tst_test->bufs)
+		tst_buffers_alloc(tst_test->bufs);
+
 	if (needs_tmpdir() && !tst_tmpdir_created())
 		tst_tmpdir();
 
-- 
2.21.0

[LTP] [PATCH v2 02/11] lib: Add a canary for guarded buffers

[Cyril Hrubis]

In a case that the buffer size is not a multiple of a page size there is
unused space before the start of the buffer. Let's fill that with
center mirrored random bytes and check that the buffer wasn't modified
before we unmap it.

The tst_free_all() function is also set to be executed atexit() in the
safe_fork() function for all child processes, which guaratees that
canaries are checked in child processes as well.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 lib/newlib_tests/test_guarded_buf.c | 18 +++++++++++++--
 lib/tst_buffers.c                   | 36 +++++++++++++++++++++++++++--
 lib/tst_test.c                      |  3 +++
 3 files changed, 53 insertions(+), 4 deletions(-)

diff --git a/lib/newlib_tests/test_guarded_buf.c b/lib/newlib_tests/test_guarded_buf.c
index 2951dce23..506802114 100644
--- a/lib/newlib_tests/test_guarded_buf.c
+++ b/lib/newlib_tests/test_guarded_buf.c
@@ -21,9 +21,16 @@ static char *buf3;
 
 static void do_test(unsigned int n)
 {
-	int pid = SAFE_FORK();
+	int pid;
 	int status;
 
+	if (n == 6) {
+		buf1[-1] = 0;
+		buf3[-1] = 0;
+		tst_res(TPASS, "Buffers dirtied!");
+	}
+
+	pid = SAFE_FORK();
 	if (!pid) {
 		switch (n) {
 		case 0:
@@ -44,6 +51,10 @@ static void do_test(unsigned int n)
 		case 5:
 			buf3[BUF3_LEN] = 0;
 		break;
+		case 6:
+			buf1[-2] = 0;
+			buf3[-2] = 0;
+		break;
 		}
 
 		exit(0);
@@ -51,6 +62,9 @@ static void do_test(unsigned int n)
 
 	SAFE_WAITPID(pid, &status, 0);
 
+	if (n == 6)
+		return;
+
 	if (n < 3) {
 		if (WIFEXITED(status) && WEXITSTATUS(status) == 0) {
 			tst_res(TPASS, "Exitted normally");
@@ -69,7 +83,7 @@ static void do_test(unsigned int n)
 static struct tst_test test = {
 	.forks_child = 1,
 	.test = do_test,
-	.tcnt = 6,
+	.tcnt = 7,
 	.bufs = (struct tst_buffers []) {
 		{&buf1, .size = BUF1_LEN},
 		{&buf2, .size = BUF2_LEN},
diff --git a/lib/tst_buffers.c b/lib/tst_buffers.c
index 4f3bbe68e..b8b597a12 100644
--- a/lib/tst_buffers.c
+++ b/lib/tst_buffers.c
@@ -11,11 +11,38 @@
 struct map {
 	void *addr;
 	size_t size;
+	size_t buf_shift;
 	struct map *next;
 };
 
 static struct map *maps;
 
+static void setup_canary(struct map *map)
+{
+	size_t i;
+	char *buf = map->addr;
+
+	for (i = 0; i < map->buf_shift/2; i++) {
+		char c = random();
+		buf[map->buf_shift - i - 1] = c;
+		buf[i] = c;
+	}
+}
+
+static void check_canary(struct map *map)
+{
+	size_t i;
+	char *buf = map->addr;
+
+	for (i = 0; i < map->buf_shift/2; i++) {
+		if (buf[map->buf_shift - i - 1] != buf[i]) {
+			tst_res(TWARN,
+				"pid %i: buffer modified address %p[%zi]",
+				getpid(), (char*)map->addr + map->buf_shift, -i-1);
+		}
+	}
+}
+
 void *tst_alloc(size_t size)
 {
 	size_t page_size = getpagesize();
@@ -40,9 +67,13 @@ void *tst_alloc(size_t size)
 	maps = map;
 
 	if (size % page_size)
-		ret += page_size - (size % page_size);
+		map->buf_shift = page_size - (size % page_size);
+	else
+		map->buf_shift = 0;
+
+	setup_canary(map);
 
-	return ret;
+	return ret + map->buf_shift;
 }
 
 static int count_iovec(int *sizes)
@@ -102,6 +133,7 @@ void tst_free_all(void)
 
 	while (i) {
 		struct map *j = i;
+		check_canary(i);
 		SAFE_MUNMAP(i->addr, i->size);
 		i = i->next;
 		free(j);
diff --git a/lib/tst_test.c b/lib/tst_test.c
index 8dc71dbb3..39f261472 100644
--- a/lib/tst_test.c
+++ b/lib/tst_test.c
@@ -390,6 +390,9 @@ pid_t safe_fork(const char *filename, unsigned int lineno)
 	if (pid < 0)
 		tst_brk_(filename, lineno, TBROK | TERRNO, "fork() failed");
 
+	if (!pid)
+		atexit(tst_free_all);
+
 	return pid;
 }
 
-- 
2.21.0

[LTP] [PATCH v2 03/11] doc: Add guarded buffers documentation

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 doc/test-writing-guidelines.txt | 68 +++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
index 573dd08d9..e5ee2fef0 100644
--- a/doc/test-writing-guidelines.txt
+++ b/doc/test-writing-guidelines.txt
@@ -1699,6 +1699,74 @@ struct tst_test test = {
 };
 -------------------------------------------------------------------------------
 
+2.2.31 Guarded buffers
+^^^^^^^^^^^^^^^^^^^^^^
+
+The test library also supports guarded buffers, which are buffers allocated so
+that:
+
+* The end of the buffer is followed by PROT_NONE page
+
+* The rest of the page before the buffer is filled with random canary
+
+Which means that the any access after the buffer with yield Segmentation
+fault or EFAULT depending on if the access happened in userspace or kernel
+respectively. The canary before the buffer will also catch any write access
+outside of the buffer.
+
+The purpose of the patch is to catch off-by-one bugs happening while buffers
+and structures are passed to syscalls. New tests should allocate guarded
+buffers for all data passed to the tested syscall which are passed by a
+pointer.
+
+[source,c]
+-------------------------------------------------------------------------------
+#include "tst_test.h"
+
+static struct foo *foo_ptr;
+static struct iovec *iov;
+static void *buf_ptr;
+static char *id;
+...
+
+static void run(void)
+{
+	...
+
+	foo_ptr->bar = 1;
+	foo_ptr->buf = buf_ptr;
+
+	...
+}
+
+static void setup(void)
+{
+	...
+
+	id = tst_strdup(string);
+
+	...
+}
+
+static struct tst_test test = {
+	...
+	.bufs = (struct tst_buffers []) {
+		{&foo_ptr, .size = sizeof(*foo_ptr)},
+		{&buf_ptr, .size = BUF_SIZE},
+		{&iov, .iov_sizes = (int[]){128, 32, -1},
+		{}
+	}
+};
+-------------------------------------------------------------------------------
+
+Guarded buffers can be allocated on runtime in a test setup() by a
+'tst_alloc()' or by 'tst_strdup()' as well as by filling up the .bufs array in
+the tst_test structure.
+
+So far the tst_test structure supports allocating either a plain buffer by
+setting up the size or struct iovec, which is allocated recursively including
+the individual buffers as described by an '-1' terminated array of buffer
+sizes.
 
 2.3 Writing a testcase in shell
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
-- 
2.21.0

[LTP] [PATCH v2 04/11] syscalls/accept02: Make use of guarded buffers.

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/syscalls/accept/accept02.c | 41 +++++++++++----------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/testcases/kernel/syscalls/accept/accept02.c b/testcases/kernel/syscalls/accept/accept02.c
index df048ede4..1a0f625c9 100644
--- a/testcases/kernel/syscalls/accept/accept02.c
+++ b/testcases/kernel/syscalls/accept/accept02.c
@@ -33,10 +33,9 @@ static int client_sockfd;
 static int server_port;
 static socklen_t addr_len;
 
-static struct sockaddr_in server_addr;
-static struct sockaddr_in client_addr;
-static struct group_req mc_group;
-
+static struct sockaddr_in *server_addr;
+static struct sockaddr_in *client_addr;
+static struct group_req *mc_group;
 
 static void *server_thread(void *arg)
 {
@@ -44,27 +43,27 @@ static void *server_thread(void *arg)
 
 	op = 1;
 	op_len = sizeof(op);
-	mc_group_len = sizeof(mc_group);
+	mc_group_len = sizeof(*mc_group);
 
 	server_sockfd = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
 
 	SAFE_SETSOCKOPT(server_sockfd, SOL_SOCKET, SO_REUSEADDR, &op, op_len);
 	SAFE_SETSOCKOPT(server_sockfd, SOL_IP, MCAST_JOIN_GROUP,
-			&mc_group, mc_group_len);
+			mc_group, mc_group_len);
 
-	SAFE_BIND(server_sockfd, (struct sockaddr *)&server_addr, addr_len);
+	SAFE_BIND(server_sockfd, (struct sockaddr *)server_addr, addr_len);
 	SAFE_LISTEN(server_sockfd, 1);
 
 	TST_CHECKPOINT_WAKE(0);
 
-	TEST(accept(server_sockfd, (struct sockaddr *)&client_addr, &addr_len));
+	TEST(accept(server_sockfd, (struct sockaddr *)client_addr, &addr_len));
 	if (TST_RET == -1)
 		tst_brk(TBROK | TTERRNO, "Could not accept connection");
 
 	clone_server_sockfd = TST_RET;
 
 	TEST(setsockopt(clone_server_sockfd, SOL_IP, MCAST_LEAVE_GROUP,
-			&mc_group, mc_group_len));
+			mc_group, mc_group_len));
 
 	if (TST_RET != -1)
 		tst_res(TFAIL, "Multicast group was copied!");
@@ -80,9 +79,9 @@ static void *server_thread(void *arg)
 static void *client_thread(void *arg)
 {
 	client_sockfd = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
-	SAFE_BIND(client_sockfd, (struct sockaddr *)&client_addr, addr_len);
+	SAFE_BIND(client_sockfd, (struct sockaddr *)client_addr, addr_len);
 
-	SAFE_CONNECT(client_sockfd, (struct sockaddr *)&server_addr, addr_len);
+	SAFE_CONNECT(client_sockfd, (struct sockaddr *)server_addr, addr_len);
 
 	SAFE_CLOSE(client_sockfd);
 	return arg;
@@ -92,8 +91,8 @@ static void run(void)
 {
 	pthread_t server_thr, client_thr;
 
-	server_addr.sin_port = server_port;
-	client_addr.sin_port = htons(0);
+	server_addr->sin_port = server_port;
+	client_addr->sin_port = htons(0);
 
 	SAFE_PTHREAD_CREATE(&server_thr, NULL, server_thread, NULL);
 	TST_CHECKPOINT_WAIT(0);
@@ -107,16 +106,20 @@ static void setup(void)
 {
 	struct sockaddr_in *mc_group_addr;
 
-	mc_group.gr_interface = 0;
-	mc_group_addr = (struct sockaddr_in *) &mc_group.gr_group;
+	server_addr = tst_alloc(sizeof(*server_addr));
+	client_addr = tst_alloc(sizeof(*client_addr));
+	mc_group = tst_alloc(sizeof(*mc_group));
+
+	mc_group->gr_interface = 0;
+	mc_group_addr = (struct sockaddr_in *) &mc_group->gr_group;
 	mc_group_addr->sin_family = AF_INET;
 	inet_aton(MULTICASTIP, &mc_group_addr->sin_addr);
 
-	server_addr.sin_family = AF_INET;
-	inet_aton(LOCALHOSTIP, &server_addr.sin_addr);
+	server_addr->sin_family = AF_INET;
+	inet_aton(LOCALHOSTIP, &server_addr->sin_addr);
 
-	client_addr.sin_family = AF_INET;
-	client_addr.sin_addr.s_addr = htons(INADDR_ANY);
+	client_addr->sin_family = AF_INET;
+	client_addr->sin_addr.s_addr = htons(INADDR_ANY);
 
 	addr_len = sizeof(struct sockaddr_in);
 
-- 
2.21.0

[LTP] [PATCH v2 05/11] syscalls/preadv01: Make use of guarded buffers.

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/syscalls/preadv/preadv01.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/testcases/kernel/syscalls/preadv/preadv01.c b/testcases/kernel/syscalls/preadv/preadv01.c
index 2452f216a..95431bc60 100644
--- a/testcases/kernel/syscalls/preadv/preadv01.c
+++ b/testcases/kernel/syscalls/preadv/preadv01.c
@@ -24,12 +24,8 @@
 #define CHUNK           64
 
 static int fd;
-static char buf[CHUNK];
 
-static struct iovec rd_iovec[] = {
-	{buf, CHUNK},
-	{NULL, 0},
-};
+static struct iovec *rd_iovec;
 
 static struct tcase {
 	int count;
@@ -111,4 +107,8 @@ static struct tst_test test = {
 	.test = verify_preadv,
 	.min_kver = "2.6.30",
 	.needs_tmpdir = 1,
+	.bufs = (struct tst_buffers []) {
+		{&rd_iovec, .iov_sizes = (int[]){CHUNK, 0, -1}},
+		{},
+	}
 };
-- 
2.21.0

[LTP] [PATCH v2 06/11] syscalls/accept4_01: Make use of guarded buffers.

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../kernel/syscalls/accept4/accept4_01.c      | 24 +++++++++++--------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/testcases/kernel/syscalls/accept4/accept4_01.c b/testcases/kernel/syscalls/accept4/accept4_01.c
index dd289cf6d..29e18f27d 100644
--- a/testcases/kernel/syscalls/accept4/accept4_01.c
+++ b/testcases/kernel/syscalls/accept4/accept4_01.c
@@ -35,7 +35,7 @@
 #define USE_SOCKETCALL 1
 #endif
 
-static struct sockaddr_in conn_addr;
+static struct sockaddr_in *conn_addr, *accept_addr;
 static int listening_fd;
 
 #if !(__GLIBC_PREREQ(2, 10))
@@ -80,10 +80,10 @@ static int create_listening_socket(void)
 
 static void setup(void)
 {
-	memset(&conn_addr, 0, sizeof(struct sockaddr_in));
-	conn_addr.sin_family = AF_INET;
-	conn_addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK);
-	conn_addr.sin_port = htons(PORT_NUM);
+	memset(conn_addr, 0, sizeof(*conn_addr));
+	conn_addr->sin_family = AF_INET;
+	conn_addr->sin_addr.s_addr = htonl(INADDR_LOOPBACK);
+	conn_addr->sin_port = htons(PORT_NUM);
 
 	listening_fd = create_listening_socket();
 }
@@ -108,18 +108,17 @@ static void verify_accept4(unsigned int nr)
 	struct test_case *tcase = &tcases[nr];
 	int connfd, acceptfd;
 	int fdf, flf, fdf_pass, flf_pass, fd_cloexec, fd_nonblock;
-	struct sockaddr_in claddr;
 	socklen_t addrlen;
 
 	connfd = SAFE_SOCKET(AF_INET, SOCK_STREAM, 0);
-	SAFE_CONNECT(connfd, (struct sockaddr *)&conn_addr, sizeof(conn_addr));
-	addrlen = sizeof(claddr);
+	SAFE_CONNECT(connfd, (struct sockaddr *)conn_addr, sizeof(*conn_addr));
+	addrlen = sizeof(*accept_addr);
 
 #if !(__GLIBC_PREREQ(2, 10))
-	TEST(accept4_01(listening_fd, (struct sockaddr *)&claddr, &addrlen,
+	TEST(accept4_01(listening_fd, (struct sockaddr *)accept_addr, &addrlen,
 				tcase->cloexec | tcase->nonblock));
 #else
-	TEST(accept4(listening_fd, (struct sockaddr *)&claddr, &addrlen,
+	TEST(accept4(listening_fd, (struct sockaddr *)accept_addr, &addrlen,
 				tcase->cloexec | tcase->nonblock));
 #endif
 	if (TST_RET == -1) {
@@ -163,4 +162,9 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test = verify_accept4,
+	.bufs = (struct tst_buffers []) {
+		{&conn_addr, .size = sizeof(*conn_addr)},
+		{&accept_addr, .size = sizeof(*accept_addr)},
+		{},
+	}
 };
-- 
2.21.0

[LTP] [PATCH v2 07/11] syscalls/add_key04: Make use of guarded buffers.

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 testcases/kernel/syscalls/add_key/add_key04.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/testcases/kernel/syscalls/add_key/add_key04.c b/testcases/kernel/syscalls/add_key/add_key04.c
index 28cc91f72..12a169eda 100644
--- a/testcases/kernel/syscalls/add_key/add_key04.c
+++ b/testcases/kernel/syscalls/add_key/add_key04.c
@@ -32,6 +32,10 @@
 
 #define ASSOC_ARRAY_FAN_OUT 16
 
+#define PAYLOAD "payload"
+
+static char *payload;
+
 static void do_test(void)
 {
 	int status;
@@ -42,7 +46,6 @@ static void do_test(void)
 
 	if (SAFE_FORK() == 0) {
 		char description[32];
-		const char payload[] = "payload";
 		int i;
 
 		for (i = 0; i < ASSOC_ARRAY_FAN_OUT; i++) {
@@ -55,7 +58,7 @@ static void do_test(void)
 			}
 		}
 
-		TEST(add_key("user", "userkey", payload, sizeof(payload),
+		TEST(add_key("user", "userkey", payload, sizeof(PAYLOAD),
 			     KEY_SPEC_SESSION_KEYRING));
 		if (TST_RET < 0)
 			tst_brk(TBROK | TTERRNO, "unable to create user key");
@@ -72,7 +75,13 @@ static void do_test(void)
 		tst_brk(TBROK, "Child %s", tst_strstatus(status));
 }
 
+static void setup(void)
+{
+	payload = tst_strdup(PAYLOAD);
+}
+
 static struct tst_test test = {
+	.setup = setup,
 	.test_all = do_test,
 	.forks_child = 1,
 };
-- 
2.21.0

[LTP] [PATCH v2 08/11] syscalls/adjtimex: Make use of guarded buffers.

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../kernel/syscalls/adjtimex/adjtimex01.c     | 23 ++++++-----
 .../kernel/syscalls/adjtimex/adjtimex02.c     | 39 +++++++++++--------
 2 files changed, 36 insertions(+), 26 deletions(-)

diff --git a/testcases/kernel/syscalls/adjtimex/adjtimex01.c b/testcases/kernel/syscalls/adjtimex/adjtimex01.c
index 51d75f3e0..60b3544a8 100644
--- a/testcases/kernel/syscalls/adjtimex/adjtimex01.c
+++ b/testcases/kernel/syscalls/adjtimex/adjtimex01.c
@@ -12,14 +12,14 @@
 #define SET_MODE (ADJ_OFFSET | ADJ_FREQUENCY | ADJ_MAXERROR | ADJ_ESTERROR | \
 	ADJ_STATUS | ADJ_TIMECONST | ADJ_TICK)
 
-static struct timex tim_save;
-static struct timex buff;
+static struct timex *tim_save;
+static struct timex *buf;
 
 void verify_adjtimex(void)
 {
-	buff = tim_save;
-	buff.modes = SET_MODE;
-	TEST(adjtimex(&buff));
+	*buf = *tim_save;
+	buf->modes = SET_MODE;
+	TEST(adjtimex(buf));
 	if ((TST_RET >= TIME_OK) && (TST_RET <= TIME_ERROR)) {
 		tst_res(TPASS, "adjtimex() with mode 0x%x ", SET_MODE);
 	} else {
@@ -27,8 +27,8 @@ void verify_adjtimex(void)
 				SET_MODE);
 	}
 
-	buff.modes = ADJ_OFFSET_SINGLESHOT;
-	TEST(adjtimex(&buff));
+	buf->modes = ADJ_OFFSET_SINGLESHOT;
+	TEST(adjtimex(buf));
 	if ((TST_RET >= TIME_OK) && (TST_RET <= TIME_ERROR)) {
 		tst_res(TPASS, "adjtimex() with mode 0x%x ",
 				ADJ_OFFSET_SINGLESHOT);
@@ -41,10 +41,10 @@ void verify_adjtimex(void)
 
 static void setup(void)
 {
-	tim_save.modes = 0;
+	tim_save->modes = 0;
 
 	/* Save current parameters */
-	if ((adjtimex(&tim_save)) == -1) {
+	if ((adjtimex(tim_save)) == -1) {
 		tst_brk(TBROK | TERRNO,
 			"adjtimex(): failed to save current params");
 	}
@@ -54,4 +54,9 @@ static struct tst_test test = {
 	.needs_root = 1,
 	.setup = setup,
 	.test_all = verify_adjtimex,
+	.bufs = (struct tst_buffers []) {
+		{&buf, .size = sizeof(*buf)},
+		{&tim_save, .size = sizeof(*tim_save)},
+		{},
+	}
 };
diff --git a/testcases/kernel/syscalls/adjtimex/adjtimex02.c b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
index 2c0031992..19ee97158 100644
--- a/testcases/kernel/syscalls/adjtimex/adjtimex02.c
+++ b/testcases/kernel/syscalls/adjtimex/adjtimex02.c
@@ -16,14 +16,14 @@
 
 static int hz;			/* HZ from sysconf */
 
-static struct timex tim_save;
-static struct timex buff;
+static struct timex *tim_save;
+static struct timex *buf;
 
 static struct passwd *ltpuser;
 
 static void verify_adjtimex(unsigned int nr)
 {
-	struct timex *buffp;
+	struct timex *bufp;
 	int expected_errno = 0;
 
 	/*
@@ -39,20 +39,20 @@ static void verify_adjtimex(unsigned int nr)
 		return;
 	}
 
-	buff = tim_save;
-	buff.modes = SET_MODE;
-	buffp = &buff;
+	*buf = *tim_save;
+	buf->modes = SET_MODE;
+	bufp = buf;
 	switch (nr) {
 	case 0:
-		buffp = (struct timex *)-1;
+		bufp = (struct timex *)-1;
 		expected_errno = EFAULT;
 		break;
 	case 1:
-		buff.tick = 900000 / hz - 1;
+		buf->tick = 900000 / hz - 1;
 		expected_errno = EINVAL;
 		break;
 	case 2:
-		buff.tick = 1100000 / hz + 1;
+		buf->tick = 1100000 / hz + 1;
 		expected_errno = EINVAL;
 		break;
 	case 3:
@@ -62,18 +62,18 @@ static void verify_adjtimex(unsigned int nr)
 		expected_errno = EPERM;
 		break;
 	case 4:
-		buff.offset = 512000L + 1;
+		buf->offset = 512000L + 1;
 		expected_errno = EINVAL;
 		break;
 	case 5:
-		buff.offset = (-1) * (512000L) - 1;
+		buf->offset = (-1) * (512000L) - 1;
 		expected_errno = EINVAL;
 		break;
 	default:
 		tst_brk(TFAIL, "Invalid test case %u ", nr);
 	}
 
-	TEST(adjtimex(buffp));
+	TEST(adjtimex(bufp));
 	if ((TST_RET == -1) && (TST_ERR == expected_errno)) {
 		tst_res(TPASS | TTERRNO,
 				"adjtimex() error %u ", expected_errno);
@@ -90,23 +90,23 @@ static void verify_adjtimex(unsigned int nr)
 
 static void setup(void)
 {
-	tim_save.modes = 0;
+	tim_save->modes = 0;
 
 	/* set the HZ from sysconf */
 	hz = SAFE_SYSCONF(_SC_CLK_TCK);
 
 	/* Save current parameters */
-	if ((adjtimex(&tim_save)) == -1)
+	if ((adjtimex(tim_save)) == -1)
 		tst_brk(TBROK | TERRNO,
-				"adjtimex(): failed to save current params");
+			"adjtimex(): failed to save current params");
 }
 
 static void cleanup(void)
 {
-	tim_save.modes = SET_MODE;
+	tim_save->modes = SET_MODE;
 
 	/* Restore saved parameters */
-	if ((adjtimex(&tim_save)) == -1)
+	if ((adjtimex(tim_save)) == -1)
 		tst_res(TWARN, "Failed to restore saved parameters");
 }
 
@@ -116,4 +116,9 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test = verify_adjtimex,
+	.bufs = (struct tst_buffers []) {
+		{&buf, .size = sizeof(*buf)},
+		{&tim_save, .size = sizeof(*tim_save)},
+		{},
+	}
 };
-- 
2.21.0

[LTP] [PATCH v2 09/11] syscalls/clock_getres01: Make use of guarded buffers.

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../kernel/syscalls/clock_getres/clock_getres01.c    | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/syscalls/clock_getres/clock_getres01.c b/testcases/kernel/syscalls/clock_getres/clock_getres01.c
index 15f323108..e39f2909b 100644
--- a/testcases/kernel/syscalls/clock_getres/clock_getres01.c
+++ b/testcases/kernel/syscalls/clock_getres/clock_getres01.c
@@ -15,12 +15,12 @@
 #include "tst_test.h"
 #include "lapi/posix_clocks.h"
 
-static struct timespec res;
+static struct timespec *res, *null;
 
 static struct test_case {
 	char *name;
 	clockid_t clk_id;
-	struct timespec *res;
+	struct timespec **res;
 	int ret;
 	int err;
 } tcase[] = {
@@ -28,7 +28,7 @@ static struct test_case {
 	{"MONOTONIC", CLOCK_MONOTONIC, &res, 0, 0},
 	{"PROCESS_CPUTIME_ID", CLOCK_PROCESS_CPUTIME_ID, &res, 0, 0},
 	{"THREAD_CPUTIME_ID", CLOCK_THREAD_CPUTIME_ID, &res, 0, 0},
-	{"REALTIME", CLOCK_REALTIME, NULL, 0, 0},
+	{"REALTIME", CLOCK_REALTIME, &null, 0, 0},
 	{"CLOCK_MONOTONIC_RAW", CLOCK_MONOTONIC_RAW, &res, 0, 0,},
 	{"CLOCK_REALTIME_COARSE", CLOCK_REALTIME_COARSE, &res, 0, 0,},
 	{"CLOCK_MONOTONIC_COARSE", CLOCK_MONOTONIC_COARSE, &res, 0, 0,},
@@ -40,7 +40,7 @@ static struct test_case {
 
 static void do_test(unsigned int i)
 {
-	TEST(clock_getres(tcase[i].clk_id, tcase[i].res));
+	TEST(clock_getres(tcase[i].clk_id, *tcase[i].res));
 
 	if (TST_RET != tcase[i].ret) {
 		if (TST_ERR == EINVAL) {
@@ -65,4 +65,8 @@ static void do_test(unsigned int i)
 static struct tst_test test = {
 	.test = do_test,
 	.tcnt = ARRAY_SIZE(tcase),
+	.bufs = (struct tst_buffers []) {
+		{&res, .size = sizeof(*res)},
+		{},
+	}
 };
-- 
2.21.0

[LTP] [PATCH v2 10/11] syscalls/clock_settime01: Make use of guarded buffers.

[Cyril Hrubis]

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../syscalls/clock_settime/clock_settime01.c  | 29 ++++++++++++-------
 1 file changed, 18 insertions(+), 11 deletions(-)

diff --git a/testcases/kernel/syscalls/clock_settime/clock_settime01.c b/testcases/kernel/syscalls/clock_settime/clock_settime01.c
index c68fe59a1..62d349154 100644
--- a/testcases/kernel/syscalls/clock_settime/clock_settime01.c
+++ b/testcases/kernel/syscalls/clock_settime/clock_settime01.c
@@ -23,23 +23,24 @@
 #define DELTA_US (long long) (DELTA_SEC * 1000000)
 #define DELTA_EPS (long long) (DELTA_US * 0.1)
 
+static struct timespec *begin, *change, *end;
+
 static void verify_clock_settime(void)
 {
 	long long elapsed;
-	struct timespec begin, change, end;
 
 	/* test 01: move forward */
 
-	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, &begin);
+	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, begin);
 
-	change = tst_timespec_add_us(begin, DELTA_US);
+	*change = tst_timespec_add_us(*begin, DELTA_US);
 
-	if (clock_settime(CLOCK_REALTIME, &change) != 0)
+	if (clock_settime(CLOCK_REALTIME, change) != 0)
 		tst_brk(TBROK | TTERRNO, "could not set realtime change");
 
-	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, &end);
+	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, end);
 
-	elapsed = tst_timespec_diff_us(end, begin);
+	elapsed = tst_timespec_diff_us(*end, *begin);
 
 	if (elapsed >= DELTA_US && elapsed < (DELTA_US + DELTA_EPS))
 		tst_res(TPASS, "clock_settime(2): was able to advance time");
@@ -48,16 +49,16 @@ static void verify_clock_settime(void)
 
 	/* test 02: move backward */
 
-	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, &begin);
+	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, begin);
 
-	change = tst_timespec_sub_us(begin, DELTA_US);
+	*change = tst_timespec_sub_us(*begin, DELTA_US);
 
-	if (clock_settime(CLOCK_REALTIME, &change) != 0)
+	if (clock_settime(CLOCK_REALTIME, change) != 0)
 		tst_brk(TBROK | TTERRNO, "could not set realtime change");
 
-	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, &end);
+	SAFE_CLOCK_GETTIME(CLOCK_REALTIME, end);
 
-	elapsed = tst_timespec_diff_us(end, begin);
+	elapsed = tst_timespec_diff_us(*end, *begin);
 
 	if (~(elapsed) <= DELTA_US && ~(elapsed) > (DELTA_US - DELTA_EPS))
 		tst_res(TPASS, "clock_settime(2): was able to recede time");
@@ -69,4 +70,10 @@ static struct tst_test test = {
 	.test_all = verify_clock_settime,
 	.needs_root = 1,
 	.restore_wallclock = 1,
+	.bufs = (struct tst_buffers []) {
+		{&begin, .size = sizeof(*begin)},
+		{&change, .size = sizeof(*change)},
+		{&end, .size = sizeof(*end)},
+		{},
+	}
 };
-- 
2.21.0

[LTP] [PATCH v2 11/11] syscalls/sendmmsg01: Make use of guarded buffers.

[Cyril Hrubis]

We also send one more byte in the second buffer in an attempt to trick
the kernel to write after the second iovec used for receive. Note that
because this is UDP connection this byte is then discarded in kernel and
we don't have to anything even when the test is running in a loop with
the -i parameter.

Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
---
 .../kernel/syscalls/sendmmsg/sendmmsg01.c     | 68 +++++++++----------
 1 file changed, 33 insertions(+), 35 deletions(-)

diff --git a/testcases/kernel/syscalls/sendmmsg/sendmmsg01.c b/testcases/kernel/syscalls/sendmmsg/sendmmsg01.c
index 7411467ee..37084102e 100644
--- a/testcases/kernel/syscalls/sendmmsg/sendmmsg01.c
+++ b/testcases/kernel/syscalls/sendmmsg/sendmmsg01.c
@@ -22,35 +22,27 @@
 
 static int send_sockfd;
 static int receive_sockfd;
-static struct mmsghdr msg[VLEN];
-static struct iovec msg1[2], msg2;
+static struct mmsghdr *snd_msg, *rcv_msg;
+static struct iovec *snd1, *snd2, *rcv1, *rcv2;
 
 static void run(void)
 {
-	struct mmsghdr msgs_in[VLEN];
-	struct iovec iovecs[VLEN];
-	char bufs[VLEN][BUFSIZE+1];
 	struct timespec timeout;
-	int i, retval;
+	int retval;
 
-	retval = do_sendmmsg(send_sockfd, msg, VLEN, 0);
-	if (retval < 0 || msg[0].msg_len != 6 || msg[1].msg_len != 5) {
+	retval = do_sendmmsg(send_sockfd, snd_msg, VLEN, 0);
+	if (retval < 0 || snd_msg[0].msg_len != 6 || snd_msg[1].msg_len != 6) {
 		tst_res(TFAIL|TTERRNO, "sendmmsg failed");
 		return;
 	}
 
-	memset(msgs_in, 0, sizeof(msgs_in));
-	for (i = 0; i < VLEN; i++) {
-		iovecs[i].iov_base = bufs[i];
-		iovecs[i].iov_len = BUFSIZE;
-		msgs_in[i].msg_hdr.msg_iov = &iovecs[i];
-		msgs_in[i].msg_hdr.msg_iovlen = 1;
-	}
+	memset(rcv1->iov_base, 0, rcv1->iov_len);
+	memset(rcv2->iov_base, 0, rcv2->iov_len);
 
 	timeout.tv_sec = 1;
 	timeout.tv_nsec = 0;
 
-	retval = do_recvmmsg(receive_sockfd, msgs_in, VLEN, 0, &timeout);
+	retval = do_recvmmsg(receive_sockfd, rcv_msg, VLEN, 0, &timeout);
 
 	if (retval == -1) {
 		tst_res(TFAIL | TTERRNO, "recvmmsg failed");
@@ -62,14 +54,12 @@ static void run(void)
 		return;
 	}
 
-	bufs[0][msgs_in[0].msg_len] = 0;
-	if (strcmp(bufs[0], "onetwo"))
+	if (memcmp(rcv1->iov_base, "onetwo", 6))
 		tst_res(TFAIL, "Error in first received message");
 	else
 		tst_res(TPASS, "First message received successfully");
 
-	bufs[1][msgs_in[1].msg_len] = 0;
-	if (strcmp(bufs[1], "three"))
+	if (memcmp(rcv2->iov_base, "three", 5))
 		tst_res(TFAIL, "Error in second received message");
 	else
 		tst_res(TPASS, "Second message received successfully");
@@ -88,24 +78,23 @@ static void setup(void)
 	addr.sin_port = port;
 
 	SAFE_BIND(receive_sockfd, (struct sockaddr *)&addr, sizeof(addr));
-	SAFE_CONNECT(send_sockfd, (struct sockaddr *) &addr, sizeof(addr));
-
-	memset(msg1, 0, sizeof(msg1));
-	msg1[0].iov_base = "one";
-	msg1[0].iov_len = 3;
-	msg1[1].iov_base = "two";
-	msg1[1].iov_len = 3;
+	SAFE_CONNECT(send_sockfd, (struct sockaddr *)&addr, sizeof(addr));
 
-	memset(&msg2, 0, sizeof(msg2));
-	msg2.iov_base = "three";
-	msg2.iov_len = 5;
+	memcpy(snd1[0].iov_base, "one", snd1[0].iov_len);
+	memcpy(snd1[1].iov_base, "two", snd1[1].iov_len);
+	memcpy(snd2->iov_base, "three3", snd2->iov_len);
 
-	memset(msg, 0, sizeof(msg));
-	msg[0].msg_hdr.msg_iov = msg1;
-	msg[0].msg_hdr.msg_iovlen = 2;
+	memset(snd_msg, 0, VLEN * sizeof(*snd_msg));
+	snd_msg[0].msg_hdr.msg_iov = snd1;
+	snd_msg[0].msg_hdr.msg_iovlen = 2;
+	snd_msg[1].msg_hdr.msg_iov = snd2;
+	snd_msg[1].msg_hdr.msg_iovlen = 1;
 
-	msg[1].msg_hdr.msg_iov = &msg2;
-	msg[1].msg_hdr.msg_iovlen = 1;
+	memset(rcv_msg, 0, VLEN * sizeof(*rcv_msg));
+	rcv_msg[0].msg_hdr.msg_iov = rcv1;
+	rcv_msg[0].msg_hdr.msg_iovlen = 1;
+	rcv_msg[1].msg_hdr.msg_iov = rcv2;
+	rcv_msg[1].msg_hdr.msg_iovlen = 1;
 
 	test_info();
 }
@@ -123,4 +112,13 @@ static struct tst_test test = {
 	.setup = setup,
 	.cleanup = cleanup,
 	.test_variants = TEST_VARIANTS,
+	.bufs = (struct tst_buffers []) {
+		{&snd1, .iov_sizes = (int[]){3, 3, -1}},
+		{&snd2, .iov_sizes = (int[]){6, -1}},
+		{&rcv1, .iov_sizes = (int[]){6, -1}},
+		{&rcv2, .iov_sizes = (int[]){5, -1}},
+		{&snd_msg, .size = VLEN * sizeof(*snd_msg)},
+		{&rcv_msg, .size = VLEN * sizeof(*rcv_msg)},
+		{},
+	}
 };
-- 
2.21.0

[LTP] [PATCH v2 00/11] Introduce guarded buffers

[Li Wang]

Hi Cyril,

I helped to compile & run the whole patch v2 on some platforms[1], the
test result looks pretty good.

Just a tiny issue was found in test_guarded_buf.c, since the .bufs
does not ending with a {}, so sometimes it gets segmental fault from:
        $ cat tst_buffers.c  -n
        115 for (i = 0; bufs[i].ptr; i++) {

And I will keep going to review the patchset in the next few days.
Will let you know if I can find more issues.

[1] (x86_64 + kernel-5.3.0-rc3), (aarch64 + kernel-4.18.0),
(rassberryPi + 4.14.78)

-- 
Regards,
Li Wang

[LTP] [PATCH v2 00/11] Introduce guarded buffers

[Cyril Hrubis]

Hi!
> I helped to compile & run the whole patch v2 on some platforms[1], the
> test result looks pretty good.
> 
> Just a tiny issue was found in test_guarded_buf.c, since the .bufs
> does not ending with a {}, so sometimes it gets segmental fault from:
>         $ cat tst_buffers.c  -n
>         115 for (i = 0; bufs[i].ptr; i++) {

Good catch, I fixed that locally in my patchset for now.

> And I will keep going to review the patchset in the next few days.
> Will let you know if I can find more issues.

Thanks a lot!

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 01/11] lib: Add support for guarded buffers

[Richard Palethorpe]

Hi,

Cyril Hrubis <chrubis@suse.cz> writes:

> +
> +/*
> + * Buffer description consist of a pointer to a pointer and buffer type/size
> + * encoded as a different structure members.
> + *
> + * Only one of the size and iov_sizes can be set at a time.
> + */
> +struct tst_buffers {
> +	/*
> +	 * This pointer points to a buffer pointer.
> +	 */
> +	void *ptr;
> +	/*
> +	 * Buffer size.
> +	 */
> +	size_t size;
> +	/*
> +	 * Array of iov buffer sizes terminated by -1.
> +	 */
> +	int *iov_sizes;
> +};
> +
> +/*
> + * Allocates buffers based on the tst_buffers structure.
> + *
> + * @bufs NULL terminated array of test buffer descriptions.
> + *
> + * This is called from the test library if the tst_test->bufs pointer is set.
> + */
> +void tst_buffers_alloc(struct tst_buffers bufs[]);
> +
> +/*
> + * strdup() that callls tst_alloc().
> + */
> +char *tst_strdup(const char *str);
> +
> +/*
> + * Allocates size bytes, returns pointer to the allocated buffer.
> + */
> +void *tst_alloc(size_t size);
> +
> +/*
> + * Allocates iovec structure including the buffers.
> + *
> + * @sizes -1 terminated array of buffer sizes.
> + */
> +struct iovec *tst_iovec_alloc(int sizes[]);
> +
> +/*
> + * Frees all allocated buffers.
> + *
> + * This is called at the end of the test automatically.
> + */
> +void tst_free_all(void);

We could add guarded buffers to huge numbers of tests by wrapping the
user supplied buffer in various calls to SAFE_* macros and tst_*
functions. This could be configurable at compile time and it should be
detectable if a buffer is already guarded, so double wrapping should not
be an issue.

However I am not sure the current API makes this easy. We should
probably have a tst_free(void *buf) and/or tst_buffer_alloc(struct
tst_buffer *buf) and tst_buffer_free(struct tst_buffer *buf) (which
could just put the buffer on a free list for reuse).

I am not sure if this is something which needs to be addressed now or
can be left for another patch set. My main concern is that one of the
existing API functions will need to be changed.

--
Thank you,
Richard.

[LTP] [PATCH v2 03/11] doc: Add guarded buffers documentation

[Richard Palethorpe]

Hi,

Just nitpicking...

Cyril Hrubis <chrubis@suse.cz> writes:

> Signed-off-by: Cyril Hrubis <chrubis@suse.cz>
> ---
>  doc/test-writing-guidelines.txt | 68 +++++++++++++++++++++++++++++++++
>  1 file changed, 68 insertions(+)
>
> diff --git a/doc/test-writing-guidelines.txt b/doc/test-writing-guidelines.txt
> index 573dd08d9..e5ee2fef0 100644
> --- a/doc/test-writing-guidelines.txt
> +++ b/doc/test-writing-guidelines.txt
> @@ -1699,6 +1699,74 @@ struct tst_test test = {
>  };
>  -------------------------------------------------------------------------------
>  
> +2.2.31 Guarded buffers
> +^^^^^^^^^^^^^^^^^^^^^^
> +
> +The test library also supports guarded buffers, which are buffers allocated so
> +that:

Don't need 'also'

> +
> +* The end of the buffer is followed by PROT_NONE page
                                         ^ a
> +
> +* The rest of the page before the buffer is filled with random canary
         ^ remainder                                                     ^data

> +
> +Which means that the any access after the buffer with yield Segmentation
                                                    ^ will    ^ a
> +fault or EFAULT depending on if the access happened in userspace or kernel
                                                                      ^the
> +respectively. The canary before the buffer will also catch any write access
> +outside of the buffer.
> +
> +The purpose of the patch is to catch off-by-one bugs happening while buffers
                                                      ^ which happen
                                                      when buffers ...

> +and structures are passed to syscalls. New tests should allocate guarded
> +buffers for all data passed to the tested syscall which are passed by a
> +pointer.
> +
> +[source,c]
> +-------------------------------------------------------------------------------
> +#include "tst_test.h"
> +
> +static struct foo *foo_ptr;
> +static struct iovec *iov;
> +static void *buf_ptr;
> +static char *id;
> +...
> +
> +static void run(void)
> +{
> +	...
> +
> +	foo_ptr->bar = 1;
> +	foo_ptr->buf = buf_ptr;
> +
> +	...
> +}
> +
> +static void setup(void)
> +{
> +	...
> +
> +	id = tst_strdup(string);
> +
> +	...
> +}
> +
> +static struct tst_test test = {
> +	...
> +	.bufs = (struct tst_buffers []) {
> +		{&foo_ptr, .size = sizeof(*foo_ptr)},
> +		{&buf_ptr, .size = BUF_SIZE},
> +		{&iov, .iov_sizes = (int[]){128, 32, -1},
> +		{}
> +	}
> +};
> +-------------------------------------------------------------------------------
> +
> +Guarded buffers can be allocated on runtime in a test setup() by a
                                    ^ at                        ^ function
> +'tst_alloc()' or by 'tst_strdup()' as well as by filling up the .bufs array in
^ call to

> +the tst_test structure.
> +
> +So far the tst_test structure supports allocating either a plain buffer by
> +setting up the size or struct iovec, which is allocated recursively including
> +the individual buffers as described by an '-1' terminated array of buffer
> +sizes.
>  
>  2.3 Writing a testcase in shell
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> -- 
> 2.21.0


-- 
Thank you,
Richard.

[LTP] [PATCH v2 01/11] lib: Add support for guarded buffers

[Cyril Hrubis]

Hi!
> We could add guarded buffers to huge numbers of tests by wrapping the
> user supplied buffer in various calls to SAFE_* macros and tst_*
> functions. This could be configurable at compile time and it should be
> detectable if a buffer is already guarded, so double wrapping should not
> be an issue.

Fair point. The detection would however be O(n), well we can do a little
trick like maintaing the memory range in which all the mmaps were done
and rule out any heap based allocation in O(1) easily, but anything that
crosses the malloc threshold would be O(n).

> However I am not sure the current API makes this easy. We should
> probably have a tst_free(void *buf) and/or tst_buffer_alloc(struct
> tst_buffer *buf) and tst_buffer_free(struct tst_buffer *buf) (which
> could just put the buffer on a free list for reuse).

I guess that this calls for completely different API since 99% time we
will do with just single buffer.

Unless we are:

* The test is compiled with pthreads
  and we managed to run two SAFE_MACROS() concurently

* The buffer is bigger than one page

* We call SAFE_MACROS() recursively, which we don't

So all of this could be done by a tst_temp_alloc() and tst_temp_free()
that would attempt to grab single pre-allocated buffer and only fall
back to allocating/freeing new buffer when the buffer is currently in
use.

> I am not sure if this is something which needs to be addressed now or
> can be left for another patch set. My main concern is that one of the
> existing API functions will need to be changed.

I guess that it would be easier to make these changes incrementally,
because adding more functionality to this patchset would only make it
harder to review and I would like to get this API in so that we can
star making use of it.

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 01/11] lib: Add support for guarded buffers

[Richard Palethorpe]

Hello,

Cyril Hrubis <chrubis@suse.cz> writes:

> Hi!
>> We could add guarded buffers to huge numbers of tests by wrapping the
>> user supplied buffer in various calls to SAFE_* macros and tst_*
>> functions. This could be configurable at compile time and it should be
>> detectable if a buffer is already guarded, so double wrapping should not
>> be an issue.
>
> Fair point. The detection would however be O(n), well we can do a little
> trick like maintaing the memory range in which all the mmaps were done
> and rule out any heap based allocation in O(1) easily, but anything that
> crosses the malloc threshold would be O(n).
>
>> However I am not sure the current API makes this easy. We should
>> probably have a tst_free(void *buf) and/or tst_buffer_alloc(struct
>> tst_buffer *buf) and tst_buffer_free(struct tst_buffer *buf) (which
>> could just put the buffer on a free list for reuse).
>
> I guess that this calls for completely different API since 99% time we
> will do with just single buffer.
>
> Unless we are:
>
> * The test is compiled with pthreads
>   and we managed to run two SAFE_MACROS() concurently
>
> * The buffer is bigger than one page
>
> * We call SAFE_MACROS() recursively, which we don't
>
> So all of this could be done by a tst_temp_alloc() and tst_temp_free()
> that would attempt to grab single pre-allocated buffer and only fall
> back to allocating/freeing new buffer when the buffer is currently in
> use.
>
>> I am not sure if this is something which needs to be addressed now or
>> can be left for another patch set. My main concern is that one of the
>> existing API functions will need to be changed.
>
> I guess that it would be easier to make these changes incrementally,
> because adding more functionality to this patchset would only make it
> harder to review and I would like to get this API in so that we can
> star making use of it.

OK, I am happy with this patchset except for the few minor comments on
the docs.

-- 
Thank you,
Richard.

[LTP] [PATCH v2 00/11] Introduce guarded buffers

[Cyril Hrubis]

Hi!
> I helped to compile & run the whole patch v2 on some platforms[1], the
> test result looks pretty good.
> 
> Just a tiny issue was found in test_guarded_buf.c, since the .bufs
> does not ending with a {}, so sometimes it gets segmental fault from:
>         $ cat tst_buffers.c  -n
>         115 for (i = 0; bufs[i].ptr; i++) {
> 
> And I will keep going to review the patchset in the next few days.
> Will let you know if I can find more issues.

Have you found anything else or can I push the patchset?

-- 
Cyril Hrubis
chrubis@suse.cz

[LTP] [PATCH v2 00/11] Introduce guarded buffers

[Li Wang]

> > And I will keep going to review the patchset in the next few days.
> > Will let you know if I can find more issues.
>
> Have you found anything else or can I push the patchset?

Sure, it's ok to push them. Actually, I have read the patchset many
times, it looks pretty good to me.

-- 
Regards,
Li Wang

[LTP] [PATCH v2 01/11] lib: Add support for guarded buffers

[Cyril Hrubis]

Hi!
> OK, I am happy with this patchset except for the few minor comments on
> the docs.

I've fixed the docs and one bug pointed out by Li Wang and pushed the
patchset.

-- 
Cyril Hrubis
chrubis@suse.cz