devm_kasprintf() returns NULL when memory allocation fails. Currently,
com20020pci_probe() does not check for this case, which results in a
NULL pointer dereference.

Add NULL check after devm_kasprintf() to prevent this issue.

Fixes: 6b17a597fc2f ("arcnet: restoring support for multiple Sohard Arcnet cards")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
V1 -> V2: Add a test after each devm_kasprintf().

 drivers/net/arcnet/com20020-pci.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
index c5e571ec94c9..b848968d6c9c 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -251,18 +251,25 @@ static int com20020pci_probe(struct pci_dev *pdev,
 			card->tx_led.default_trigger = devm_kasprintf(&pdev->dev,
 							GFP_KERNEL, "arc%d-%d-tx",
 							dev->dev_id, i);
+			if (!card->tx_led.default_trigger)
+				return -ENOMEM;
 			card->tx_led.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
 							"pci:green:tx:%d-%d",
 							dev->dev_id, i);
-
+			if (!card->tx_led.name)
+				return -ENOMEM;
 			card->tx_led.dev = &dev->dev;
 			card->recon_led.brightness_set = led_recon_set;
 			card->recon_led.default_trigger = devm_kasprintf(&pdev->dev,
 							GFP_KERNEL, "arc%d-%d-recon",
 							dev->dev_id, i);
+			if (!card->recon_led.default_trigger)
+				return -ENOMEM;
 			card->recon_led.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
 							"pci:red:recon:%d-%d",
 							dev->dev_id, i);
+			if (!card->recon_led.name)
+				return -ENOMEM;
 			card->recon_led.dev = &dev->dev;
 
 			ret = devm_led_classdev_register(&pdev->dev, &card->tx_led);
-- 
2.34.1

> devm_kasprintf() return NULL if memory allocation fails. Currently,
…
                call?                               failed?


> Add NULL check after devm_kasprintf() to prevent this issue.

Please complete also the corresponding exception handling.

Source code example for further inspiration:
https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/net/arcnet/com20020-pci.c#L239-L244

Thus I suggest to use another label like “e_nomem” for this purpose.

Regards,
Markus

On Tue, Apr 01, 2025 at 05:37:18PM +0200, Markus Elfring wrote:
> > devm_kasprintf() return NULL if memory allocation fails. Currently,
> …
>                 call?                               failed?
> 
> 
> > Add NULL check after devm_kasprintf() to prevent this issue.
> 
> Please complete also the corresponding exception handling.
> 
> Source code example for further inspiration:
> https://elixir.bootlin.com/linux/v6.14-rc6/source/drivers/net/arcnet/com20020-pci.c#L239-L244
> 
> Thus I suggest to use another label like “e_nomem” for this purpose.

Ok, but surely the err_free_arcdev label can be reused for this purpose.

devm_kasprintf() returns NULL when memory allocation fails. Currently,
com20020pci_probe() does not check for this case, which results in a
NULL pointer dereference.

Add NULL check after devm_kasprintf() to prevent this issue and ensure
no resources are left allocated.

Fixes: 6b17a597fc2f ("arcnet: restoring support for multiple Sohard Arcnet cards")
Signed-off-by: Henry Martin <bsdhenrymartin@gmail.com>
---
V2 -> V3: Reuse label err_free_arcdev for exception handing.
V1 -> V2: Add a test after each devm_kasprintf().

 drivers/net/arcnet/com20020-pci.c | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
index c5e571ec94c9..0472bcdff130 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -251,18 +251,33 @@ static int com20020pci_probe(struct pci_dev *pdev,
 			card->tx_led.default_trigger = devm_kasprintf(&pdev->dev,
 							GFP_KERNEL, "arc%d-%d-tx",
 							dev->dev_id, i);
+			if (!card->tx_led.default_trigger) {
+				ret = -ENOMEM;
+				goto err_free_arcdev;
+			}
 			card->tx_led.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
 							"pci:green:tx:%d-%d",
 							dev->dev_id, i);
-
+			if (!card->tx_led.name) {
+				ret = -ENOMEM;
+				goto err_free_arcdev;
+			}
 			card->tx_led.dev = &dev->dev;
 			card->recon_led.brightness_set = led_recon_set;
 			card->recon_led.default_trigger = devm_kasprintf(&pdev->dev,
 							GFP_KERNEL, "arc%d-%d-recon",
 							dev->dev_id, i);
+			if (!card->recon_led.default_trigger) {
+				ret = -ENOMEM;
+				goto err_free_arcdev;
+			}
 			card->recon_led.name = devm_kasprintf(&pdev->dev, GFP_KERNEL,
 							"pci:red:recon:%d-%d",
 							dev->dev_id, i);
+			if (!card->recon_led.name) {
+				ret = -ENOMEM;
+				goto err_free_arcdev;
+			}
 			card->recon_led.dev = &dev->dev;
 
 			ret = devm_led_classdev_register(&pdev->dev, &card->tx_led);
-- 
2.34.1

…
> Add NULL check after devm_kasprintf() to prevent this issue and ensure
> no resources are left allocated.

I hope that further refinement possibilities can be taken better into account.


…
> ---
> V2 -> V3: Reuse label err_free_arcdev for exception handing.
…
> +++ b/drivers/net/arcnet/com20020-pci.c
> @@ -251,18 +251,33 @@ static int com20020pci_probe(struct pci_dev *pdev,
>  			card->tx_led.default_trigger = devm_kasprintf(&pdev->dev,
>  							GFP_KERNEL, "arc%d-%d-tx",
>  							dev->dev_id, i);
> +			if (!card->tx_led.default_trigger) {
> +				ret = -ENOMEM;
> +				goto err_free_arcdev;
> +			}
…

I propose to avoid duplicate source code also for the shown completion of
the corresponding exception handling.
https://wiki.sei.cmu.edu/confluence/display/c/MEM12-C.+Consider+using+a+goto+chain+when+leaving+a+function+on+error+when+using+and+releasing+resources#MEM12C.Considerusingagotochainwhenleavingafunctiononerrorwhenusingandreleasingresources-CompliantSolution(copy_process()fromLinuxkernel)

See also once more:
* https://lore.kernel.org/linux-kernel/?q=e_nomem

* https://docs.kernel.org/process/maintainer-netdev.html

Regards,
Markus