Re: [net-next,V5,10/15] net/mlx5: Add a shared devlink instance for PFs on same chip

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
net/mlx5: Add a shared devlink instance for PFs on same chip

Creates shared devlink infrastructure for mlx5 PFs on the same physical
chip, identified by chip serial number from PCI VPD.

> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c
> new file mode 100644
> index 000000000000..abae5f0130e9
> --- /dev/null
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/sh_devlink.c

[ ... ]

> +int mlx5_shd_init(struct mlx5_core_dev *dev)
> +{
> +	u8 *vpd_data __free(kfree) = NULL;

The __free(kfree) annotation here combined with the early return below
looks problematic.

> +	struct pci_dev *pdev = dev->pdev;
> +	unsigned int vpd_size, kw_len;
> +	struct devlink *devlink;
> +	const char *sn;
> +	char *end;
> +	int start;
> +	int err;
> +
> +	if (!mlx5_core_is_pf(dev))
> +		return 0;
> +
> +	vpd_data = pci_vpd_alloc(pdev, &vpd_size);
> +	if (IS_ERR(vpd_data)) {
> +		err = PTR_ERR(vpd_data);
> +		return err == -ENODEV ? 0 : err;
> +	}

When pci_vpd_alloc() returns an ERR_PTR (e.g., -EIO or -ENOMEM) and the
function returns here, does the __free(kfree) cleanup call kfree() on an
ERR_PTR value? The kfree() implementation only handles NULL and
ZERO_SIZE_PTR safely via the ZERO_OR_NULL_PTR() check - ERR_PTR values
are large unsigned values that would pass this check and proceed to
virt_to_page() on an invalid address.

This same issue was raised by Krzysztof Kozlowski in review of V4:
https://lore.kernel.org/netdev/2df5ac80-5db6-4b8f-ab5b-b8b89237777f@kernel.org/

To fix this, either set vpd_data = NULL before the early return, or
remove the __free(kfree) annotation and handle cleanup manually.

[ ... ]