From a6b0d3e8fd860f9e9c8a6da10be8d3a6dd811449 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Mon, 3 Jun 2019 14:47:32 -0700 Subject: [PATCH 01/19] Fixed the bug that causes unmap_mapping_range to not perform unmapping --- gdrdrv/gdrdrv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index ce83d10f..d732915c 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -223,7 +223,7 @@ static void gdrdrv_zap_vma(struct address_space *mapping, struct vm_area_struct up_write(&mm->mmap_sem); mmput(mm); #else - unmap_mapping_range(mapping, vma->vm_start, vma->vm_end - vma->vm_start, 0); + unmap_mapping_range(mapping, vma->vm_pgoff << PAGE_SHIFT, vma->vm_end - vma->vm_start, 0); #endif } From d394e2a3cba620650b8ed4422995f4cc9b04d678 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Wed, 17 Jul 2019 16:42:37 -0700 Subject: [PATCH 02/19] Fixed the bug allowing the child process the shared gdr_map with the parent process by way of fork --- gdrdrv/gdrdrv.c | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index d732915c..6eaca414 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -293,7 +293,7 @@ static int gdrdrv_release(struct inode *inode, struct file *filp) mutex_lock(&info->lock); list_for_each_safe(p, n, &info->mr_list) { mr = list_entry(p, gdr_mr_t, node); - gdr_info("freeing MR=%px\n", mr); + gdr_info("freeing MR=0x%p\n", mr); if (gdr_mr_is_mapped(mr)) { mutex_unlock(&info->lock); gdr_mr_destroy_all_mappings(mr); @@ -329,6 +329,7 @@ static gdr_mr_t *gdr_mr_from_handle_unlocked(gdr_info_t *info, gdr_hnd_t handle) list_for_each(p, &info->mr_list) { mr = list_entry(p, gdr_mr_t, node); + gdr_dbg("mr->handle=0x%x handle=0x%x\n", mr->handle, handle); if (handle == mr->handle) break; } @@ -534,7 +535,7 @@ static int gdrdrv_unpin_buffer(gdr_info_t *info, void __user *_params) ret = -EINVAL; } else { if (gdr_mr_is_mapped(mr)) { - gdr_err("trying to unpin mapped mr %px\n", mr); + gdr_err("trying to unpin mapped mr 0x%p\n", mr); ret = -EBUSY; } else { list_del(&mr->node); @@ -679,7 +680,7 @@ static long gdrdrv_unlocked_ioctl(struct file *filp, unsigned int cmd, unsigned void gdrdrv_vma_close(struct vm_area_struct *vma) { gdr_mr_t *mr = (gdr_mr_t *)vma->vm_private_data; - gdr_dbg("closing vma=%px vm_file=%px vm_private_data=%px mr=%px mr->vma=%px\n", vma, vma->vm_file, vma->vm_private_data, mr, mr->vma); + gdr_dbg("closing vma=0x%p vm_file=0x%p vm_private_data=0x%p mr=0x%p mr->vma=0x%p\n", vma, vma->vm_file, vma->vm_private_data, mr, mr->vma); // TODO: handle multiple vma's mr->vma = NULL; mr->cpu_mapping_type = GDR_MR_NONE; @@ -717,6 +718,7 @@ static int gdrdrv_remap_gpu_mem(struct vm_area_struct *vma, unsigned long vaddr, goto out; } pfn = paddr >> PAGE_SHIFT; + vma->vm_flags |= VM_DONTCOPY; #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,9) vma->vm_pgoff = pfn; vma->vm_flags |= VM_RESERVED; @@ -758,7 +760,13 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) int p = 0; unsigned long vaddr; - gdr_info("mmap vma=%px start=0x%lx size=%zu off=0x%lx\n", vma, vma->vm_start, size, vma->vm_pgoff); + gdr_info("mmap filp=0x%p vma=0x%p vm_file=0x%p start=0x%lx size=%zu off=0x%lx\n", filp, vma, vma->vm_file, vma->vm_start, size, vma->vm_pgoff); + + if (info == NULL) { + gdr_dbg("gdr_info is not found\n"); + ret = -EINVAL; + goto out; + } handle = gdrdrv_handle_from_off(vma->vm_pgoff); mr = gdr_mr_from_handle(info, handle); @@ -811,7 +819,7 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) // this also works as the mapped flag for this mr mr->cpu_mapping_type = GDR_MR_CACHING; vma->vm_ops = &gdrdrv_vm_ops; - gdr_dbg("overwriting vma->vm_private_data=%px with mr=%px\n", vma->vm_private_data, mr); + gdr_dbg("overwriting vma->vm_private_data=0x%p with mr=0x%p\n", vma->vm_private_data, mr); vma->vm_private_data = mr; p = 0; vaddr = vma->vm_start; @@ -872,14 +880,16 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) out: if (ret) { - mr->vma = NULL; - mr->mapping = NULL; - mr->cpu_mapping_type = GDR_MR_NONE; - // TODO: tear down stale partial mappings + if (mr) { + mr->vma = NULL; + mr->mapping = NULL; + mr->cpu_mapping_type = GDR_MR_NONE; + // TODO: tear down stale partial mappings + } } else { mr->vma = vma; mr->mapping = filp->f_mapping; - gdr_dbg("mr vma=%px mapping=%px\n", mr->vma, mr->mapping); + gdr_dbg("mr vma=0x%p mapping=0x%p\n", mr->vma, mr->mapping); } return ret; } From e82a4c5103ba8f40bf7d84128adf63e92b2802da Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Wed, 17 Jul 2019 16:45:35 -0700 Subject: [PATCH 03/19] Prevented sharing gdrcopy fd between processes --- gdrapi.c | 2 +- gdrdrv/gdrdrv.c | 24 ++++++++++++++++++++---- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/gdrapi.c b/gdrapi.c index ae4cf35a..fa71c2a9 100644 --- a/gdrapi.c +++ b/gdrapi.c @@ -134,7 +134,7 @@ gdr_t gdr_open() return NULL; } - int fd = open(gdrinode, O_RDWR); + int fd = open(gdrinode, O_RDWR | O_CLOEXEC); if (-1 == fd ) { int ret = errno; gdr_err("error opening driver (errno=%d/%s)\n", ret, strerror(ret)); diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 6eaca414..2cf0fef5 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -36,6 +36,7 @@ #include #include #include +#include #ifndef random_get_entropy #define random_get_entropy() get_cycles() @@ -240,6 +241,7 @@ struct gdr_info { // simple low-performance linked-list implementation struct list_head mr_list; struct mutex lock; + struct pid *pid; }; typedef struct gdr_info gdr_info_t; @@ -273,6 +275,8 @@ static int gdrdrv_open(struct inode *inode, struct file *filp) INIT_LIST_HEAD(&info->mr_list); mutex_init(&info->lock); + info->pid = task_pid(current); + filp->private_data = info; out: @@ -642,6 +646,15 @@ static int gdrdrv_ioctl(struct inode *inode, struct file *filp, unsigned int cmd return -EINVAL; } + if (!info) { + gdr_err("filp contains no info\n"); + return -EIO; + } + if (info->pid != task_pid(current)) { + gdr_err("filp is not opened by the current process\n"); + return -EACCES; + } + switch (cmd) { case GDRDRV_IOC_PIN_BUFFER: ret = gdrdrv_pin_buffer(info, argp); @@ -762,10 +775,13 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) gdr_info("mmap filp=0x%p vma=0x%p vm_file=0x%p start=0x%lx size=%zu off=0x%lx\n", filp, vma, vma->vm_file, vma->vm_start, size, vma->vm_pgoff); - if (info == NULL) { - gdr_dbg("gdr_info is not found\n"); - ret = -EINVAL; - goto out; + if (!info) { + gdr_err("filp contains no info\n"); + return -EIO; + } + if (info->pid != task_pid(current)) { + gdr_err("filp is not opened by the current process\n"); + return -EACCES; } handle = gdrdrv_handle_from_off(vma->vm_pgoff); From 835cc12c63f72db269ce14f69d7eda0b3051c2cf Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Wed, 17 Jul 2019 16:50:43 -0700 Subject: [PATCH 04/19] Separated mapping address space of Process A from Process B --- gdrdrv/gdrdrv.c | 45 ++++++++++++++++++++++----------------------- gdrdrv/gdrdrv.h | 1 - 2 files changed, 22 insertions(+), 24 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 2cf0fef5..46b39886 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -38,10 +38,6 @@ #include #include -#ifndef random_get_entropy -#define random_get_entropy() get_cycles() -#endif - #if defined(CONFIG_X86_64) || defined(CONFIG_X86_32) static inline pgprot_t pgprot_modify_writecombine(pgprot_t old_prot) { @@ -211,21 +207,9 @@ static int gdr_mr_is_wc_mapping(gdr_mr_t *mr ) return (mr->cpu_mapping_type == GDR_MR_WC ) ? 1 : 0; } -static void gdrdrv_zap_vma(struct address_space *mapping, struct vm_area_struct *vma) +static inline void gdrdrv_zap_vma(struct address_space *mapping, struct vm_area_struct *vma) { - // BUG: this needs to be called from the same process, i.e. it - // does not store the original mm. -#if 0 - struct mm_struct *mm; - mm = get_task_mm(current); - down_write(&mm->mmap_sem); - // this API is not exported to modules - zap_page_range(vma, vma->vm_start, vma->vm_end - vma->vm_start); - up_write(&mm->mmap_sem); - mmput(mm); -#else unmap_mapping_range(mapping, vma->vm_pgoff << PAGE_SHIFT, vma->vm_end - vma->vm_start, 0); -#endif } static void gdr_mr_destroy_all_mappings(gdr_mr_t *mr) @@ -242,6 +226,8 @@ struct gdr_info { struct list_head mr_list; struct mutex lock; struct pid *pid; + struct address_space mapping; + off_t next_offset; }; typedef struct gdr_info gdr_info_t; @@ -258,14 +244,14 @@ static int gdrdrv_open(struct inode *inode, struct file *filp) int ret = 0; gdr_info_t *info = NULL; - gdr_dbg("minor=%d filep=%p\n", minor, filp); + gdr_dbg("minor=%d filep=0x%p\n", minor, filp); if(minor >= 1) { gdr_err("device minor number too big!\n"); ret = -ENXIO; goto out; } - info = kmalloc(sizeof(gdr_info_t), GFP_KERNEL); + info = kzalloc(sizeof(gdr_info_t), GFP_KERNEL); if (!info) { gdr_err("can't alloc kernel memory\n"); ret = -ENOMEM; @@ -277,6 +263,13 @@ static int gdrdrv_open(struct inode *inode, struct file *filp) info->pid = task_pid(current); + // Create mapping per opened file. By preventing sharing of this file, the + // mapping is local to the process. + address_space_init_once(&info->mapping); + info->mapping.host = filp->f_mapping->host; + info->mapping.a_ops = filp->f_mapping->a_ops; + filp->f_mapping = &info->mapping; + filp->private_data = info; out: @@ -430,7 +423,6 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) mr->cpu_mapping_type = GDR_MR_NONE; mr->page_table = NULL; mr->cb_flag = 0; - mr->handle = random_get_entropy() & GDR_HANDLE_MASK; // this is a hack, we need something really unique and randomized gdr_info("invoking nvidia_p2p_get_pages(va=0x%llx len=%lld p2p_tok=%llx va_tok=%x)\n", mr->va, mr->mapped_size, mr->p2p_token, mr->va_space); @@ -484,16 +476,23 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) } } - // here a typical driver would use the page_table to fill in some HW // DMA data structure - params.handle = mr->handle; - mutex_lock(&info->lock); list_add(&mr->node, &info->mr_list); + + // The user treats handle as the offset value for mmap. To guarantee no + // overlapping, the next offset will sit next to this mapping space. + // Remember that the address space is local to the process. Thus, two + // processes do mmap with the same offset get different mappings. + mr->handle = info->next_offset; + info->next_offset += mr->mapped_size; + mutex_unlock(&info->lock); + params.handle = mr->handle; + out: if (ret && mr && mr->page_table) { diff --git a/gdrdrv/gdrdrv.h b/gdrdrv/gdrdrv.h index 5585b645..24b66b97 100644 --- a/gdrdrv/gdrdrv.h +++ b/gdrdrv/gdrdrv.h @@ -26,7 +26,6 @@ #define GDRDRV_IOCTL 0xDA typedef __u32 gdr_hnd_t; -#define GDR_HANDLE_MASK ((1UL<<32)-1) //----------- From 490a10903baec411529e349c6aa091f1f9ce80f6 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Wed, 5 Jun 2019 11:20:33 -0700 Subject: [PATCH 05/19] Added comments to gdrdrv.c --- gdrdrv/gdrdrv.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 46b39886..e022a261 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -197,18 +197,21 @@ struct gdr_mr { }; typedef struct gdr_mr gdr_mr_t; -static int gdr_mr_is_mapped(gdr_mr_t *mr ) +static int gdr_mr_is_mapped(gdr_mr_t *mr) { return mr->cpu_mapping_type != GDR_MR_NONE; } -static int gdr_mr_is_wc_mapping(gdr_mr_t *mr ) +static int gdr_mr_is_wc_mapping(gdr_mr_t *mr) { - return (mr->cpu_mapping_type == GDR_MR_WC ) ? 1 : 0; + return (mr->cpu_mapping_type == GDR_MR_WC) ? 1 : 0; } static inline void gdrdrv_zap_vma(struct address_space *mapping, struct vm_area_struct *vma) { + // This function is mainly used for files and the address is relative to + // the file offset. We use vma->pg_off here to unmap this entire range but + // not the other mapped ranges. unmap_mapping_range(mapping, vma->vm_pgoff << PAGE_SHIFT, vma->vm_end - vma->vm_start, 0); } @@ -223,11 +226,22 @@ static void gdr_mr_destroy_all_mappings(gdr_mr_t *mr) struct gdr_info { // simple low-performance linked-list implementation - struct list_head mr_list; - struct mutex lock; - struct pid *pid; - struct address_space mapping; - off_t next_offset; + struct list_head mr_list; + struct mutex lock; + + // Pointer to the pid struct of the creator process. We do not use + // numerical pid here to avoid issues from pid reuse. + struct pid *pid; + + // Address space uniqued to this opened file. We need to create a new one + // because filp->f_mapping usually points to inode->i_mapping. + struct address_space mapping; + + // The handle number of mmap's offset are equivalent. However, the mmap + // offset is used by the linux kernel when doing m(un)map; hence the range + // cannot be overlapped. We place two ranges next two each other to avoid + // this issue. + off_t next_offset; }; typedef struct gdr_info gdr_info_t; @@ -265,9 +279,12 @@ static int gdrdrv_open(struct inode *inode, struct file *filp) // Create mapping per opened file. By preventing sharing of this file, the // mapping is local to the process. + + // TODO: Old kernels don't have address_space_init_once address_space_init_once(&info->mapping); info->mapping.host = filp->f_mapping->host; info->mapping.a_ops = filp->f_mapping->a_ops; + // TODO: Old kernels have f_mapping->backing_dev_info filp->f_mapping = &info->mapping; filp->private_data = info; @@ -483,7 +500,7 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) list_add(&mr->node, &info->mr_list); // The user treats handle as the offset value for mmap. To guarantee no - // overlapping, the next offset will sit next to this mapping space. + // overlapping, the next offset will sit next to this mapping range. // Remember that the address space is local to the process. Thus, two // processes do mmap with the same offset get different mappings. mr->handle = info->next_offset; @@ -649,6 +666,7 @@ static int gdrdrv_ioctl(struct inode *inode, struct file *filp, unsigned int cmd gdr_err("filp contains no info\n"); return -EIO; } + // Check that the caller is the same process that did gdrdrv_open if (info->pid != task_pid(current)) { gdr_err("filp is not opened by the current process\n"); return -EACCES; @@ -730,7 +748,10 @@ static int gdrdrv_remap_gpu_mem(struct vm_area_struct *vma, unsigned long vaddr, goto out; } pfn = paddr >> PAGE_SHIFT; + + // Disallow mmapped VMA to propagate to child processes vma->vm_flags |= VM_DONTCOPY; + #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,9) vma->vm_pgoff = pfn; vma->vm_flags |= VM_RESERVED; @@ -778,6 +799,7 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) gdr_err("filp contains no info\n"); return -EIO; } + // Check that the caller is the same process that did gdrdrv_open if (info->pid != task_pid(current)) { gdr_err("filp is not opened by the current process\n"); return -EACCES; From 094ea6ba8187c89b15a65caa820c2f6fb5eab4ec Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Thu, 13 Jun 2019 15:34:59 -0700 Subject: [PATCH 06/19] Changed %p to %px in gdrdrv --- gdrdrv/gdrdrv.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index e022a261..e6930d66 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -258,7 +258,7 @@ static int gdrdrv_open(struct inode *inode, struct file *filp) int ret = 0; gdr_info_t *info = NULL; - gdr_dbg("minor=%d filep=0x%p\n", minor, filp); + gdr_dbg("minor=%d filep=0x%px\n", minor, filp); if(minor >= 1) { gdr_err("device minor number too big!\n"); ret = -ENXIO; @@ -307,7 +307,7 @@ static int gdrdrv_release(struct inode *inode, struct file *filp) mutex_lock(&info->lock); list_for_each_safe(p, n, &info->mr_list) { mr = list_entry(p, gdr_mr_t, node); - gdr_info("freeing MR=0x%p\n", mr); + gdr_info("freeing MR=0x%px\n", mr); if (gdr_mr_is_mapped(mr)) { mutex_unlock(&info->lock); gdr_mr_destroy_all_mappings(mr); @@ -405,7 +405,7 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) cycles_t ta, tb; if (copy_from_user(¶ms, _params, sizeof(params))) { - gdr_err("copy_from_user failed on user pointer %p\n", _params); + gdr_err("copy_from_user failed on user pointer 0x%px\n", _params); ret = -EFAULT; goto out; } @@ -526,7 +526,7 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) } if (!ret && copy_to_user(_params, ¶ms, sizeof(params))) { - gdr_err("copy_to_user failed on user pointer %p\n", _params); + gdr_err("copy_to_user failed on user pointer 0x%px\n", _params); ret = -EFAULT; } @@ -542,7 +542,7 @@ static int gdrdrv_unpin_buffer(gdr_info_t *info, void __user *_params) gdr_mr_t *mr = NULL; if (copy_from_user(¶ms, _params, sizeof(params))) { - gdr_err("copy_from_user failed on user pointer %p\n", _params); + gdr_err("copy_from_user failed on user pointer 0x%px\n", _params); return -EFAULT; } @@ -555,7 +555,7 @@ static int gdrdrv_unpin_buffer(gdr_info_t *info, void __user *_params) ret = -EINVAL; } else { if (gdr_mr_is_mapped(mr)) { - gdr_err("trying to unpin mapped mr 0x%p\n", mr); + gdr_err("trying to unpin mapped mr 0x%px\n", mr); ret = -EBUSY; } else { list_del(&mr->node); @@ -591,7 +591,7 @@ static int gdrdrv_get_cb_flag(gdr_info_t *info, void __user *_params) gdr_mr_t *mr = NULL; if (copy_from_user(¶ms, _params, sizeof(params))) { - gdr_err("copy_from_user failed on user pointer %p\n", _params); + gdr_err("copy_from_user failed on user pointer 0x%px\n", _params); return -EFAULT; } mr = gdr_mr_from_handle(info, params.handle); @@ -604,7 +604,7 @@ static int gdrdrv_get_cb_flag(gdr_info_t *info, void __user *_params) params.flag = !!mr->cb_flag; if (copy_to_user(_params, ¶ms, sizeof(params))) { - gdr_err("copy_to_user failed on user pointer %p\n", _params); + gdr_err("copy_to_user failed on user pointer 0x%px\n", _params); ret = -EFAULT; } out: @@ -620,7 +620,7 @@ static int gdrdrv_get_info(gdr_info_t *info, void __user *_params) gdr_mr_t *mr = NULL; if (copy_from_user(¶ms, _params, sizeof(params))) { - gdr_err("copy_from_user failed on user pointer %p\n", _params); + gdr_err("copy_from_user failed on user pointer 0x%px\n", _params); ret = -EFAULT; goto out; } @@ -640,7 +640,7 @@ static int gdrdrv_get_info(gdr_info_t *info, void __user *_params) params.mapped = gdr_mr_is_mapped(mr); params.wc_mapping = gdr_mr_is_wc_mapping(mr); if (copy_to_user(_params, ¶ms, sizeof(params))) { - gdr_err("copy_to_user failed on user pointer %p\n", _params); + gdr_err("copy_to_user failed on user pointer 0x%px\n", _params); ret = -EFAULT; } out: @@ -710,7 +710,7 @@ static long gdrdrv_unlocked_ioctl(struct file *filp, unsigned int cmd, unsigned void gdrdrv_vma_close(struct vm_area_struct *vma) { gdr_mr_t *mr = (gdr_mr_t *)vma->vm_private_data; - gdr_dbg("closing vma=0x%p vm_file=0x%p vm_private_data=0x%p mr=0x%p mr->vma=0x%p\n", vma, vma->vm_file, vma->vm_private_data, mr, mr->vma); + gdr_dbg("closing vma=0x%px vm_file=0x%px vm_private_data=0x%px mr=0x%px mr->vma=0x%px\n", vma, vma->vm_file, vma->vm_private_data, mr, mr->vma); // TODO: handle multiple vma's mr->vma = NULL; mr->cpu_mapping_type = GDR_MR_NONE; @@ -793,7 +793,7 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) int p = 0; unsigned long vaddr; - gdr_info("mmap filp=0x%p vma=0x%p vm_file=0x%p start=0x%lx size=%zu off=0x%lx\n", filp, vma, vma->vm_file, vma->vm_start, size, vma->vm_pgoff); + gdr_info("mmap filp=0x%px vma=0x%px vm_file=0x%px start=0x%lx size=%zu off=0x%lx\n", filp, vma, vma->vm_file, vma->vm_start, size, vma->vm_pgoff); if (!info) { gdr_err("filp contains no info\n"); @@ -856,7 +856,7 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) // this also works as the mapped flag for this mr mr->cpu_mapping_type = GDR_MR_CACHING; vma->vm_ops = &gdrdrv_vm_ops; - gdr_dbg("overwriting vma->vm_private_data=0x%p with mr=0x%p\n", vma->vm_private_data, mr); + gdr_dbg("overwriting vma->vm_private_data=0x%px with mr=0x%px\n", vma->vm_private_data, mr); vma->vm_private_data = mr; p = 0; vaddr = vma->vm_start; @@ -926,7 +926,7 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) } else { mr->vma = vma; mr->mapping = filp->f_mapping; - gdr_dbg("mr vma=0x%p mapping=0x%p\n", mr->vma, mr->mapping); + gdr_dbg("mr vma=0x%px mapping=0x%px\n", mr->vma, mr->mapping); } return ret; } From 2d95c64e2e9947b4a255d39f602570d514bd66c9 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Thu, 13 Jun 2019 15:42:49 -0700 Subject: [PATCH 07/19] Added pid checking to gdrdrv_release --- gdrdrv/gdrdrv.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index e6930d66..0ba71a8d 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -304,6 +304,16 @@ static int gdrdrv_release(struct inode *inode, struct file *filp) gdr_dbg("closing\n"); + if (!info) { + gdr_err("filp contains no info\n"); + return -EIO; + } + // Check that the caller is the same process that did gdrdrv_open + if (info->pid != task_pid(current)) { + gdr_err("filp is not opened by the current process\n"); + return -EACCES; + } + mutex_lock(&info->lock); list_for_each_safe(p, n, &info->mr_list) { mr = list_entry(p, gdr_mr_t, node); From e452d9c215b278eef98d306f185e9681dc0948d4 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Thu, 13 Jun 2019 16:49:17 -0700 Subject: [PATCH 08/19] Created gdr_generate_mr_handle and fixed the wrap-around bug --- gdrdrv/gdrdrv.c | 58 +++++++++++++++++++++++++++++++++++++------------ gdrdrv/gdrdrv.h | 2 +- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 0ba71a8d..eb360308 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -241,7 +241,7 @@ struct gdr_info { // offset is used by the linux kernel when doing m(un)map; hence the range // cannot be overlapped. We place two ranges next two each other to avoid // this issue. - off_t next_offset; + gdr_hnd_t next_handle; }; typedef struct gdr_info gdr_info_t; @@ -353,7 +353,7 @@ static gdr_mr_t *gdr_mr_from_handle_unlocked(gdr_info_t *info, gdr_hnd_t handle) list_for_each(p, &info->mr_list) { mr = list_entry(p, gdr_mr_t, node); - gdr_dbg("mr->handle=0x%x handle=0x%x\n", mr->handle, handle); + gdr_dbg("mr->handle=0x%llx handle=0x%llx\n", mr->handle, handle); if (handle == mr->handle) break; } @@ -403,6 +403,39 @@ static void gdrdrv_get_pages_free_callback(void *data) //----------------------------------------------------------------------------- +/** + * Generate mr->handle. This function should be called under info->lock. + * + * Prerequisite: + * - mr->mapped_size is set and round to max(PAGE_SIZE, GPU_PAGE_SIZE) + * + * Return 0 if success, -1 if failed. + */ +static inline int gdr_generate_mr_handle(gdr_info_t *info, gdr_mr_t *mr) +{ + // The user-space library passes the memory (handle << PAGE_SHIFT) as the + // mmap offset, and offsets are used to determine the VMAs to delete during + // invalidation. + // Hence, we need [(handle << PAGE_SHIFT), (handle << PAGE_SHIFT) + size - 1] + // to correspond to a unique VMA. Note that size here must match the + // original mmap size + + gdr_hnd_t next_handle = info->next_handle + (mr->mapped_size >> PAGE_SHIFT); + + // The end of this range is wrapped around. The calculation uses PAGE_SHIFT + // because handle is equal vm_pgoff (aka file offset << PAGE_SHIFT). We + // indicate this case as -ENOMEM error. + if ((next_handle & ((gdr_hnd_t)(-1) >> PAGE_SHIFT)) < info->next_handle) + return -1; + + mr->handle = info->next_handle; + info->next_handle = next_handle; + + return 0; +} + +//----------------------------------------------------------------------------- + static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) { struct GDRDRV_IOC_PIN_BUFFER_PARAMS params = {0}; @@ -473,7 +506,7 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) goto out; } - switch(page_table->page_size) { + switch (page_table->page_size) { case NVIDIA_P2P_PAGE_SIZE_4KB: mr->page_size = 4*1024; break; @@ -490,7 +523,7 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) } // we are not really ready for a different page size - if(page_table->page_size != NVIDIA_P2P_PAGE_SIZE_64KB) { + if (page_table->page_size != NVIDIA_P2P_PAGE_SIZE_64KB) { gdr_err("nvidia_p2p_get_pages assumption of 64KB pages failed size_id=%d\n", page_table->page_size); ret = -EINVAL; goto out; @@ -509,13 +542,10 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) mutex_lock(&info->lock); list_add(&mr->node, &info->mr_list); - // The user treats handle as the offset value for mmap. To guarantee no - // overlapping, the next offset will sit next to this mapping range. - // Remember that the address space is local to the process. Thus, two - // processes do mmap with the same offset get different mappings. - mr->handle = info->next_offset; - info->next_offset += mr->mapped_size; - + if (gdr_generate_mr_handle(info, mr) != 0) { + gdr_err("No address space left for future BAR1 mapping.\n"); + ret = -ENOMEM; + } mutex_unlock(&info->lock); params.handle = mr->handle; @@ -561,7 +591,7 @@ static int gdrdrv_unpin_buffer(gdr_info_t *info, void __user *_params) mutex_lock(&info->lock); mr = gdr_mr_from_handle_unlocked(info, params.handle); if (NULL == mr) { - gdr_err("unexpected handle %x while unmapping buffer\n", params.handle); + gdr_err("unexpected handle %llx while unmapping buffer\n", params.handle); ret = -EINVAL; } else { if (gdr_mr_is_mapped(mr)) { @@ -606,7 +636,7 @@ static int gdrdrv_get_cb_flag(gdr_info_t *info, void __user *_params) } mr = gdr_mr_from_handle(info, params.handle); if (NULL == mr) { - gdr_err("unexpected handle %x in get_cb_flag\n", params.handle); + gdr_err("unexpected handle %llx in get_cb_flag\n", params.handle); ret = -EINVAL; goto out; } @@ -637,7 +667,7 @@ static int gdrdrv_get_info(gdr_info_t *info, void __user *_params) mr = gdr_mr_from_handle(info, params.handle); if (NULL == mr) { - gdr_err("unexpected handle %x in get_cb_flag\n", params.handle); + gdr_err("unexpected handle %llx in get_cb_flag\n", params.handle); ret = -EINVAL; goto out; } diff --git a/gdrdrv/gdrdrv.h b/gdrdrv/gdrdrv.h index 24b66b97..48649d2f 100644 --- a/gdrdrv/gdrdrv.h +++ b/gdrdrv/gdrdrv.h @@ -25,7 +25,7 @@ #define GDRDRV_IOCTL 0xDA -typedef __u32 gdr_hnd_t; +typedef __u64 gdr_hnd_t; //----------- From df021398f21ec6757c4345a260160134ad12a4f4 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Thu, 13 Jun 2019 17:40:34 -0700 Subject: [PATCH 09/19] Handled the mismatch of the address_space struct in different Linux kernel versions --- gdrdrv/gdrdrv.c | 39 +++++++++++++++++++++++++++++++++++---- 1 file changed, 35 insertions(+), 4 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index eb360308..90dc89aa 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -38,6 +38,34 @@ #include #include +#if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,32) +/** + * This API is available after Linux kernel 2.6.42 + */ +void address_space_init_once(struct address_space *mapping) +{ + memset(mapping, 0, sizeof(*mapping)); + INIT_RADIX_TREE(&mapping->page_tree, GFP_ATOMIC); + +#if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,26) + // + // The .tree_lock member variable was changed from type rwlock_t, to + // spinlock_t, on 25 July 2008, by mainline commit + // 19fd6231279be3c3bdd02ed99f9b0eb195978064. + // + rwlock_init(&mapping->tree_lock); +#else + spin_lock_init(&mapping->tree_lock); +#endif + + spin_lock_init(&mapping->i_mmap_lock); + INIT_LIST_HEAD(&mapping->private_list); + spin_lock_init(&mapping->private_lock); + INIT_RAW_PRIO_TREE_ROOT(&mapping->i_mmap); + INIT_LIST_HEAD(&mapping->i_mmap_nonlinear); +} +#endif + #if defined(CONFIG_X86_64) || defined(CONFIG_X86_32) static inline pgprot_t pgprot_modify_writecombine(pgprot_t old_prot) { @@ -280,11 +308,12 @@ static int gdrdrv_open(struct inode *inode, struct file *filp) // Create mapping per opened file. By preventing sharing of this file, the // mapping is local to the process. - // TODO: Old kernels don't have address_space_init_once address_space_init_once(&info->mapping); - info->mapping.host = filp->f_mapping->host; - info->mapping.a_ops = filp->f_mapping->a_ops; - // TODO: Old kernels have f_mapping->backing_dev_info + info->mapping.host = inode; + info->mapping.a_ops = inode->i_mapping->a_ops; +#if LINUX_VERSION_CODE < KERNEL_VERSION(4,0,0) + info->mapping.backing_dev_info = inode->i_mapping->backing_dev_info; +#endif filp->f_mapping = &info->mapping; filp->private_data = info; @@ -338,6 +367,8 @@ static int gdrdrv_release(struct inode *inode, struct file *filp) } mutex_unlock(&info->lock); + filp->f_mapping = NULL; + kfree(info); filp->private_data = NULL; From 34daeb29add07b3fa7818500f56473ecc116d45b Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Thu, 13 Jun 2019 17:46:20 -0700 Subject: [PATCH 10/19] Removed unused #include from gdrdrv/gdrdrv.c --- gdrdrv/gdrdrv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 90dc89aa..547663d3 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -35,7 +35,6 @@ #include #include #include -#include #include #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,32) From d1a266c296bf320d7226c9898071e7e3ae0e82c6 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Fri, 14 Jun 2019 12:40:58 -0700 Subject: [PATCH 11/19] Fixed the bug that makes gdrdrv_pin_buffer eagerly fail when the assigned handle is starting to wrap around --- gdrdrv/gdrdrv.c | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 547663d3..84b07200 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -269,6 +269,7 @@ struct gdr_info { // cannot be overlapped. We place two ranges next two each other to avoid // this issue. gdr_hnd_t next_handle; + int next_handle_overflow; }; typedef struct gdr_info gdr_info_t; @@ -450,13 +451,17 @@ static inline int gdr_generate_mr_handle(gdr_info_t *info, gdr_mr_t *mr) // to correspond to a unique VMA. Note that size here must match the // original mmap size - gdr_hnd_t next_handle = info->next_handle + (mr->mapped_size >> PAGE_SHIFT); + gdr_hnd_t next_handle; - // The end of this range is wrapped around. The calculation uses PAGE_SHIFT - // because handle is equal vm_pgoff (aka file offset << PAGE_SHIFT). We - // indicate this case as -ENOMEM error. - if ((next_handle & ((gdr_hnd_t)(-1) >> PAGE_SHIFT)) < info->next_handle) + // We run out of handle, so fail. + if (unlikely(info->next_handle_overflow)) return -1; + + next_handle = info->next_handle + (mr->mapped_size >> PAGE_SHIFT); + + // The next handle will be overflowed, so we mark it. + if (unlikely((next_handle & ((gdr_hnd_t)(-1) >> PAGE_SHIFT)) < info->next_handle)) + info->next_handle_overflow = 1; mr->handle = info->next_handle; info->next_handle = next_handle; @@ -570,12 +575,12 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) // DMA data structure mutex_lock(&info->lock); - list_add(&mr->node, &info->mr_list); - if (gdr_generate_mr_handle(info, mr) != 0) { - gdr_err("No address space left for future BAR1 mapping.\n"); + gdr_err("No address space left for BAR1 mapping.\n"); ret = -ENOMEM; } + + list_add(&mr->node, &info->mr_list); mutex_unlock(&info->lock); params.handle = mr->handle; From 58575d57cfea60e741aa86010b081b52f7fd4132 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Fri, 14 Jun 2019 12:45:49 -0700 Subject: [PATCH 12/19] Fixed the bug that adds mr to info->mr_list when creating the mr fail --- gdrdrv/gdrdrv.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 84b07200..a1c1851e 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -580,7 +580,8 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) ret = -ENOMEM; } - list_add(&mr->node, &info->mr_list); + if (!ret) + list_add(&mr->node, &info->mr_list); mutex_unlock(&info->lock); params.handle = mr->handle; From a25c4fe17dd079d095f03b64a84de39417a2deb2 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Tue, 18 Jun 2019 13:44:30 -0700 Subject: [PATCH 13/19] Fixed comments and checked for info->lock in gdrdrv.c --- gdrdrv/gdrdrv.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index a1c1851e..3d009798 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -39,7 +39,7 @@ #if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,32) /** - * This API is available after Linux kernel 2.6.42 + * This API is available after Linux kernel 2.6.32 */ void address_space_init_once(struct address_space *mapping) { @@ -264,7 +264,7 @@ struct gdr_info { // because filp->f_mapping usually points to inode->i_mapping. struct address_space mapping; - // The handle number of mmap's offset are equivalent. However, the mmap + // The handle number and mmap's offset are equivalent. However, the mmap // offset is used by the linux kernel when doing m(un)map; hence the range // cannot be overlapped. We place two ranges next two each other to avoid // this issue. @@ -303,11 +303,11 @@ static int gdrdrv_open(struct inode *inode, struct file *filp) INIT_LIST_HEAD(&info->mr_list); mutex_init(&info->lock); + // GPU driver does not support sharing GPU allocations at fork time. Hence + // here we track the process owning the driver fd and prevent other process + // to use it. info->pid = task_pid(current); - // Create mapping per opened file. By preventing sharing of this file, the - // mapping is local to the process. - address_space_init_once(&info->mapping); info->mapping.host = inode; info->mapping.a_ops = inode->i_mapping->a_ops; @@ -453,6 +453,8 @@ static inline int gdr_generate_mr_handle(gdr_info_t *info, gdr_mr_t *mr) gdr_hnd_t next_handle; + WARN_ON(!mutex_is_locked(&info->lock)); + // We run out of handle, so fail. if (unlikely(info->next_handle_overflow)) return -1; From b93ad9cb9bf05498290c2692b3422ef0ca0471f7 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Wed, 17 Jul 2019 16:53:28 -0700 Subject: [PATCH 14/19] gdr_close() now automatically cleans up resources --- gdrapi.c | 27 +++++++++++++++++++++++++-- gdrdrv/gdrdrv.c | 6 ++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/gdrapi.c b/gdrapi.c index fa71c2a9..63fee845 100644 --- a/gdrapi.c +++ b/gdrapi.c @@ -39,6 +39,7 @@ #include #include #include +#include #include "gdrapi.h" #include "gdrdrv.h" @@ -100,8 +101,9 @@ static void gdr_msg(enum gdrcopy_msg_level lvl, const char* fmt, ...) #define gdr_warn(FMT, ARGS...) gdr_msg(GDRCOPY_MSG_WARN, "WARN: " FMT, ## ARGS) #define gdr_err(FMT, ARGS...) gdr_msg(GDRCOPY_MSG_ERROR, "ERR: " FMT, ## ARGS) -typedef struct { +typedef struct gdr_memh_t { gdr_hnd_t handle; + LIST_ENTRY(gdr_memh_t) entries; unsigned mapped:1; unsigned wc_mapping:1; } gdr_memh_t; @@ -121,6 +123,7 @@ static gdr_mh_t from_memh(gdr_memh_t *memh) { struct gdr { int fd; + LIST_HEAD(memh_list, gdr_memh_t) memhs; }; gdr_t gdr_open() @@ -143,6 +146,7 @@ gdr_t gdr_open() } g->fd = fd; + LIST_INIT(&g->memhs); return g; } @@ -150,7 +154,23 @@ gdr_t gdr_open() int gdr_close(gdr_t g) { int ret = 0; - int retcode = close(g->fd); + int retcode; + gdr_memh_t *mh, *next_mh; + + mh = g->memhs.lh_first; + while (mh != NULL) { + // gdr_unpin_buffer frees mh, so we need to get the next one + // beforehand. + next_mh = mh->entries.le_next; + ret = gdr_unpin_buffer(g, from_memh(mh)); + if (ret) { + gdr_err("error unpinning buffer inside gdr_close (errno=%d/%s)\n", ret, strerror(ret)); + return ret; + } + mh = next_mh; + } + + retcode = close(g->fd); if (-1 == retcode) { ret = errno; gdr_err("error closing driver (errno=%d/%s)\n", ret, strerror(ret)); @@ -188,6 +208,7 @@ int gdr_pin_buffer(gdr_t g, unsigned long addr, size_t size, uint64_t p2p_token, goto err; } mh->handle = params.handle; + LIST_INSERT_HEAD(&g->memhs, mh, entries); *handle = from_memh(mh); err: return ret; @@ -206,6 +227,8 @@ int gdr_unpin_buffer(gdr_t g, gdr_mh_t handle) ret = errno; gdr_err("ioctl error (errno=%d)\n", ret); } + LIST_REMOVE(mh, entries); + free(mh); return ret; } diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 3d009798..10f306f4 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -633,11 +633,9 @@ static int gdrdrv_unpin_buffer(gdr_info_t *info, void __user *_params) ret = -EINVAL; } else { if (gdr_mr_is_mapped(mr)) { - gdr_err("trying to unpin mapped mr 0x%px\n", mr); - ret = -EBUSY; - } else { - list_del(&mr->node); + gdr_mr_destroy_all_mappings(mr); } + list_del(&mr->node); } mutex_unlock(&info->lock); if (ret) From 11079f5ea08c62e090b63e178008818fc5089913 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Tue, 18 Jun 2019 16:56:20 -0700 Subject: [PATCH 15/19] Removed deprecated gds_close comment --- gdrapi.h | 4 ---- 1 file changed, 4 deletions(-) diff --git a/gdrapi.h b/gdrapi.h index 218c3949..73c83784 100644 --- a/gdrapi.h +++ b/gdrapi.h @@ -62,10 +62,6 @@ gdr_t gdr_open(); // Destroy library state object, e.g. it closes the connection to kernel-mode // driver. -// -// Note that altough BAR mappings of GPU memory are destroyed, user-space -// mappings are not. So therefore user code is responsible of calling -// gdr_unmap on all mappings before calling gdr_close. int gdr_close(gdr_t g); typedef struct gdr_mh_s { From 00b947e4f210b11c350bff96f0a738d9434ea17f Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Mon, 15 Jul 2019 11:11:15 -0700 Subject: [PATCH 16/19] Dropped support for Linux kernel version <= 2.6.9, and fixed a typo --- gdrdrv/gdrdrv.c | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 10f306f4..2380def2 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -260,7 +260,7 @@ struct gdr_info { // numerical pid here to avoid issues from pid reuse. struct pid *pid; - // Address space uniqued to this opened file. We need to create a new one + // Address space unique to this opened file. We need to create a new one // because filp->f_mapping usually points to inode->i_mapping. struct address_space mapping; @@ -828,16 +828,6 @@ static int gdrdrv_remap_gpu_mem(struct vm_area_struct *vma, unsigned long vaddr, // Disallow mmapped VMA to propagate to child processes vma->vm_flags |= VM_DONTCOPY; -#if LINUX_VERSION_CODE <= KERNEL_VERSION(2,6,9) - vma->vm_pgoff = pfn; - vma->vm_flags |= VM_RESERVED; - vma->vm_flags |= VM_IO; - if (remap_page_range(vma, vaddr, paddr, size, vma->vm_page_prot)) { - gdr_err("error in remap_page_range()\n"); - ret = -EAGAIN; - goto out; - } -#else if (is_wcomb) { // override prot to create non-coherent WC mappings vma->vm_page_prot = pgprot_modify_writecombine(vma->vm_page_prot); @@ -849,7 +839,6 @@ static int gdrdrv_remap_gpu_mem(struct vm_area_struct *vma, unsigned long vaddr, ret = -EAGAIN; goto out; } -#endif out: return ret; From 91241886ade16e06338d14c84d764624296e1ba0 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Wed, 17 Jul 2019 16:54:54 -0700 Subject: [PATCH 17/19] Fixed the indentation issue in gdrapi.h --- gdrapi.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gdrapi.h b/gdrapi.h index 73c83784..f4084dab 100644 --- a/gdrapi.h +++ b/gdrapi.h @@ -65,7 +65,7 @@ gdr_t gdr_open(); int gdr_close(gdr_t g); typedef struct gdr_mh_s { - unsigned long h; + unsigned long h; } gdr_mh_t; // Map device memory buffer on GPU BAR1, returning an handle. From d91d76b8f058dd0ce3062a03c29436c71faa3f7a Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Mon, 15 Jul 2019 13:37:41 -0700 Subject: [PATCH 18/19] Removed "// TODO: tear down stale partial mappings" because partial mappings are destroyed by Linux kernel immediately after our mmap function returns --- gdrdrv/gdrdrv.c | 1 - 1 file changed, 1 deletion(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 2380def2..716f98ad 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -986,7 +986,6 @@ static int gdrdrv_mmap(struct file *filp, struct vm_area_struct *vma) mr->vma = NULL; mr->mapping = NULL; mr->cpu_mapping_type = GDR_MR_NONE; - // TODO: tear down stale partial mappings } } else { mr->vma = vma; From 3786215b536f028d9e7c37f3fea1987e0539a966 Mon Sep 17 00:00:00 2001 From: Pak Markthub Date: Thu, 18 Jul 2019 16:47:49 -0700 Subject: [PATCH 19/19] Changed child -> children and checked ret before assigning params.handle = mr->handle in gdrdrv/gdrdrv.c --- gdrdrv/gdrdrv.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/gdrdrv/gdrdrv.c b/gdrdrv/gdrdrv.c index 716f98ad..17bbbf0a 100644 --- a/gdrdrv/gdrdrv.c +++ b/gdrdrv/gdrdrv.c @@ -540,6 +540,7 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) // check version before accessing page table if (!NVIDIA_P2P_PAGE_TABLE_VERSION_COMPATIBLE(page_table)) { gdr_err("incompatible page table version 0x%08x\n", page_table->version); + ret = -EFAULT; goto out; } @@ -586,8 +587,6 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) list_add(&mr->node, &info->mr_list); mutex_unlock(&info->lock); - params.handle = mr->handle; - out: if (ret && mr && mr->page_table) { @@ -603,11 +602,16 @@ static int gdrdrv_pin_buffer(gdr_info_t *info, void __user *_params) mr = NULL; } - if (!ret && copy_to_user(_params, ¶ms, sizeof(params))) { - gdr_err("copy_to_user failed on user pointer 0x%px\n", _params); - ret = -EFAULT; + if (!ret) { + params.handle = mr->handle; + + if (copy_to_user(_params, ¶ms, sizeof(params))) { + gdr_err("copy_to_user failed on user pointer 0x%px\n", _params); + ret = -EFAULT; + } } + return ret; } @@ -825,7 +829,7 @@ static int gdrdrv_remap_gpu_mem(struct vm_area_struct *vma, unsigned long vaddr, } pfn = paddr >> PAGE_SHIFT; - // Disallow mmapped VMA to propagate to child processes + // Disallow mmapped VMA to propagate to children processes vma->vm_flags |= VM_DONTCOPY; if (is_wcomb) {