Re: [PATCH igt 02/10] igt/gem_mmap_gtt: Simulate gdb inspecting a GTT mmap using ptrace()

Chris Wilson
2017-07-31T10:18:08Z

Quoting Tvrtko Ursulin (2017-07-31 10:41:06)
>
> On 28/07/2017 13:08, Chris Wilson wrote:
> > gdb uses ptrace() to peek and poke bytes of the target's address space.
> > The kernel must implement an vm_ops->access() handler or else gdb will
> > be unable to inspect the pointer and report it as out-of-bounds. Worse
> > than useless as it causes immediate suspicion of the valid GTT pointer.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > tests/gem_mmap_gtt.c | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 79 insertions(+)
> >
> > diff --git a/tests/gem_mmap_gtt.c b/tests/gem_mmap_gtt.c
> > index 4ff5e7f1..61c08406 100644
> > --- a/tests/gem_mmap_gtt.c
> > +++ b/tests/gem_mmap_gtt.c
> > @@ -36,6 +36,8 @@
> > #include <errno.h>
> > #include <sys/stat.h>
> > #include <sys/ioctl.h>
> > +#include <sys/ptrace.h>
> > +#include <sys/wait.h>
> > #include "drm.h"
> >
> > #include "igt.h"
> > @@ -310,6 +312,81 @@ test_write_gtt(int fd)
> > munmap(src, OBJECT_SIZE);
> > }
> >
> > +static void *memchr_inv(const void *s, int c, size_t n)
> > +{
> > + const uint8_t *us = s;
> > + const uint8_t uc = c;
> > +
> > +#pragma GCC diagnostic push
> > +#pragma GCC diagnostic ignored "-Wcast-qual"
> > + while (n--) {
> > + if (*us != uc)
> > + return (void *) us;
> > + us++;
> > + }
> > +#pragma GCC diagnostic pop
>
> Stripping away constness wouldn't work with a simpler:

Or just saying const void *memchr_inv().

In this case we don't need memchr_inv, as just a bool will do,
so memchr_eq().

>
> uint8_t *us = (uint8_t *)s?
>
> > +
> > + return NULL;
> > +}
> > +
> > +static void
> > +test_ptrace(int fd)
> > +{
> > + long AA, CC;
> > + long *gtt, *copy;
> > + uint32_t bo;
> > + pid_t pid;
> > +
> > + memset(&AA, 0xaa, sizeof(AA));
> > + memset(&CC, 0x55, sizeof(CC));
> > +
> > + copy = malloc(OBJECT_SIZE);
> > + memset(copy, AA, OBJECT_SIZE);
> > +
> > + bo = gem_create(fd, OBJECT_SIZE);
> > + gtt = mmap_bo(fd, bo);
> > + memset(gtt, CC, OBJECT_SIZE);
> > + gem_close(fd, bo);
> > +
> > + igt_assert(!memchr_inv(gtt, CC, OBJECT_SIZE));
> > + igt_assert(!memchr_inv(copy, AA, OBJECT_SIZE));
> > +
> > + switch ((pid = fork())) {
> > + case -1:
> > + igt_assert(pid != -1);
> > + break;
> > +
> > + case 0:
> > + ptrace(PTRACE_TRACEME, 0, NULL, NULL);
> > + raise(SIGSTOP);
> > + raise(SIGKILL);
> > + exit(0);
> > + break;
> > +
> > + default:
> > + /* Wait for the child to ready themselves */
> > + wait(NULL);
> > +
> > + ptrace(PTRACE_ATTACH, pid, NULL, NULL);
> > + for (int i = 0; i < OBJECT_SIZE/sizeof(long); i++) {
> > + copy[i] = ptrace(PTRACE_PEEKDATA, pid, gtt+i, NULL);
> > + ptrace(PTRACE_POKEDATA, pid, gtt + i, AA);
>
> Inconsistent whitespace in the above three lines. First and second need
> a tidy.

? The whitespace carries meaning for me.