| Message ID | 1580156212-28267-1-git-send-email-manjunath.b.patil@oracle.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Jason Gunthorpe |
| Headers | show |
| Series | IB/mlx4: Fix use after free in RDMA CM disconnect code path | expand |
On Mon, Jan 27, 2020 at 12:16:52PM -0800, Manjunath Patil wrote: > PS: > This fixes the recently submitted patch from Haakon[cc'd] > The commit hash used in here has to be updated while applying > Commit 01528b332860 ("IB/mlx4: Fix leak in id_map_find_del") introduces > two code paths that can pontentially cause use after free. Okay, I squished this into the original patch Thanks, Jason
Thank you. -Manjunath On 1/27/2020 12:50 PM, Jason Gunthorpe wrote: > On Mon, Jan 27, 2020 at 12:16:52PM -0800, Manjunath Patil wrote: >> PS: >> This fixes the recently submitted patch from Haakon[cc'd] >> The commit hash used in here has to be updated while applying >> Commit 01528b332860 ("IB/mlx4: Fix leak in id_map_find_del") introduces >> two code paths that can pontentially cause use after free. > Okay, I squished this into the original patch > > Thanks, > Jason
diff --git a/drivers/infiniband/hw/mlx4/cm.c b/drivers/infiniband/hw/mlx4/cm.c index 84ea003..9d2f216 100644 --- a/drivers/infiniband/hw/mlx4/cm.c +++ b/drivers/infiniband/hw/mlx4/cm.c @@ -331,8 +331,7 @@ int mlx4_ib_multiplex_cm_handler(struct ib_device *ibdev, int port, int slave_id cont: set_local_comm_id(mad, id->pv_cm_id); - if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID || - mad->mad_hdr.attr_id == CM_DREP_ATTR_ID) + if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID) schedule_delayed(ibdev, id); return 0; } @@ -373,8 +372,7 @@ int mlx4_ib_demux_cm_handler(struct ib_device *ibdev, int port, int *slave, set_remote_comm_id(mad, id->sl_cm_id); if (mad->mad_hdr.attr_id == CM_DREQ_ATTR_ID || - mad->mad_hdr.attr_id == CM_REJ_ATTR_ID || - mad->mad_hdr.attr_id == CM_DREP_ATTR_ID) + mad->mad_hdr.attr_id == CM_REJ_ATTR_ID) schedule_delayed(ibdev, id); return 0;
--- PS: This fixes the recently submitted patch from Haakon[cc'd] The commit hash used in here has to be updated while applying --- Commit 01528b332860 ("IB/mlx4: Fix leak in id_map_find_del") introduces two code paths that can pontentially cause use after free. 1. active side - Send DREQ and Recv DREP Sending of DREQ in mux schedules a work[id->timeout] to clean up the id_map_entry. Receive of DREP in demux also try to schedule the work. This can cause the second schedule_delayed() call to access the id, which might have been freed by the first scheduled work. We actually don't need to schedule clean up work when we receive DREP. Hence avoid scheduling of clean up work on receiving DREP[mlx4_ib_demux_cm_handler()]. 2. passive side - Recv DREQ and Send DREP Similar thing can happen on the passive side, when we receive a DREQ and send a DREP. We don't need to schedule clean up work when we send DREP. Hence avoid scheduling of clean up work while sending DREP[mlx4_ib_multiplex_cm_handler()]. How race can happen on active side: thread 1 thread 2 send DREQ schedule id->timeout ..delay.. receive DREP id = id_map_get(..) id->timeout() -> frees id schedule_delayed(ibdev, id) id is accessed inside schedule_delayed() Fixes: 01528b332860 ("IB/mlx4: Fix leak in id_map_find_del") Signed-off-by: Manjunath Patil <manjunath.b.patil@oracle.com> Reviewed-by: Rama Nichanamatlu <rama.nichanamatlu@oracle.com> --- drivers/infiniband/hw/mlx4/cm.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)