On Sun, May 27, 2018 at 07:33:39PM +0300, Leon Romanovsky wrote: > On Tue, May 15, 2018 at 02:33:52PM -0700, Roland Dreier wrote: > > From: Roland Dreier <roland@xxxxxxxxxxxxxxx> > > > > There is a race (which userspace can trigger through ucma) that leads to > > use-after free in cma: > > > > rdma_bind_addr(id, bogus address) > > > > cma_comp_exch(RDMA_CM_IDLE, > > RDMA_CM_ADDR_BOUND) [succeed] > > copy address into id_priv struct > > fail to find device for address > > > > rdma_listen(id, any address); > > id_priv state isn't RDMA_CM_IDLE > > cma_comp_exch(RDMA_CM_ADDR_BOUND, > > RDMA_CM_LISTEN) [succeed] > > id->device not set, call > > cma_listen_on_all() > > add id_priv->list to listen_any_list > > return success > > > > cma_comp_exch(RDMA_CM_ADDR_BOUND, > > RDMA_CM_IDLE) [fail] > > return failure > > > > Now, when the id is destroyed, cma_release_dev() won't be called because > > id_priv->cma_dev isn't set. And cma_cancel_operation() won't call > > cma_cancel_listens() because cma_src_addr(id_priv) is the bogus address > > passed into rdma_bind_addr(). So neither of the paths that does > > > > list_del(&id_priv->list); > > > > will be followed, but the code will go ahead and kfree(id_priv) even > > though it is still linked into the listen_any_list. So we end up with > > use-after-free when listen_any_list is traversed. > > > > We can close this race by having rdma_bind_addr() put the CM ID into an > > intermediate "binding" state during the time that we are modifying the > > state but don't know whether the bind operation will succeed yet. > > > > Reported-and-tested-by: syzbot+db1c219466daac1083df@xxxxxxxxxxxxxxxxxxxxxxxxx > > Reported-by: Eric Biggers <ebiggers3@xxxxxxxxx> > > Signed-off-by: Roland Dreier <roland@xxxxxxxxxxxxxxx> > > drivers/infiniband/core/cma.c | 6 ++++-- > > drivers/infiniband/core/cma_priv.h | 1 + > > 2 files changed, 5 insertions(+), 2 deletions(-) > > > > What is the decision regarding this race? The fix is good, but it only solves this one race. I am thinking we should fix all the issues here at once though.. That needs a respin to do one of the ideas around locking from the 'test' cmpxchng to the 'set' in all places that follow that pattern Jason -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
