On Sun, May 13, 2018 at 09:07:07PM +0300, Michal Kalderon wrote:
> Each user_context receives a separate dpi value and thus a different
> address on the doorbell bar. The qedr_mmap function needs to validate
> the address and map the doorbell bar accordingly.
> The current implementation always checked against dpi=0 doorbell range
> leading to a wrong mapping for doorbell bar. (It entered an else case
> that mapped the address differently). qedr_mmap should only be used
> for doorbells, so the else was actually wrong in the first place.
> This only has an affect on arm architecture and not an issue on a
> x86 based architecture.
> This lead to doorbells not occurring on arm based systems and left
> applications that use more than one dpi (or several applications
> run simultaneously ) to hang.
>
> Fixes: ac1b36e55a51 ("qedr: Add support for user context verbs")
> Signed-off-by: Ariel Elior <Ariel.Elior@xxxxxxxxxx>
> Signed-off-by: Michal Kalderon <Michal.Kalderon@xxxxxxxxxx>
> ---
> drivers/infiniband/hw/qedr/verbs.c | 59 ++++++++++++++++++--------------------
> 1 file changed, 28 insertions(+), 31 deletions(-)
Please don't leak kernel pointers [1], and use %pK instead of %x [2].
[1] https://lwn.net/Articles/735589/
[2] Documentation/core-api/printk-formats.rst
Thanks
>
> diff --git a/drivers/infiniband/hw/qedr/verbs.c b/drivers/infiniband/hw/qedr/verbs.c
> index 7d3763b..27c0f00 100644
> --- a/drivers/infiniband/hw/qedr/verbs.c
> +++ b/drivers/infiniband/hw/qedr/verbs.c
> @@ -401,49 +401,46 @@ int qedr_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
> {
> struct qedr_ucontext *ucontext = get_qedr_ucontext(context);
> struct qedr_dev *dev = get_qedr_dev(context->device);
> - unsigned long vm_page = vma->vm_pgoff << PAGE_SHIFT;
> - u64 unmapped_db = dev->db_phys_addr;
> + unsigned long phys_addr = vma->vm_pgoff << PAGE_SHIFT;
> unsigned long len = (vma->vm_end - vma->vm_start);
> - int rc = 0;
> - bool found;
> + unsigned long dpi_start;
> +
> + dpi_start = dev->db_phys_addr + (ucontext->dpi * ucontext->dpi_size);
>
> DP_DEBUG(dev, QEDR_MSG_INIT,
> - "qedr_mmap called vm_page=0x%lx vm_pgoff=0x%lx unmapped_db=0x%llx db_size=%x, len=%lx\n",
> - vm_page, vma->vm_pgoff, unmapped_db, dev->db_size, len);
> - if (vma->vm_start & (PAGE_SIZE - 1)) {
> - DP_ERR(dev, "Vma_start not page aligned = %ld\n",
> - vma->vm_start);
> + "mmap invoked with vm_start=0x%lx, vm_end=0x%lx,vm_pgoff=0x%lx; dpi_start=0x%lx dpi_size=0x%x\n",
> + vma->vm_start, vma->vm_end, vma->vm_pgoff, dpi_start,
> + ucontext->dpi_size);
> +
> + if ((vma->vm_start & (PAGE_SIZE - 1)) || (len & (PAGE_SIZE - 1))) {
> + DP_ERR(dev,
> + "failed mmap, adrresses must be page aligned: start=0x%lx, end=0x%lx\n",
> + vma->vm_start, vma->vm_end);
> return -EINVAL;
> }
>
> - found = qedr_search_mmap(ucontext, vm_page, len);
> - if (!found) {
> - DP_ERR(dev, "Vma_pgoff not found in mapped array = %ld\n",
> + if (!qedr_search_mmap(ucontext, phys_addr, len)) {
> + DP_ERR(dev, "failed mmap, vm_pgoff=0x%lx is not authorized\n",
> vma->vm_pgoff);
> return -EINVAL;
> }
>
> - DP_DEBUG(dev, QEDR_MSG_INIT, "Mapping doorbell bar\n");
> -
> - if ((vm_page >= unmapped_db) && (vm_page <= (unmapped_db +
> - dev->db_size))) {
> - DP_DEBUG(dev, QEDR_MSG_INIT, "Mapping doorbell bar\n");
> - if (vma->vm_flags & VM_READ) {
> - DP_ERR(dev, "Trying to map doorbell bar for read\n");
> - return -EPERM;
> - }
> -
> - vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> + if (phys_addr < dpi_start ||
> + ((phys_addr + len) > (dpi_start + ucontext->dpi_size))) {
> + DP_ERR(dev,
> + "failed mmap, pages are outside of dpi; page address=0x%lx, dpi_start=0x%lx, dpi_size=0x%x\n",
> + phys_addr, dpi_start, ucontext->dpi_size);
> + return -EINVAL;
> + }
>
> - rc = io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff,
> - PAGE_SIZE, vma->vm_page_prot);
> - } else {
> - DP_DEBUG(dev, QEDR_MSG_INIT, "Mapping chains\n");
> - rc = remap_pfn_range(vma, vma->vm_start,
> - vma->vm_pgoff, len, vma->vm_page_prot);
> + if (vma->vm_flags & VM_READ) {
> + DP_ERR(dev, "failed mmap, cannot map doorbell bar for read\n");
> + return -EINVAL;
> }
> - DP_DEBUG(dev, QEDR_MSG_INIT, "qedr_mmap return code: %d\n", rc);
> - return rc;
> +
> + vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot);
> + return io_remap_pfn_range(vma, vma->vm_start, vma->vm_pgoff, len,
> + vma->vm_page_prot);
> }
>
> struct ib_pd *qedr_alloc_pd(struct ib_device *ibdev,
> --
> 2.9.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachment:
signature.asc
Description: PGP signature
