Re: [PATCH 5/7] RDMA/zrdma: Add device probe and remove logic

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] RDMA/zrdma: memory leaks on error paths in zxdh_probe()
--

commit 28ece0add9abc6cac3d040b5b5512cf8b3c518c3
Author: Yanze Zhang <zhang.yanze@zte.com.cn>

RDMA/zrdma: Add device probe and remove logic

This commit implements the auxiliary bus probe and remove callbacks for the
zrdma driver, including device initialization, hardware resource setup, and
IB device registration.

> diff --git a/drivers/infiniband/hw/zrdma/zrdma_main.c b/drivers/infiniband/hw/zrdma/zrdma_main.c
> new file mode 100644
> index 0000000000000..5127e56fd1642
> --- /dev/null
> +++ b/drivers/infiniband/hw/zrdma/zrdma_main.c
> @@ -0,0 +1,294 @@

[ ... ]

> +static int zxdh_probe(struct auxiliary_device *aux_dev,
> +		      const struct auxiliary_device_id *id)
> +{
> +	struct zxdh_auxiliary_dev *zxdh_adev =
> +		container_of(aux_dev, struct zxdh_auxiliary_dev, adev);
> +	struct zxdh_handler *hdl;
> +	struct zxdh_device *zdev;
> +	struct zxdh_pci_f *rf;
> +	struct net_device *netdev;
> +	int err;
> +
> +	if (!zxdh_adev->rdma_ops || !zxdh_adev->rdma_ops->get_rdma_netdev) {
> +		pr_err("zrdma: rdma_ops or get_rdma_netdev is NULL\n");
> +		return -EINVAL;
> +	}
> +
> +	netdev = zxdh_adev->rdma_ops->get_rdma_netdev(zxdh_adev->parent);
> +	if (!netdev) {
> +		pr_err("zrdma: failed to get netdev\n");
> +		return -ENODEV;
> +	}
> +
> +	zdev = ib_alloc_device(zxdh_device, ibdev);
> +	if (!zdev)
> +		return -ENOMEM;
> +
> +	zdev->zxdh_adev = zxdh_adev;
> +	zdev->netdev = netdev;
> +
> +	zdev->rf = kzalloc_obj(*zdev->rf);
> +	if (!zdev->rf) {
> +		ib_dealloc_device(&zdev->ibdev);
> +		return -ENOMEM;
> +	}
> +
> +	zxdh_fill_device_info(zdev, zxdh_adev->zxdh_info);
> +
> +	hdl = kzalloc_obj(*hdl);
> +	if (!hdl) {
> +		ib_dealloc_device(&zdev->ibdev);
> +		return -ENOMEM;
> +	}
> +
> +	hdl->zdev = zdev;
> +	zdev->hdl = hdl;
> +	zdev->netdev_speed = SPEED_UNKNOWN;
> +
> +	rf = zdev->rf;
> +	err = zxdh_ctrl_init_hw(rf);
> +	if (err)
> +		goto err_ctrl_init;
> +
> +	err = zxdh_ib_register_device(zdev);
> +	if (err)
> +		goto err_ib_reg;
> +
> +	/* Publish the handler only after the device is registered, so a
> +	 * failed probe cannot leave a freed handler on the global list.
> +	 */
> +	zxdh_add_handler(hdl);
> +
> +	dev_set_drvdata(&aux_dev->dev, zdev);
> +
> +	return 0;
> +
> +err_ib_reg:
> +err_ctrl_init:
> +	ib_dealloc_device(&zdev->ibdev);
> +
> +	return err;
> +}

[Severity: Medium]
Does zxdh_probe() leak zdev->rf, hdl, and the hardware resource buffers on its
error paths?

When kzalloc_obj(*hdl) fails, zxdh_probe() calls
ib_dealloc_device(&zdev->ibdev) and returns -ENOMEM without freeing zdev->rf
or destroying zdev->ah_list_lock initialized in zxdh_fill_device_info().

Similarly, zxdh_ctrl_init_hw() -> zxdh_initialize_hw_rsrc() allocates
rf->qp_cnt_8k_idxs and rf->mem_rsrc with vzalloc():

drivers/infiniband/hw/zrdma/zrdma_hw.c:zxdh_initialize_hw_rsrc() {
	...
	rf->qp_cnt_8k_idxs = vzalloc(rf->max_8k_idx * sizeof(u32));
	if (!rf->qp_cnt_8k_idxs) {
		ret = -ENOMEM;
		goto mem_8k_qp_cnt_vmalloc_fail;
	}

	rsrc_size = zxdh_calc_mem_rsrc_size(rf);
	rf->mem_rsrc = vzalloc(rsrc_size);
	...
}

If zxdh_ctrl_init_hw() or zxdh_ib_register_device() fails, jumping to
err_ctrl_init or err_ib_reg only calls ib_dealloc_device(&zdev->ibdev),
leaking hdl, zdev->rf, rf->qp_cnt_8k_idxs, and rf->mem_rsrc.

Later in the series, commit 42854f9bc8b3 ("RDMA/zrdma: Implement context, PD,
and mmap operations") wires up zxdh_ib_dealloc_device() as .dealloc_driver in
zxdh_dev_ops. However, ib_set_device_ops(&zdev->ibdev, &zxdh_dev_ops) is only
called inside zxdh_ib_register_device() -> zxdh_init_rdma_device() after the
roce_mode check. If zxdh_probe() fails at the hdl allocation, during
zxdh_ctrl_init_hw(), or before ib_set_device_ops() is reached,
zdev->ibdev.ops.dealloc_driver is still NULL when ib_dealloc_device() runs, so
zxdh_ib_dealloc_device() will not be called to clean up those allocations
(and hdl is not freed by zxdh_ib_dealloc_device() either).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024071539.2182864-1-zhang.yanze@zte.com.cn?part=5