Skip to content

Commit

Permalink
RDMA/mlx5: Fix multiple NULL-ptr deref errors in rereg_mr flow
Browse files Browse the repository at this point in the history
Failure in rereg MR releases UMEM but leaves the MR to be destroyed
by the user. As a result the following scenario may happen:
"create MR -> rereg MR with failure -> call to rereg MR again" and
hit "NULL-ptr deref or user memory access" errors.

Ensure that rereg MR is only performed on a non-dead MR.

Cc: syzkaller <syzkaller@googlegroups.com>
Cc: <stable@vger.kernel.org> # 4.5
Fixes: 395a8e4 ("IB/mlx5: Refactoring register MR code")
Reported-by: Noa Osherovich <noaos@mellanox.com>
Signed-off-by: Leon Romanovsky <leonro@mellanox.com>
  • Loading branch information
Leon Romanovsky committed Apr 5, 2018
1 parent 67c5d72 commit 207d9fe
Showing 1 changed file with 23 additions and 9 deletions.
32 changes: 23 additions & 9 deletions drivers/infiniband/hw/mlx5/mr.c
Original file line number Diff line number Diff line change
Expand Up @@ -863,25 +863,28 @@ static int mr_umem_get(struct ib_pd *pd, u64 start, u64 length,
int *order)
{
struct mlx5_ib_dev *dev = to_mdev(pd->device);
struct ib_umem *u;
int err;

*umem = ib_umem_get(pd->uobject->context, start, length,
access_flags, 0);
err = PTR_ERR_OR_ZERO(*umem);
*umem = NULL;

u = ib_umem_get(pd->uobject->context, start, length, access_flags, 0);
err = PTR_ERR_OR_ZERO(u);
if (err) {
*umem = NULL;
mlx5_ib_err(dev, "umem get failed (%d)\n", err);
mlx5_ib_dbg(dev, "umem get failed (%d)\n", err);
return err;
}

mlx5_ib_cont_pages(*umem, start, MLX5_MKEY_PAGE_SHIFT_MASK, npages,
mlx5_ib_cont_pages(u, start, MLX5_MKEY_PAGE_SHIFT_MASK, npages,
page_shift, ncont, order);
if (!*npages) {
mlx5_ib_warn(dev, "avoid zero region\n");
ib_umem_release(*umem);
ib_umem_release(u);
return -EINVAL;
}

*umem = u;

mlx5_ib_dbg(dev, "npages %d, ncont %d, order %d, page_shift %d\n",
*npages, *ncont, *order, *page_shift);

Expand Down Expand Up @@ -1455,27 +1458,38 @@ int mlx5_ib_rereg_user_mr(struct ib_mr *ib_mr, int flags, u64 start,
int access_flags = flags & IB_MR_REREG_ACCESS ?
new_access_flags :
mr->access_flags;
u64 addr = (flags & IB_MR_REREG_TRANS) ? virt_addr : mr->umem->address;
u64 len = (flags & IB_MR_REREG_TRANS) ? length : mr->umem->length;
int page_shift = 0;
int upd_flags = 0;
int npages = 0;
int ncont = 0;
int order = 0;
u64 addr, len;
int err;

mlx5_ib_dbg(dev, "start 0x%llx, virt_addr 0x%llx, length 0x%llx, access_flags 0x%x\n",
start, virt_addr, length, access_flags);

atomic_sub(mr->npages, &dev->mdev->priv.reg_pages);

if (!mr->umem)
return -EINVAL;

if (flags & IB_MR_REREG_TRANS) {
addr = virt_addr;
len = length;
} else {
addr = mr->umem->address;
len = mr->umem->length;
}

if (flags != IB_MR_REREG_PD) {
/*
* Replace umem. This needs to be done whether or not UMR is
* used.
*/
flags |= IB_MR_REREG_TRANS;
ib_umem_release(mr->umem);
mr->umem = NULL;
err = mr_umem_get(pd, addr, len, access_flags, &mr->umem,
&npages, &page_shift, &ncont, &order);
if (err)
Expand Down

0 comments on commit 207d9fe

Please sign in to comment.