On Mon, Nov 23, 2020 at 04:56:24PM -0600, Shiraz Saleem wrote: > i40iw_mmap manipulates the vma->vm_pgoff to differentiate a push page > mmap vs a doorbell mmap, and uses it to compute the pfn in remap_pfn_range > without any validation. This is vulnerable to an mmap exploit as > described in [1]. > > Push feature is disabled in the driver currently and therefore no push > mmaps are issued from user-space. The feature does not work as expected > in the x722 product. So remove it along with the VMA attribute > manipulations for it in i40iw_mmap. > > Update i40iw_mmap to only allow DB user mmapings at offset = 0. > Check vm_pgoff for zero and if the mmaps are bound to a single page. > > [1] https://lore.kernel.org/linux-rdma/20201119093523.7588-1-zhudi21@xxxxxxxxxx/raw > > Fixes: d37498417947 ("i40iw: add files for iwarp interface") > Cc: stable@xxxxxxxxxx > Reported-by: Di Zhu <zhudi21@xxxxxxxxxx> > Signed-off-by: Shiraz Saleem <shiraz.saleem@xxxxxxxxx> > --- > drivers/infiniband/hw/i40iw/i40iw.h | 1 - > drivers/infiniband/hw/i40iw/i40iw_ctrl.c | 52 +------------ > drivers/infiniband/hw/i40iw/i40iw_d.h | 35 +++----- > drivers/infiniband/hw/i40iw/i40iw_main.c | 5 - > drivers/infiniband/hw/i40iw/i40iw_status.h | 1 - > drivers/infiniband/hw/i40iw/i40iw_type.h | 18 ---- > drivers/infiniband/hw/i40iw/i40iw_uk.c | 41 +-------- > drivers/infiniband/hw/i40iw/i40iw_user.h | 8 -- > drivers/infiniband/hw/i40iw/i40iw_verbs.c | 123 ++-------------------------- > 9 files changed, 26 insertions(+), 258 deletions(-) This is great, but it is too big for a single security patch stable back port. Please split it to two patches > diff --git a/drivers/infiniband/hw/i40iw/i40iw_main.c b/drivers/infiniband/hw/i40iw/i40iw_main.c > index 2408b27..584932d 100644 > --- a/drivers/infiniband/hw/i40iw/i40iw_main.c > +++ b/drivers/infiniband/hw/i40iw/i40iw_main.c > @@ -54,10 +54,6 @@ > #define DRV_VERSION __stringify(DRV_VERSION_MAJOR) "." \ > __stringify(DRV_VERSION_MINOR) "." __stringify(DRV_VERSION_BUILD) > > -static int push_mode; > -module_param(push_mode, int, 0644); > -MODULE_PARM_DESC(push_mode, "Low latency mode: 0=disabled (default), 1=enabled)"); The first should delete this module param (so push_mode == 0), and then this: > diff --git a/drivers/infiniband/hw/i40iw/i40iw_verbs.c b/drivers/infiniband/hw/i40iw/i40iw_verbs.c > index 581ecba..26dac09 100644 > --- a/drivers/infiniband/hw/i40iw/i40iw_verbs.c > +++ b/drivers/infiniband/hw/i40iw/i40iw_verbs.c > @@ -167,111 +167,18 @@ static void i40iw_dealloc_ucontext(struct ib_ucontext *context) > */ > static int i40iw_mmap(struct ib_ucontext *context, struct vm_area_struct *vma) > { > - struct i40iw_ucontext *ucontext; > - u64 db_addr_offset, push_offset, pfn; > - > - ucontext = to_ucontext(context); > - if (ucontext->iwdev->sc_dev.is_pf) { > - db_addr_offset = I40IW_DB_ADDR_OFFSET; > - push_offset = I40IW_PUSH_OFFSET; > - if (vma->vm_pgoff) > - vma->vm_pgoff += I40IW_PF_FIRST_PUSH_PAGE_INDEX - 1; > - } else { > - db_addr_offset = I40IW_VF_DB_ADDR_OFFSET; > - push_offset = I40IW_VF_PUSH_OFFSET; > - if (vma->vm_pgoff) > - vma->vm_pgoff += I40IW_VF_FIRST_PUSH_PAGE_INDEX - 1; > - } > + struct i40iw_ucontext *ucontext = to_ucontext(context); > + u64 dbaddr_pgoff, pfn; > > - vma->vm_pgoff += db_addr_offset >> PAGE_SHIFT; > - > - if (vma->vm_pgoff == (db_addr_offset >> PAGE_SHIFT)) { > - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > - } else { > - if ((vma->vm_pgoff - (push_offset >> PAGE_SHIFT)) % 2) > - vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > - else > - vma->vm_page_prot = pgprot_writecombine(vma->vm_page_prot); > - } > + if (vma->vm_pgoff || vma->vm_end - vma->vm_start != PAGE_SIZE) > + return -EINVAL; > > - pfn = vma->vm_pgoff + > - (pci_resource_start(ucontext->iwdev->ldev->pcidev, 0) >> > - PAGE_SHIFT); > + dbaddr_pgoff = I40IW_DB_ADDR_OFFSET >> PAGE_SHIFT; > + pfn = dbaddr_pgoff + (pci_resource_start(ucontext->iwdev->ldev->pcidev, 0) > + >> PAGE_SHIFT); > > return rdma_user_mmap_io(context, vma, pfn, PAGE_SIZE, > - vma->vm_page_prot, NULL); > -} Which should fix the bug and is a reasonable security backport I would also write the math as dbaddr = pci_resource_start(ucontext->iwdev->ldev->pcidev, 0) + I40IW_DB_ADDR_OFFSET return rdma_user_mmap_io(context, vma, dbaddr >> PAGE_SHIFT, PAGE_SIZE, vma->vm_page_prot, NULL); Which is easier to understand Then the 2nd patch is all the purging Jason
