Re: [PATCH] RDMA/rxe: fix responder UAF on IB_QP_MAX_DEST_RD_ATOMIC modify_qp

[Ibrahim Hashimov]

Hi Zhu Yanjun, thanks for the review.

> > BUG: KASAN: slab-use-after-free in rxe_receiver+0x4f78/0x89e0 [rdma_rxe]
> > Workqueue: rxe_wq do_work [rdma_rxe]
> Can you share all the stack trace with us? Thanks a lot.

Sure. The read side -- the recv_task kworker running rxe_receiver() on
the freed rd_atomic resource array -- from the pre-fix v6.19 kernel
(CONFIG_KASAN generic, rxe loopback):

  BUG: KASAN: slab-use-after-free in rxe_receiver+0x4f78/0x89e0 [rdma_rxe]
  Read of size 4 at addr ffff88800b79f84c by task kworker/u8:2/51
  CPU: 0 UID: 0 PID: 51 Comm: kworker/u8:2 Not tainted 6.19.0 #1
  Workqueue: rxe_wq do_work [rdma_rxe]
  Call Trace:
   <TASK>
   dump_stack_lvl+0x4d/0x70
   print_report+0x170/0x4f3
   kasan_report+0xda/0x110
   rxe_receiver+0x4f78/0x89e0 [rdma_rxe]
   do_work+0x144/0x470 [rdma_rxe]
   process_one_work+0x611/0xe80
   worker_thread+0x52e/0xdc0
   kthread+0x30c/0x630
   ret_from_fork+0x2fd/0x3e0
   ret_from_fork_asm+0x1a/0x30
   </TASK>

  The buggy address belongs to the object at ffff88800b79f800
   which belongs to the cache kmalloc-1k of size 1024
  The buggy address is located 76 bytes inside of
   freed 1024-byte region [ffff88800b79f800, ffff88800b79fc00)

The freed kmalloc-1k object is qp->resp.resources[] (the rd_atomic
array); it is freed by the concurrent
ib_modify_qp(IB_QP_MAX_DEST_RD_ATOMIC) thread in
free_rd_atomic_resources() <- rxe_qp_from_attr(), while this recv_task
kworker reads it. This was a kasan_multi_shot run and the per-object
Allocated-by/Freed-by backtraces were lost in the splat storm -- I can
send a single-shot capture that includes those two stacks if it would
help.

> If alloc_rd_atomic_resources fails, that is, qp->resp.resources is NULL.
> After rxe_enable_task(&qp->recv_task);  qp->resp.resources(NULL) will be
> used in resp. This will cause problems.
> drivers/infiniband/sw/rxe/rxe_resp.c:656:    res = &qp->resp.resources[qp->resp.res_head];
> drivers/infiniband/sw/rxe/rxe_resp.c:1325:        struct resp_res *res = &qp->resp.resources[i];

Good catch -- you're right. Re-enabling recv_task before the error
check resumes the responder against a NULL qp->resp.resources on the
ENOMEM path. v2 moves rxe_enable_task() below the error check, so the
responder is re-enabled only once a fresh array has been installed and
stays quiesced on failure:

	rxe_disable_task(&qp->recv_task);
	free_rd_atomic_resources(qp);
	err = alloc_rd_atomic_resources(qp, max_dest_rd_atomic);
	if (err)
		return err;
	rxe_enable_task(&qp->recv_task);

rxe_disable_task()/rxe_enable_task() are state-based (TASK_STATE_DRAINED
/IDLE), not refcounted, so leaving recv_task drained on the failed
modify is safe and recoverable: a subsequent successful modify_qp or a
QP destroy both handle the DRAINED state.

I'll send this as [PATCH v2].

Thanks,
Ibrahim