From 90f65e45ad3994340710bc6ec4d30b20003a743a Mon Sep 17 00:00:00 2001 From: Guangguan Wang Date: Fri, 7 Jul 2023 10:45:45 +0800 Subject: [PATCH 1/3] anolis: net/smc: do not validate rkey by testing rkey value When sending rdma_write data, checking whether the rkey is valid is needed. The value of valid range of rkey depends on the vendor implementation of RNIC. So validate rkey by testing rtokens_used_mask instead of testing rkey value. Fixes: b4919d778d9a ("anolis: net/smc: Introduce rtoken validity check before sending") Signed-off-by: Guangguan Wang --- net/smc/smc_tx.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/smc/smc_tx.c b/net/smc/smc_tx.c index 645abd73453c..0555f553824d 100644 --- a/net/smc/smc_tx.c +++ b/net/smc/smc_tx.c @@ -349,7 +349,7 @@ static int smc_tx_rdma_write(struct smc_connection *conn, int peer_rmbe_offset, peer_rmbe_offset; rdma_wr->rkey = lgr->rtokens[conn->rtoken_idx][link->link_idx].rkey; /* rtoken might be deleted if peer freed connection */ - if (!rdma_wr->rkey || + if (!test_bit(conn->rtoken_idx, lgr->rtokens_used_mask) || (rdma_wr->remote_addr == (conn->tx_off + peer_rmbe_offset))) { pr_warn_ratelimited("smc: unexpected sends during connection termination flow\n"); return -EINVAL; -- Gitee From 5a57b23cc41366158bdf883f2d23b56ca9d5d722 Mon Sep 17 00:00:00 2001 From: Guangguan Wang Date: Fri, 7 Jul 2023 11:45:01 +0800 Subject: [PATCH 2/3] anolis: net/smc: protect rdma_write processing from deleting rkey Rdma write operation depends on rkey and dma_addr, but rdma write processing is not mutually exclusive with delete rkey processing. Thus, when processing rdma write operations, delete rkey may happen, which may result to rdma write operation failure or other unforeseen errors. Using sendlock in connection to protect rdma_write processing from deleting rkey. Signed-off-by: Guangguan Wang --- net/smc/smc_core.c | 14 ++++++++++++++ net/smc/smc_core.h | 24 ++++++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/net/smc/smc_core.c b/net/smc/smc_core.c index e97cf71cebca..95c0ee1d86f7 100644 --- a/net/smc/smc_core.c +++ b/net/smc/smc_core.c @@ -2032,6 +2032,7 @@ int smc_conn_create(struct smc_sock *smc, struct smc_init_info *ini) &smc_lgr_list.lock; ini->first_contact_local = 1; role = smc->listen_smc ? SMC_SERV : SMC_CLNT; + conn->rtoken_idx = -1; /* -1 means rtoken not handled */ if (role == SMC_CLNT && ini->first_contact_peer) /* create new link group as well */ goto create; @@ -2693,17 +2694,30 @@ int smc_rtoken_add(struct smc_link *lnk, __be64 nw_vaddr, __be32 nw_rkey) int smc_rtoken_delete(struct smc_link *lnk, __be32 nw_rkey) { struct smc_link_group *lgr = smc_get_lgr(lnk); + struct smc_sock *smc = NULL; u32 rkey = ntohl(nw_rkey); int i, j; for (i = 0; i < SMC_RMBS_PER_LGR_MAX; i++) { if (lgr->rtokens[i][lnk->link_idx].rkey == rkey && test_bit(i, lgr->rtokens_used_mask)) { + read_lock_bh(&lgr->conns_lock); + smc = smc_lgr_get_sock_by_rtoken(i, lgr); + read_unlock_bh(&lgr->conns_lock); + if (smc) + spin_lock_bh(&smc->conn.send_lock); + for (j = 0; j < SMC_LINKS_PER_LGR_MAX; j++) { lgr->rtokens[i][j].rkey = 0; lgr->rtokens[i][j].dma_addr = 0; } clear_bit(i, lgr->rtokens_used_mask); + + if (smc) { + spin_unlock_bh(&smc->conn.send_lock); + /* sock_hold in smc_lgr_get_sock_by_rtoken */ + sock_put(&smc->sk); + } return 0; } } diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h index 57605e056370..c39d2612b62f 100644 --- a/net/smc/smc_core.h +++ b/net/smc/smc_core.h @@ -469,6 +469,30 @@ static inline struct smc_connection *smc_lgr_find_conn( return res; } +/* Find the smc sock associated with the given rtoken_idx in the link group. + * Requires @conns_lock + * @rtoken_idx rtoken index to search for + * @lgr link group to search in + * Returns smc_sock associated with rtoken_idx if found, NULL otherwise. + * sock_put(&smc->sk) must be called after using the smc_sock. + */ +static inline struct smc_sock * +smc_lgr_get_sock_by_rtoken(int rtoken_idx, struct smc_link_group *lgr) +{ + struct smc_connection *cur, *tmp; + struct smc_sock *res = NULL; + + rbtree_postorder_for_each_entry_safe(cur, tmp, &lgr->conns_all, alert_node) { + if (cur->rtoken_idx == rtoken_idx) { + res = container_of(cur, struct smc_sock, conn); + sock_hold(&res->sk); + break; + } + } + + return res; +} + static inline bool smc_conn_lgr_valid(struct smc_connection *conn) { return conn->lgr && conn->alert_token_local; -- Gitee From 1ff942a79701dbaa7b54879074d031dcee334f90 Mon Sep 17 00:00:00 2001 From: Guangguan Wang Date: Fri, 7 Jul 2023 17:36:18 +0800 Subject: [PATCH 3/3] anolis: net/smc: sock_hold in conns_lock protect area when smc_lgr_find_conn The smc_sock ptr may be freed after smc_lgr_find_conn. It may be occur UAF even if sock_hold after smc_lgr_find_conn. The conns_lock is used to protect conn from removing when smc_lgr_find_conn, which also protect smc sock from freeing. Thus, sock_hold the smc sock in conns_lock protect area when smc_lgr_find_conn can avoid the occurrance of UAF. Signed-off-by: Guangguan Wang --- net/smc/smc_cdc.c | 17 ++++++++--------- net/smc/smc_core.h | 22 ++++++++++++++++++++++ 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/net/smc/smc_cdc.c b/net/smc/smc_cdc.c index c25bc9b64f3a..382fe12e19a9 100644 --- a/net/smc/smc_cdc.c +++ b/net/smc/smc_cdc.c @@ -91,12 +91,12 @@ void smc_cdc_tx_handler_rwwi(struct ib_wc *wc) wr_id.data = wc->wr_id; read_lock_bh(&lgr->conns_lock); - conn = smc_lgr_find_conn(wr_id.token, lgr); + smc = smc_lgr_get_sock(wr_id.token, lgr); read_unlock_bh(&lgr->conns_lock); - if (!conn) + if (!smc) return; - smc = container_of(conn, struct smc_sock, conn); + conn = &smc->conn; bh_lock_sock(&smc->sk); if (!wc->status) { @@ -125,6 +125,7 @@ void smc_cdc_tx_handler_rwwi(struct ib_wc *wc) smc_tx_sndbuf_nonfull(smc); bh_unlock_sock(&smc->sk); + sock_put(&smc->sk); /* sock_hold in smc_lgr_get_sock */ } int smc_cdc_get_free_slot(struct smc_connection *conn, @@ -670,14 +671,12 @@ void smc_cdc_rx_handler_rwwi(struct ib_wc *wc) imm_msg.imm_data = be32_to_cpu(wc->ex.imm_data); read_lock_bh(&lgr->conns_lock); - conn = smc_lgr_find_conn(imm_msg.hdr.token, lgr); + smc = smc_lgr_get_sock(imm_msg.hdr.token, lgr); read_unlock_bh(&lgr->conns_lock); - if (!conn) + if (!smc) return; - smc = container_of(conn, struct smc_sock, conn); - - sock_hold(&smc->sk); + conn = &smc->conn; bh_lock_sock(&smc->sk); diff_prod = wc->byte_len; if (diff_prod) @@ -702,7 +701,7 @@ void smc_cdc_rx_handler_rwwi(struct ib_wc *wc) } bh_unlock_sock(&smc->sk); - sock_put(&smc->sk); /* no free sk in softirq-context */ + sock_put(&smc->sk); /* sock_hold in smc_lgr_get_sock */ } static struct smc_wr_rx_handler smc_cdc_rx_handlers[] = { diff --git a/net/smc/smc_core.h b/net/smc/smc_core.h index c39d2612b62f..925acef02dd9 100644 --- a/net/smc/smc_core.h +++ b/net/smc/smc_core.h @@ -469,6 +469,28 @@ static inline struct smc_connection *smc_lgr_find_conn( return res; } +/* Find the smc sock associated with the given alert token in the link group. + * Requires @conns_lock + * @token alert token to search for + * @lgr link group to search in + * Returns smc_sock associated with token if found, NULL otherwise. + * sock_put(&smc->sk) must be called after using the smc_sock. + */ +static inline struct smc_sock * +smc_lgr_get_sock(u32 token, struct smc_link_group *lgr) +{ + struct smc_connection *conn = NULL; + struct smc_sock *smc = NULL; + + conn = smc_lgr_find_conn(token, lgr); + if (!conn) + return NULL; + + smc = container_of(conn, struct smc_sock, conn); + sock_hold(&smc->sk); + return smc; +} + /* Find the smc sock associated with the given rtoken_idx in the link group. * Requires @conns_lock * @rtoken_idx rtoken index to search for -- Gitee