Re: [PATCH v20 net-next 08/10] net/nebula-matrix: add Dispatch layer implementation

[Paolo Abeni]

From: AI Reviewer <ai@xxxxxxxxxxx>

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/
---
net/nebula-matrix: add Dispatch layer implementation
    
This patch introduces a control-level routing mechanism for the Dispatch layer,
supporting direct and channel routing paths based on declared control levels.
It establishes the message handling framework and sets up the resource layer
locking strategy for operations like MSI-X configuration.

> diff --git a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c
> index f0b4406ca5601..ca565c6c4819f 100644
> --- a/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c
> +++ b/drivers/net/ethernet/nebula-matrix/nbl/nbl_core/nbl_dispatch.c

[ ... ]

> +static void nbl_disp_chan_configure_msix_map_resp(void *priv, u16 src_id,
> +						  u16 msg_id, void *data,
> +						  u32 data_len)
> +{
> +	struct nbl_dispatch_mgt *disp_mgt = (struct nbl_dispatch_mgt *)priv;
> +	struct nbl_channel_ops *chan_ops = disp_mgt->chan_ops_tbl->ops;
> +	struct nbl_resource_ops *res_ops = disp_mgt->res_ops_tbl->ops;
> +	struct nbl_resource_mgt *p = disp_mgt->res_ops_tbl->priv;
> +	struct device *dev = disp_mgt->common->dev;
> +	struct nbl_chan_param_cfg_msix_map param = { 0 };
> +	struct nbl_chan_ack_info chan_ack;
> +	int err = NBL_CHAN_RESP_OK;
> +	int copy_len;
> +	int ret;
> +
> +	copy_len = data_len < sizeof(param) ? data_len : sizeof(param);
> +	memcpy(&param, data, copy_len);
> +	ret = NBL_OPS_CALL_LOCK_RET(disp_mgt, res_ops->configure_msix_map, p,
> +				    src_id, le16_to_cpu(param.num_net_msix),
> +				    le16_to_cpu(param.num_others_msix),
> +				    le16_to_cpu(param.msix_mask_en));

[Severity: High]
This is a pre-existing issue, but does exposing this configuration via
channel messages allow a VM to trigger an unbounded memory leak?

Looking at the underlying nbl_res_intr_configure_msix_map() implementation,
it allocates device-managed memory for the MSI-X map table:

nbl_res_intr_configure_msix_map() {
    ...
    msix_map_table->base_addr = dmam_alloc_coherent(dev,
                                                msix_map_table->size,
                                                &msix_map_table->dma,
                                                GFP_KERNEL);
    ...
}

However, prior to this allocation, it appears to call
nbl_res_intr_destroy_msix_map(), which merely clears the pointer without
freeing the previously allocated memory:

nbl_res_intr_destroy_msix_map() {
    ...
    intr_mgt->func_intr_res[func_id].msix_map_table.base_addr = NULL;
    ...
}

Since this patch wires up nbl_disp_chan_configure_msix_map_resp() to
process NBL_CHAN_MSG_CONFIGURE_MSIX_MAP channel messages from untrusted VMs,
could a compromised VM repeatedly send these messages to exhaust host memory
and DMA mapping space?

> +	if (ret)
> +		err = NBL_CHAN_RESP_ERR;
> +
> +	NBL_CHAN_ACK(chan_ack, src_id, NBL_CHAN_MSG_CONFIGURE_MSIX_MAP, msg_id,
> +		     err, NULL, 0);
> +	ret = chan_ops->send_ack(disp_mgt->chan_ops_tbl->priv, &chan_ack);
> +	if (ret)
> +		dev_err(dev,
> +			"channel send ack failed with ret: %d, msg_type: %d\n",
> +			ret, NBL_CHAN_MSG_CONFIGURE_MSIX_MAP);
> +}
-- 
This is an AI-generated review.