Re: [PATCH 1/2] kunit: test: Add vm_mmap() allocation resource manager

On Sun, May 19, 2024 at 12:12:52PM -0700, Kees Cook wrote:

For tests that need to allocate using vm_mmap() (e.g. usercopy and execve), provide the interface to have the allocation tracked by KUnit itself. This requires bringing up a placeholder userspace mm.

This combines my earlier attempt at this with Mark Rutland's version[1].

Link: https://lore.kernel.org/lkml/20230321122514.1743889-2-mark.rutland@arm.com/ [1] Co-developed-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Mark Rutland mark.rutland@arm.com Signed-off-by: Kees Cook keescook@chromium.org

---

include/kunit/test.h | 17 ++++++ lib/kunit/test.c | 139 ++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 155 insertions(+), 1 deletion(-)

diff --git a/include/kunit/test.h b/include/kunit/test.h index 61637ef32302..8c3835a6f282 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -478,6 +478,23 @@ static inline void *kunit_kcalloc(struct kunit *test, size_t n, size_t size, gfp return kunit_kmalloc_array(test, n, size, gfp | __GFP_ZERO); } +/**

• • kunit_vm_mmap() - Allocate KUnit-tracked vm_mmap() area
• • @test: The test context object.
• • @file: struct file pointer to map from, if any
• • @addr: desired address, if any
• • @len: how many bytes to allocate
• • @prot: mmap PROT_* bits
• • @flag: mmap flags
• • @offset: offset into @file to start mapping from.
• • See vm_mmap() for more information.
• */

+unsigned long kunit_vm_mmap(struct kunit *test, struct file *file,

• 	    unsigned long addr, unsigned long len,
  

• 	    unsigned long prot, unsigned long flag,
  

• 	    unsigned long offset);
  

void kunit_cleanup(struct kunit *test); void __printf(2, 3) kunit_log_append(struct string_stream *log, const char *fmt, ...); diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 1d1475578515..09194dbffb63 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -11,13 +11,14 @@ #include <kunit/test-bug.h> #include <kunit/attributes.h> #include <linux/kernel.h> +#include <linux/kthread.h> +#include <linux/mm.h> #include <linux/module.h> #include <linux/moduleparam.h> #include <linux/mutex.h> #include <linux/panic.h> #include <linux/sched/debug.h> #include <linux/sched.h> -#include <linux/mm.h> #include "debugfs.h" #include "device-impl.h" @@ -871,6 +872,142 @@ void kunit_kfree(struct kunit *test, const void *ptr) } EXPORT_SYMBOL_GPL(kunit_kfree); +struct kunit_vm_mmap_resource {

• unsigned long addr;
• size_t size;

+};

+/* vm_mmap() arguments */ +struct kunit_vm_mmap_params {

• struct file *file;
• unsigned long addr;
• unsigned long len;
• unsigned long prot;
• unsigned long flag;
• unsigned long offset;

+};

+/*

• • Arbitrarily chosen user address for the base allocation.
• */

+#define UBUF_ADDR_BASE SZ_2M

+/* Create and attach a new mm if it doesn't already exist. */ +static int kunit_attach_mm(void) +{

• struct vm_area_struct *vma;
• struct mm_struct *mm;
• if (current->mm)

• return 0;
  

My tests deliberately created/destroyed the mm for each test; surely we don't want to inherit an MM in some arbitrary state? ... or is this just so the mm can be allocated lazily upon the first mmap() within a test?

• mm = mm_alloc();
• if (!mm)

• return -ENOMEM;
  

• if (mmap_write_lock_killable(mm))

• goto out_free;
  

• /* Define the task size. */
• mm->task_size = TASK_SIZE;
• /* Prepare the base VMA. */
• vma = vm_area_alloc(mm);
• if (!vma)

• goto out_unlock;
  

• vma_set_anonymous(vma);
• vma->vm_start = UBUF_ADDR_BASE;
• vma->vm_end = UBUF_ADDR_BASE + PAGE_SIZE;
• vm_flags_init(vma, VM_READ | VM_MAYREAD | VM_WRITE | VM_MAYWRITE);
• vma->vm_page_prot = vm_get_page_prot(vma->vm_flags);
• if (insert_vm_struct(mm, vma))

• goto out_free_vma;
  

• mmap_write_unlock(mm);

Why do we need this VMA given you have kunit_vm_mmap()?

This existed in my uaccess tests because I didn't use vm_mmap(), and I wanted complete control over the addresses used.

Given you add kunit_vm_mmap(), I don't think we want this VMA -- it doesn't serve any real purpose to tests, and accesses can erroneously hit it, which is problematic.

UBUF_ADDR_BASE shouldn't be necessary either with kunit_vm_mmap(), unless you want to use fixed addresses. That was just arbitrarily chosen to be above NULL and the usual minimum mmap limit.

Mark.

• /* Make sure we can allocate new VMAs. */
• arch_pick_mmap_layout(mm, &current->signal->rlim[RLIMIT_STACK]);
• /* Attach the mm. It will be cleaned up when the process dies. */
• kthread_use_mm(mm);
• return 0;

+out_free_vma:

• vm_area_free(vma);

+out_unlock:

• mmap_write_unlock(mm);

+out_free:

• mmput(mm);
• return -ENOMEM;

+}

+static int kunit_vm_mmap_init(struct kunit_resource *res, void *context) +{

• struct kunit_vm_mmap_params *p = context;
• struct kunit_vm_mmap_resource vres;
• int ret;
• ret = kunit_attach_mm();
• if (ret)

• return ret;
  

• vres.size = p->len;
• vres.addr = vm_mmap(p->file, p->addr, p->len, p->prot, p->flag, p->offset);
• if (!vres.addr)

• return -ENOMEM;
  

• res->data = kmemdup(&vres, sizeof(vres), GFP_KERNEL);
• if (!res->data) {

• vm_munmap(vres.addr, vres.size);
  

• return -ENOMEM;
  

• }
• return 0;

+}

+static void kunit_vm_mmap_free(struct kunit_resource *res) +{

• struct kunit_vm_mmap_resource *vres = res->data;
• /*

• * Since this is executed from the test monitoring process,
  

• * the test's mm has already been torn down. We don't need
  

• * to run vm_munmap(vres->addr, vres->size), only clean up
  

• * the vres.
  

• */
  

• kfree(vres);
• res->data = NULL;

+}

+unsigned long kunit_vm_mmap(struct kunit *test, struct file *file,

• 	    unsigned long addr, unsigned long len,
  

• 	    unsigned long prot, unsigned long flag,
  

• 	    unsigned long offset)
  

+{

• struct kunit_vm_mmap_params params = {

• .file = file,
  

• .addr = addr,
  

• .len = len,
  

• .prot = prot,
  

• .flag = flag,
  

• .offset = offset,
  

• };
• struct kunit_vm_mmap_resource *vres;
• vres = kunit_alloc_resource(test,

• 		    kunit_vm_mmap_init,
  

• 		    kunit_vm_mmap_free,
  

• 		    GFP_KERNEL,
  

• 		    &params);
  

• if (vres)

• return vres->addr;
  

• return 0;

+} +EXPORT_SYMBOL_GPL(kunit_vm_mmap);

void kunit_cleanup(struct kunit *test) { struct kunit_resource *res; -- 2.34.1