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

["Saleem, Shiraz" <shiraz.saleem@intel.com>]

> Subject: Re: [PATCH] RDMA/i40iw: Fix a mmap handler exploitation
> 
> 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(+)
> 

[...]

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

I am ok with it. At least it reduces the severity of the issue ☹

> 
> 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

That’s right. But...see below

> 
> 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.
> 

Well, the push feature is disabled by default and today there will be no push page mmap from
user-space since uresp.push_idx is an invalid one. Its disabled for good reason
as its not working as expected. There is an option to turn it on via module param but that
does not work as expected still resulting in an invalid uresp.push_idx passed to user-space
and no mmap.

So isn’t it better to just remove the push related code in the driver? which should also remove the manipulation on the vm_pgoff I believe.

I will review the mmap API and see how we can use it for DB mmap.

Shiraz