On 7/25/25 2:47 PM, Marc Kleine-Budde wrote:

On 25.07.2025 14:32:27, Jimmy Assarsson wrote:

quoted

--- a/drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c
+++ b/drivers/net/can/kvaser_pciefd/kvaser_pciefd_core.c

@@ -1751,14 +1751,16 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
  			       const struct pci_device_id *id)
  {
  	int ret;
+	struct devlink *devlink;
  	struct device *dev = &pdev->dev;
  	struct kvaser_pciefd *pcie;
  	const struct kvaser_pciefd_irq_mask *irq_mask;
  
-	pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
-	if (!pcie)
+	devlink = devlink_alloc(&kvaser_pciefd_devlink_ops, sizeof(*pcie), dev);
+	if (!devlink)
  		return -ENOMEM;
  
+	pcie = devlink_priv(devlink);
  	pci_set_drvdata(pdev, pcie);
  	pcie->pci = pdev;
  	pcie->driver_data = (const struct kvaser_pciefd_driver_data *)id->driver_data;

@@ -1766,7 +1768,7 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
  
  	ret = pci_enable_device(pdev);
  	if (ret)
-		return ret;
+		goto err_free_devlink;
  
  	ret = pci_request_regions(pdev, KVASER_PCIEFD_DRV_NAME);
  	if (ret)

@@ -1830,6 +1832,8 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
  	if (ret)
  		goto err_free_irq;
  
+	devlink_register(devlink);
+
  	return 0;
  
  err_free_irq:

@@ -1853,6 +1857,9 @@ static int kvaser_pciefd_probe(struct pci_dev *pdev,
  err_disable_pci:
  	pci_disable_device(pdev);
  
+err_free_devlink:
+	devlink_free(devlink);
+
  	return ret;
  }
  

@@ -1876,6 +1883,8 @@ static void kvaser_pciefd_remove(struct pci_dev *pdev)
  	for (i = 0; i < pcie->nr_channels; ++i)
  		free_candev(pcie->can[i]->can.dev);
  
+	devlink_unregister(priv_to_devlink(pcie));
+	devlink_free(priv_to_devlink(pcie));
  	pci_iounmap(pdev, pcie->reg_base);

                           ^^^^

This smells like a use after free. Please call the cleanup function in
reverse order of allocation functions, i.e. move devlink_free() to the
end of this function.

quoted

  	pci_release_regions(pdev);
  	pci_disable_device(pdev);

regards,
Marc

I agree. Thanks for finding this!
I've tested moving devlink_free() to the end of the function, without any
issues.

If you don't find any other problems, do you mind making this change before sending
the PR? Otherwise I need to wait for the netdev 24h grace period, as Paolo
pointed out [1], before sending v5.

[1] https://lore.kernel.org/linux-can/20250725-furry-precise-jerboa-d9e29d-mkl@pengutronix.de/T/#m174402d37840f225b2799fbc53d7658ccc27be72 (local)

Best regards,
jimmy