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

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

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.

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.

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.

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.