[PATCH 2/2] userfaultfd: Add ioctl tests

Dr. David Alan Gilbert dgilbert at redhat.com
Tue May 10 10:16:03 UTC 2016


* Dmitry V. Levin (ldv at altlinux.org) wrote:
> 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.

Done.

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

Done.

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

Done.

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

Done.

> > +
> > +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);

OK, done.  I'm not sure I understand the logic behind the tail_alloc(1)'s -
I've placed them at the start of each group.

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

Done.

> > +	/* 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.

Oops, yes; changed those to:
        void *area1 = mmap(NULL, pagesize, PROT_READ|PROT_WRITE,
                           MAP_PRIVATE|MAP_ANONYMOUS,
                           -1, 0);
        if (area1 == MAP_FAILED)
                perror_msg_and_fail("mmap area1");
        void *area2 = mmap(NULL, pagesize, PROT_READ|PROT_WRITE,
                           MAP_PRIVATE|MAP_ANONYMOUS,
                           -1, 0);
        if (area2 == MAP_FAILED)
                perror_msg_and_fail("mmap area2");

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

Done.

Thanks,

Dave

> 
> 
> -- 
> ldv



> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications Manager
> Applications Manager provides deep performance insights into multiple tiers of
> your business applications. It resolves application problems quickly and
> reduces your MTTR. Get your free trial!
> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z

> _______________________________________________
> Strace-devel mailing list
> Strace-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel

--
Dr. David Alan Gilbert / dgilbert at redhat.com / Manchester, UK




More information about the Strace-devel mailing list