[PATCH 2/2] userfaultfd: Add ioctl tests

[Dmitry V. Levin]

On Fri, May 06, 2016 at 12:08:41PM +0100, Dr. David Alan Gilbert (git) wrote:
> From: "Dr. David Alan Gilbert" <dgilbert at redhat.com>
> 
> * tests/userfaultfd.c: Add test
> * tests/userfaultfd.test: Call test

First of all, thanks for writing tests for your new UFFDIO_* parser,
I hope other contributors will follow your example.

I suggest keeping the test separately from userfaultfd.test, mostly for
technical reasons: all ioctl tests that use exact match method need ioctl
specific workaround (see tests/ioctl.test) and making them all like
ioctl_v4l2.test is just the simplest way to go.

So just create a new test called e.g. ioctl_uffdio.

>  tests/userfaultfd.c    | 179 ++++++++++++++++++++++++++++++++++++++++++++++++-
>  tests/userfaultfd.test |   2 +-
>  2 files changed, 177 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/userfaultfd.c b/tests/userfaultfd.c
> index 5747a2a..9410fe4 100644
> --- a/tests/userfaultfd.c
> +++ b/tests/userfaultfd.c
> @@ -26,7 +26,11 @@
>   */
>  
>  #include "tests.h"
> +#include <assert.h>
>  #include <fcntl.h>
> +#include <stdint.h>
> +#include <inttypes.h>
> +#include <string.h>
>  #include <sys/syscall.h>

Let's include all new headers after #ifdef.

>  
>  #if defined __NR_userfaultfd && defined O_CLOEXEC

In the ioctl test, there is no need neither to check nor to use O_CLOEXEC,
you can simply write

#if defined __NR_userfaultfd && defined HAVE_LINUX_USERFAULTFD_H
...

#else
SKIP_MAIN_UNDEFINED("__NR_userfaultfd && HAVE_LINUX_USERFAULTFD_H")
#endif

>  # include <stdio.h>
>  # include <unistd.h>
>  
> +#ifdef HAVE_LINUX_USERFAULTFD_H
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <linux/ioctl.h>
> +#include <linux/userfaultfd.h>
> +#endif

We indent preprocessor directives.

> +
> +void
> +ioctl_test(int fd)
> +{
> +#ifdef HAVE_LINUX_USERFAULTFD_H
> +	int rc;
> +	size_t pagesize = getpagesize();
> +
> +	/* ---- API ---- */
> +	struct uffdio_api api_struct;
> +	/* With a bad fd */
> +	memset(&api_struct, 0, sizeof(api_struct));
> +	rc = ioctl(-1, UFFDIO_API, &api_struct);

We have a function called tail_alloc: it allocates memory that ends on the
page boundary; pages allocated by tail_alloc are preceded by an unmapped
page and followed by another unmapped page.

This method is used in many tests to check that strace parsers do not read
beyond the end of structure they are supposed to parse, see e.g.
tests/ioctl_v4l2.c.

For example,
	(void) tail_alloc(1); /* initial memory hole just in case */
	struct uffdio_api *api_struct = tail_alloc(sizeof(*api_struct));
	memset(api_struct, 0, sizeof(*api_struct));
	rc = ioctl(-1, UFFDIO_API, api_struct);

> +	printf("ioctl(-1, UFFDIO_API, {api=0, features=0, "
> +		"features.out=0, ioctls=0"
> +		"}) = %d %s (%m)\n", rc, errno2name());

We usually align wrapped arguments under the first argument.

> +	/* With a bad pointer */
> +	rc = ioctl(fd, UFFDIO_API, NULL);
> +	printf("ioctl(%d, UFFDIO_API, NULL) = %d %s (%m)\n",
> +		fd, rc, errno2name());
> +	/* Normal call */
> +	api_struct.api = UFFD_API;
> +	api_struct.features = 0;
> +	rc = ioctl(fd, UFFDIO_API, &api_struct);
> +	printf("ioctl(%d, UFFDIO_API, {api=0xaa, features=0, "
> +		"features.out=%#" PRIx64 ", " "ioctls=1<<_UFFDIO_REGISTER|"
> +		"1<<_UFFDIO_UNREGISTER|1<<_UFFDIO_API",
> +		fd, (uint64_t)api_struct.features);
> +	api_struct.ioctls &= ~(1ull<<_UFFDIO_REGISTER|
> +				1ull<<_UFFDIO_UNREGISTER|
> +				1ull<<_UFFDIO_API);
> +	if (api_struct.ioctls)
> +		printf("|%#" PRIx64, (uint64_t)api_struct.ioctls);
> +	printf("}) = %d\n", rc);
> +
> +	/* For the rest of the tests we need some anonymous memory */
> +	void *area1 = mmap(NULL, pagesize, PROT_READ|PROT_WRITE,
> +				MAP_PRIVATE|MAP_ANONYMOUS,
> +				-1, 0);
> +	assert(area1);
> +	void *area2 = mmap(NULL, pagesize, PROT_READ|PROT_WRITE,
> +				MAP_PRIVATE|MAP_ANONYMOUS,
> +				-1, 0);
> +	assert(area2);

I'm not sure what do you mean by using these asserts.
Checking mmap return code is usually done using MAP_FAILED.

[...]
>  int
>  main(void)
>  {
> -	long rc = syscall(__NR_userfaultfd, 1 | O_NONBLOCK | O_CLOEXEC);
> -	printf("userfaultfd(O_NONBLOCK|O_CLOEXEC|0x1) = %ld %s (%m)\n",
> -	       rc, errno2name());
> +	int fd = syscall(__NR_userfaultfd, O_NONBLOCK | O_CLOEXEC);
> +	printf("userfaultfd(O_NONBLOCK|O_CLOEXEC) = %d\n",
> +	       fd);
> +	if (fd != -1)
> +		ioctl_test(fd);

If this is a separate test, you can simplify it:

	int fd = syscall(__NR_userfaultfd, 0);
	if (fd < 0)
		perror_msg_and_skip("userfaultfd");

	[contents of ioctl_test]


-- 
ldv
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.strace.io/pipermail/strace-devel/attachments/20160509/eacad7a6/attachment.bin>