mmap abuse in ehca, was Re: [PATCH] ehca: fix do_mmap() error check

[Christoph Hellwig]

> Index: 2.6-mm/drivers/infiniband/hw/ehca/ehca_uverbs.c
> ===================================================================
> --- 2.6-mm.orig/drivers/infiniband/hw/ehca/ehca_uverbs.c
> +++ 2.6-mm/drivers/infiniband/hw/ehca/ehca_uverbs.c
> @@ -321,14 +321,14 @@ int ehca_mmap_nopage(u64 foffset, u64 le
>                    struct vm_area_struct **vma)
>  {
>       down_write(&current->mm->mmap_sem);
> -     *mapped = (void*)do_mmap(NULL,0, length, PROT_WRITE,
> +     *mapped = (void*)do_mmap(NULL, 0, length, PROT_WRITE,
>                                MAP_SHARED | MAP_ANONYMOUS,
>                                foffset);
>       up_write(&current->mm->mmap_sem);
> -     if (!(*mapped)) {
> +     if (IS_ERR(*mapped)) {
>               ehca_gen_err("couldn't mmap foffset=%lx length=%lx",
>                            foffset, length);
> -             return -EINVAL;
> +             return PTR_ERR(*mmaped);

Folks, could you explain what this code is actually trying to do.
Just to quote it in a little more detail the code looks like this:

int ehca_mmap_nopage(u64 foffset, u64 length, void **mapped,
                     struct vm_area_struct **vma)
{
        down_write(&current->mm->mmap_sem);
        *mapped = (void*)do_mmap(NULL,0, length, PROT_WRITE,
                                 MAP_SHARED | MAP_ANONYMOUS,
                                 foffset);
        up_write(&current->mm->mmap_sem);
        
        if (!(*mapped)) {
                ehca_gen_err("couldn't mmap foffset=%lx length=%lx",
                             foffset, length);
                return -EINVAL;
        }
G
        *vma = find_vma(current->mm, (u64)*mapped);
        if (!(*vma)) {
                down_write(&current->mm->mmap_sem);
                do_munmap(current->mm, 0, length);
                up_write(&current->mm->mmap_sem);
                ehca_gen_err("couldn't find vma queue=%p", *mapped);
                return -EINVAL;
        }

        (*vma)->vm_flags |= VM_RESERVED;
        (*vma)->vm_ops = &ehcau_vm_ops;

        return 0;
}

the worst caller then continues as:

int ehca_mmap_register(u64 physical, void **mapped,
                       struct vm_area_struct **vma)
{
        int ret;
        unsigned long vsize;
        /* ehca hw supports only 4k page */
        ret = ehca_mmap_nopage(0, EHCA_PAGESIZE, mapped, vma);

        (*vma)->vm_flags |= VM_RESERVED;
        vsize = (*vma)->vm_end - (*vma)->vm_start;
        if (vsize != EHCA_PAGESIZE) {
                ehca_gen_err("invalid vsize=%lx",
                             (*vma)->vm_end - (*vma)->vm_start);
                return -EINVAL;
        }

        (*vma)->vm_page_prot = pgprot_noncached((*vma)->vm_page_prot);
        (*vma)->vm_flags |= VM_IO | VM_RESERVED;

        ret = remap_pfn_range((*vma), (*vma)->vm_start,
                              physical >> PAGE_SHIFT,
                              vsize,
                              (*vma)->vm_page_prot);

Aside from the very questionably naming of ehca_mmap_nopage this maps
anonymous shared memory into a random location in the calling process
from something that is not defined to change the callers address space,
then racy looks up the vma for it and sets the VM_RESERVED flags and
vm ops on this anonoymous vma.  Not enough ehca_mmap_register then does
a remap_pfn_range into that anonymous vma.  This is definitly not
how the mmap infrastructure should be used.

I'd go as far as saying do_mmap(_pgoff) should not be exported at all,
but we'd need to fix similar braindamage in drm first.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/