[PATCH] IB/usnic: fix wild pointer dereference in usnic_ib_qp_grp_dump_rows()

[PATCH] IB/usnic: fix wild pointer dereference in usnic_ib_qp_grp_dump_rows()

[Li Ximo]

From: Wang Haoran <haoranwangsec@gmail.com>

While auditing list_first_entry() users we found that
usnic_ib_qp_grp_dump_rows() calls list_first_entry() on
qp_grp->flows_lst without checking whether the list is empty.
list_first_entry() never returns NULL: on an empty list head->next ==
head, so container_of() yields a bogus non-NULL pointer derived from
the list head address. The function then dereferences it twice through
"default_flow->flow->flow_id", which reads a garbage pointer out of
the enclosing structure and dereferences that in turn, almost
certainly causing a kernel oops.

The empty-list state is reachable on a normal path: transitioning a QP
group to IB_QPS_RESET via usnic_ib_qp_grp_modify() calls
release_and_remove_all_flows(), which empties flows_lst, while
disable_qp_grp() only disables the hardware queues and does not
release the VNIC resources, so res->owner still points at the qp_grp.
A later usnic_ib_create_qp() on the same VF unconditionally dumps the
whole VF via usnic_ib_log_vf() -> usnic_vnic_dump(), which invokes
this callback for every res->owner != NULL - including the reset
qp_grp with an empty flows_lst. The existing "if (obj)" guard does not
help because obj (the qp_grp itself) is non-NULL; nothing checks the
flow list.

Fix it by switching to list_first_entry_or_null() and printing "N/A"
for the flow ID column when the qp_grp has no flow, mirroring the
existing all-"N/A" formatting used for a NULL obj. This was found by
code audit; we do not have Cisco usNIC hardware to reproduce it, but
the triggering sequence (create QP, modify to RESET, create another QP
on the same VF) is a plain libibverbs flow.

Reported-by: Wang Haoran <haoranwangsec@gmail.com>
Reported-by: Li Ximo <lixicaicai@gmail.com>
Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>
---
 drivers/infiniband/hw/usnic/usnic_ib_qp_grp.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/infiniband/hw/usnic/usnic_ib_qp_grp.c b/drivers/infiniband/hw/usnic/usnic_ib_qp_grp.c
index a7327c7a7..6f6aaea06 100644
--- a/drivers/infiniband/hw/usnic/usnic_ib_qp_grp.c
+++ b/drivers/infiniband/hw/usnic/usnic_ib_qp_grp.c
@@ -78,8 +78,17 @@ int usnic_ib_qp_grp_dump_rows(void *obj, char *buf, int buf_sz)
 	struct usnic_ib_qp_grp *qp_grp = obj;
 	struct usnic_ib_qp_grp_flow *default_flow;
 	if (obj) {
-		default_flow = list_first_entry(&qp_grp->flows_lst,
-					struct usnic_ib_qp_grp_flow, link);
+		default_flow = list_first_entry_or_null(&qp_grp->flows_lst,
+							struct usnic_ib_qp_grp_flow,
+							link);
+		if (!default_flow)
+			return scnprintf(buf, buf_sz,
+					"|%d\t|%s\t|%d\t|%hu\t|N/A",
+					qp_grp->ibqp.qp_num,
+					usnic_ib_qp_grp_state_to_string(
+							qp_grp->state),
+					qp_grp->owner_pid,
+					usnic_vnic_get_index(qp_grp->vf->vnic));
 		return scnprintf(buf, buf_sz, "|%d\t|%s\t|%d\t|%hu\t|%d",
 					qp_grp->ibqp.qp_num,
 					usnic_ib_qp_grp_state_to_string(
-- 
2.30.2

Re: [PATCH] IB/usnic: fix wild pointer dereference in usnic_ib_qp_grp_dump_rows()

[sashiko-bot]

> From: Wang Haoran <haoranwangsec@gmail.com>
> 
> While auditing list_first_entry() users we found that
> usnic_ib_qp_grp_dump_rows() calls list_first_entry() on
> qp_grp->flows_lst without checking whether the list is empty.
> list_first_entry() never returns NULL: on an empty list head->next ==
> head, so container_of() yields a bogus non-NULL pointer derived from
> the list head address. The function then dereferences it twice through
> "default_flow->flow->flow_id", which reads a garbage pointer out of
> the enclosing structure and dereferences that in turn, almost
> certainly causing a kernel oops.
> 
> The empty-list state is reachable on a normal path: transitioning a QP
> group to IB_QPS_RESET via usnic_ib_qp_grp_modify() calls
> release_and_remove_all_flows(), which empties flows_lst, while
> [ ... ]
> on the same VF) is a plain libibverbs flow.
> 
> Reported-by: Wang Haoran <haoranwangsec@gmail.com>
> Reported-by: Li Ximo <lixicaicai@gmail.com>
> Signed-off-by: Wang Haoran <haoranwangsec@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023073349.1287022-1-lixicaicai@gmail.com?part=1