* Dmitry V. Levin (l...@altlinux.org) wrote:
> On Fri, May 06, 2016 at 12:08:41PM +0100, Dr. David Alan Gilbert (git) wrote:
> > From: "Dr. David Alan Gilbert" <dgilb...@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@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/strace-devel
--
Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK
------------------------------------------------------------------------------
Mobile security can be enabling, not merely restricting. Employees who
bring their own devices (BYOD) to work are irked by the imposition of MDM
restrictions. Mobile Device Manager Plus allows you to control only the
apps on BYO-devices by containerizing them, leaving personal data untouched!
https://ad.doubleclick.net/ddm/clk/304595813;131938128;j
_______________________________________________
Strace-devel mailing list
Strace-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/strace-devel
