smc_llc_srv_add_link() keeps add_llc pointing into the queue entry: add_llc = &qentry->msg.add_link; smc_llc.c:1482 ... smc_llc_save_add_link_info(link_new, add_llc); smc_llc.c:1494 smc_llc_flow_qentry_del(&lgr->llc_flow_lcl); smc_llc.c:1495 ... u8 *llc_msg = smc_link_shared_v2_rxbuf(link) ? (u8 *)lgr->wr_rx_buf_v2 : (u8 *)add_llc; smc_llc.c:1504 smc_llc_save_add_link_rkeys(link, link_new, llc_msg); smc_llc.c:1506 smc_llc_flow_qentry_del() kfree()s the entry, so on a link without a shared v2 receive buffer the pointer handed to smc_llc_save_add_link_rkeys() is already freed. Before the Fixes: commit that branch always used lgr->wr_rx_buf_v2 and add_llc was not used after the free. Detach the entry instead of freeing it there, and free it explicitly on both exit paths. The reject path has to detach as well, otherwise it would be freed twice. Fixes: 27ef6a9981fe ("net/smc: support SMC-R V2 for rdma devices with max_recv_sge equals to 1") Cc: stable@vger.kernel.org Signed-off-by: Yehyeong Lee --- Reproduced on an unpatched tree over rxe, with KASAN, kasan_multi_shot and a link forced to max_recv_sge == 1: the entry is freed and read by the same call, and the freeing frame is smc_llc_srv_add_link() itself. BUG: KASAN: slab-use-after-free in smc_llc_save_add_link_rkeys+0x333/0x350 Read of size 2 at addr ffff8880052194de by task kworker/0:1/11 Workqueue: smc_hs_wq smc_listen_work smc_llc_save_add_link_rkeys+0x333/0x350 smc_llc_srv_add_link+0xaa2/0x1e50 smc_listen_work+0x489e/0x4d00 Allocated by task 48: smc_llc_enqueue+0x72/0x560 smc_wr_rx_tasklet_fn+0x474/0xa80 Freed by task 11: kfree+0x121/0x380 smc_llc_srv_add_link+0x9a8/0x1e50 smc_listen_work+0x489e/0x4d00 The buggy address is located 94 bytes inside of freed 96-byte region [ffff888005219480, ffff8880052194e0) The offset is past the 72-byte queue entry because the out-of-bounds read fixed by the next patch is on the same line; what this patch removes is the free at smc_llc_srv_add_link+0x9a8 happening before the read at +0xaa2. net/smc/smc_llc.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/net/smc/smc_llc.c b/net/smc/smc_llc.c index 954b2ff1815c..90746cd1e29a 100644 --- a/net/smc/smc_llc.c +++ b/net/smc/smc_llc.c @@ -1481,7 +1481,7 @@ int smc_llc_srv_add_link(struct smc_link *link, } add_llc = &qentry->msg.add_link; if (add_llc->hd.flags & SMC_LLC_FLAG_ADD_LNK_REJ) { - smc_llc_flow_qentry_del(&lgr->llc_flow_lcl); + smc_llc_flow_qentry_clr(&lgr->llc_flow_lcl); rc = -ENOLINK; goto out_err; } @@ -1492,7 +1492,8 @@ int smc_llc_srv_add_link(struct smc_link *link, lgr_new_t = SMC_LGR_ASYMMETRIC_PEER; } smc_llc_save_add_link_info(link_new, add_llc); - smc_llc_flow_qentry_del(&lgr->llc_flow_lcl); + /* add_llc still points into qentry, so only detach it here */ + smc_llc_flow_qentry_clr(&lgr->llc_flow_lcl); rc = smc_ib_ready_link(link_new); if (rc) @@ -1512,6 +1513,7 @@ int smc_llc_srv_add_link(struct smc_link *link, rc = smc_llc_srv_conf_link(link, link_new, lgr_new_t); if (rc) goto out_err; + kfree(qentry); kfree(ini); return 0; out_err: @@ -1520,6 +1522,7 @@ int smc_llc_srv_add_link(struct smc_link *link, smcr_link_clear(link_new, false); } out: + kfree(qentry); kfree(ini); if (send_req_add_link_resp) smc_llc_send_req_add_link_response(req_qentry); -- 2.43.0