Re: [PATCH] RDMA/cma: Avoid using invalid state during rdma_bind_addr()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

This is basically what I was thinking as well..

My fear is that we have lots and lots of places where the
cma_comp_exch "lock" isn't right in this same sort of way, and
everything has to be audited..

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



[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Photo]     [Yosemite News]     [Yosemite Photos]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux