[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20260122033959.2579113-1-kuba@kernel.org>
Date: Wed, 21 Jan 2026 19:39:59 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: tariqt@...dia.com
Cc: Jakub Kicinski <kuba@...nel.org>,
cratiu@...dia.com,
saeedm@...dia.com,
cjubran@...dia.com,
davem@...emloft.net,
horms@...nel.org,
linux-doc@...r.kernel.org,
mbloch@...dia.com,
moshe@...dia.com,
jiri@...dia.com,
edumazet@...gle.com,
gal@...dia.com,
andrew+netdev@...n.ch,
donald.hunter@...il.com,
jiri@...nulli.us,
krzk@...nel.org,
pabeni@...hat.com,
linux-kernel@...r.kernel.org,
linux-rdma@...r.kernel.org,
netdev@...r.kernel.org,
rdunlap@...radead.org,
leon@...nel.org,
corbet@....net
Subject: Re: [net-next,V5,10/15] net/mlx5: Add a shared devlink instance for PFs on same chip
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.
[ ... ]
Powered by blists - more mailing lists