Re: [PATCH] RDMA/i40iw: Fix a mmap handler exploitation

[Jason Gunthorpe <jgg@nvidia.com>]

On Thu, Nov 19, 2020 at 05:35:23PM +0800, zhudi wrote:
> From: Di Zhu <zhudi21@huawei.com>
> 
> Notice that i40iw_mmap() is used as mmap for file
> /dev/infiniband/uverbs%d and these files have access mode
> with 0666 specified by uverbs_devnode() and vma->vm_pgoff
> is directly used to calculate pfn which is passed in
> remap_pfn_range function without any valid validation.
> 
> This would result in a malicious process being able to pass an arbitrary
> physical address to ‘mmap’ which would allow for access to all of
> kernel memory from user space and eventually gain root privileges.
> 
> So, we should check whether final calculated value of vm_pgoff is
> in range of specified pci resource before we use it to calculate
> pfn which is passed in remap_pfn_range
> 
> Signed-off-by: Di Zhu <zhudi21@huawei.com>

needs a  fixes line
and cc stable

> ---
>  drivers/infiniband/hw/i40iw/i40iw_verbs.c | 4 ++++
>  1 file changed, 4 insertions(+)

Wow. Yes, you are completely right, and this is a huge security flaw.

Shiraz, I think your company has some process for handling security
bugs of this magnitude, I suggest you get started.

I see the same bug exists in the irdma out of tree driver too.

> diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> index 5ad381800f4d..7ec8f221eadb 100644
> --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c
> @@ -185,6 +185,10 @@ static int i40iw_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
>  
>  	vma->vm_pgoff += db_addr_offset >> PAGE_SHIFT;
>  
> +	if (vma->vm_pgoff >
> +		pci_resource_len(ucontext->iwdev->ldev->pcidev, 0) >> PAGE_SHIFT)
> +		return -EINVAL;

I am willing to apply this if Shiraz confirms it is OK

However, it is not the right fix. Shiraz you need to send me a patch
to make proper use of the new mmap cookie framework.

I see in the userspace there are only 3 acceptable values for offset:

- 0
- i40iw_ucreate_qp_resp->push_idx + I40IW_BASE_PUSH_PAGE
- i40iw_ucreate_qp_resp->push_idx + I40IW_BASE_PUSH_PAGE + 1

So create mmap cookies for only those values and derive the pfn only
from entry after extracting it from the cookie. This should also be
blocking access to parts of the BAR the process is not allowed to
access.

EFA has a pretty easy to follow example for the API in __efa_mmap:

	rdma_entry = rdma_user_mmap_entry_get(&ucontext->ibucontext, vma);
[..]
	pfn = entry->address >> PAGE_SHIFT;
[..]
		err = rdma_user_mmap_io(&ucontext->ibucontext, vma, pfn,
					entry->rdma_entry.npages * PAGE_SIZE,
					pgprot_noncached(vma->vm_page_prot),
					rdma_entry);

efa_user_mmap_entry_insert() shows how to get the cookie you'd return
in push_idx, for compatability you'd have to make some adjustments
here, but there are APIs for that too, mlx5 has examples.

Jason