From e7467515b6150debd50b4d270dfa8ad9aef5ed67 Mon Sep 17 00:00:00 2001 From: Nicolas Morey Date: May 24 2023 07:12:53 +0000 Subject: RDMA/cma: Ensure rdma_addr_cancel() happens before issuing more requests (bsc#1210629 CVE-2023-2176) --- diff --git a/patches.suse/RDMA-cma-Ensure-rdma_addr_cancel-happens-before-issu.patch b/patches.suse/RDMA-cma-Ensure-rdma_addr_cancel-happens-before-issu.patch new file mode 100644 index 0000000..51c54ee --- /dev/null +++ b/patches.suse/RDMA-cma-Ensure-rdma_addr_cancel-happens-before-issu.patch @@ -0,0 +1,128 @@ +From 305d568b72f17f674155a2a8275f865f207b3808 Mon Sep 17 00:00:00 2001 +From: Jason Gunthorpe +Date: Thu, 16 Sep 2021 15:34:46 -0300 +Subject: [PATCH 1/1] RDMA/cma: Ensure rdma_addr_cancel() happens before + issuing more requests +Git-commit: 305d568b72f17f674155a2a8275f865f207b3808 +Patch-mainline: v5.15 +References: bsc#1210629 CVE-2023-2176 + +The FSM can run in a circle allowing rdma_resolve_ip() to be called twice +on the same id_priv. While this cannot happen without going through the +work, it violates the invariant that the same address resolution +background request cannot be active twice. + + CPU 1 CPU 2 + +rdma_resolve_addr(): + RDMA_CM_IDLE -> RDMA_CM_ADDR_QUERY + rdma_resolve_ip(addr_handler) #1 + + process_one_req(): for #1 + addr_handler(): + RDMA_CM_ADDR_QUERY -> RDMA_CM_ADDR_BOUND + mutex_unlock(&id_priv->handler_mutex); + [.. handler still running ..] + +rdma_resolve_addr(): + RDMA_CM_ADDR_BOUND -> RDMA_CM_ADDR_QUERY + rdma_resolve_ip(addr_handler) + !! two requests are now on the req_list + +rdma_destroy_id(): + destroy_id_handler_unlock(): + _destroy_id(): + cma_cancel_operation(): + rdma_addr_cancel() + + // process_one_req() self removes it + spin_lock_bh(&lock); + cancel_delayed_work(&req->work); + if (!list_empty(&req->list)) == true + + ! rdma_addr_cancel() returns after process_on_req #1 is done + + kfree(id_priv) + + process_one_req(): for #2 + addr_handler(): + mutex_lock(&id_priv->handler_mutex); + !! Use after free on id_priv + +rdma_addr_cancel() expects there to be one req on the list and only +cancels the first one. The self-removal behavior of the work only happens +after the handler has returned. This yields a situations where the +req_list can have two reqs for the same "handle" but rdma_addr_cancel() +only cancels the first one. + +The second req remains active beyond rdma_destroy_id() and will +use-after-free id_priv once it inevitably triggers. + +Fix this by remembering if the id_priv has called rdma_resolve_ip() and +always cancel before calling it again. This ensures the req_list never +gets more than one item in it and doesn't cost anything in the normal flow +that never uses this strange error path. + +Link: https://lore.kernel.org/r/0-v1-3bc675b8006d+22-syz_cancel_uaf_jgg@nvidia.com +Cc: stable@vger.kernel.org +Fixes: e51060f08a61 ("IB: IP address based RDMA connection manager") +Reported-by: syzbot+dc3dfba010d7671e05f5@syzkaller.appspotmail.com +Signed-off-by: Jason Gunthorpe +Acked-by: Nicolas Morey +--- + drivers/infiniband/core/cma.c | 23 +++++++++++++++++++++++ + drivers/infiniband/core/cma_priv.h | 1 + + 2 files changed, 24 insertions(+) + +diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c +index 8862b0e572f0..704ce595542c 100644 +--- a/drivers/infiniband/core/cma.c ++++ b/drivers/infiniband/core/cma.c +@@ -365,6 +365,7 @@ struct rdma_id_private { + bool tos_set; + u8 reuseaddr; + u8 afonly; ++ u8 used_resolve_ip; + enum ib_gid_type gid_type; + }; + +@@ -1783,6 +1783,14 @@ static void cma_cancel_operation(struct rdma_id_private *id_priv, + { + switch (state) { + case RDMA_CM_ADDR_QUERY: ++ /* ++ * We can avoid doing the rdma_addr_cancel() based on state, ++ * only RDMA_CM_ADDR_QUERY has a work that could still execute. ++ * Notice that the addr_handler work could still be exiting ++ * outside this state, however due to the interaction with the ++ * handler_mutex the work is guaranteed not to touch id_priv ++ * during exit. ++ */ + rdma_addr_cancel(&id_priv->id.route.addr.dev_addr); + break; + case RDMA_CM_ROUTE_QUERY: +@@ -3425,6 +3433,21 @@ int rdma_resolve_addr(struct rdma_cm_id *id, struct sockaddr *src_addr, + if (dst_addr->sa_family == AF_IB) { + ret = cma_resolve_ib_addr(id_priv); + } else { ++ /* ++ * The FSM can return back to RDMA_CM_ADDR_BOUND after ++ * rdma_resolve_ip() is called, eg through the error ++ * path in addr_handler(). If this happens the existing ++ * request must be canceled before issuing a new one. ++ * Since canceling a request is a bit slow and this ++ * oddball path is rare, keep track once a request has ++ * been issued. The track turns out to be a permanent ++ * state since this is the only cancel as it is ++ * immediately before rdma_resolve_ip(). ++ */ ++ if (id_priv->used_resolve_ip) ++ rdma_addr_cancel(&id->route.addr.dev_addr); ++ else ++ id_priv->used_resolve_ip = 1; + ret = rdma_resolve_ip(&addr_client, cma_src_addr(id_priv), + dst_addr, &id->route.addr.dev_addr, + timeout_ms, addr_handler, id_priv); +-- +2.39.1.1.gbe015eda0162 + diff --git a/series.conf b/series.conf index dee6cf3..48420a9 100644 --- a/series.conf +++ b/series.conf @@ -26649,6 +26649,7 @@ patches.suse/KVM-Add-infrastructure-and-macro-to-mark-VM-as-bugged patches.suse/ipc-remove-memcg-accounting-for-sops-objects-in-do_semtimedop.patch patches.suse/HID-betop-fix-slab-out-of-bounds-Write-in-betop_prob.patch + patches.suse/RDMA-cma-Ensure-rdma_addr_cancel-happens-before-issu.patch patches.suse/ovl-fix-missing-negative-dentry-check-in-ovl_rename.patch patches.suse/bpf-Fix-integer-overflow-in-prealloc_elems_and_freel.patch patches.suse/powerpc-lib-Add-helper-to-check-if-offset-is-within-.patch