[igt-dev] [i-g-t, v2] igt/gem_mmap_offset: Adding subtest oob_read

[Matthew Auld]

On Fri, 11 Mar 2022 at 07:02, Mastan Katragadda
<mastanx.katragadda at intel.com> wrote:
>
> This test will Detect A missing bounds check in vm_access().
> can lead to an out-of-bounds read or write in the adjacent memory area.
>
> Changes Since V1:
>         - Incorporated minor changes as suggested in feedback [Chris/Matthew].
>         - Fixed build failures.
>
> Signed-off-by: Mastan Katragadda <mastanx.katragadda at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Matthew Auld <matthew.auld at intel.com>
> ---
>  tests/i915/gem_mmap_offset.c | 44 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 44 insertions(+)
>
> diff --git a/tests/i915/gem_mmap_offset.c b/tests/i915/gem_mmap_offset.c
> index 8148f0a2..c8fab18b 100644
> --- a/tests/i915/gem_mmap_offset.c
> +++ b/tests/i915/gem_mmap_offset.c
> @@ -22,6 +22,8 @@
>   */
>
>  #include <errno.h>
> +#include <unistd.h>
> +#include <fcntl.h>

Nit: These should be kept sorted.

>  #include <pthread.h>
>  #include <signal.h>
>  #include <stdatomic.h>
> @@ -289,6 +291,44 @@ static void *memchr_inv(const void *s, int c, size_t n)
>         return NULL;
>  }
>
> +static void
> +test_oob_read(int i915)
> +{
> +       unsigned char read_buf[4096];
> +       uint32_t handle;
> +       uintptr_t addr;
> +       int memfd;
> +       int ret;
> +
> +       handle = gem_create(i915, 4096);
> +
> +       for_each_mmap_offset_type(i915, t) {
> +               uint32_t *ptr;
> +
> +               ptr = __mmap_offset(i915, handle, 0, 4096,
> +                                   PROT_READ | PROT_WRITE,
> +                                   t->type);
> +               if (!ptr)
> +                       continue;
> +
> +               memfd = open("/proc/self/mem", O_RDWR);
> +               igt_require_f(memfd != -1, "/proc/self/mem\n");
> +
> +               addr = (uintptr_t)ptr + 4092;
> +               ret = lseek(memfd, addr, SEEK_SET);
> +               igt_assert_f(ret != -1, "lseek failed\n");
> +
> +               /* Triggering the buf (out of bound read) */
> +               ret = read(memfd, read_buf, 8);
> +               igt_assert(ret == -1 && errno ==  EIO);
> +
> +               munmap(ptr, 4096);
> +               close(memfd);
> +       }
> +
> +       gem_close(i915, handle);
> +}
> +
>  static void test_ptrace(int i915)
>  {
>         const unsigned int SZ = 3 * 4096;
> @@ -692,6 +732,10 @@ igt_main
>         igt_subtest_f("pf-nonblock")
>                 pf_nonblock(i915);
>
> +       igt_describe("Check Linear Out-Of-Bound in Vm_access");

Odd capitalisation here. No need to resend just for that, I can fix up
when pushing. Can you just login and hit the retest button here[1],
just to ensure this now passes with the kernel changes merged? Or
alternatively I guess just re-send to trigger a new CI run.

Reviewed-by: Matthew Auld <matthew.auld at intel.com>

[1] https://patchwork.freedesktop.org/series/101187/

> +       igt_subtest("oob-read")
> +               test_oob_read(i915);
> +
>         igt_subtest_with_dynamic("ptrace")
>                 test_ptrace(i915);
>
> --
> 2.25.1
>