netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] arcnet: Add NULL check in com20020pci_probe()
@ 2025-04-01 14:53 Henry Martin
  2025-04-01 15:37 ` Markus Elfring
  0 siblings, 1 reply; 6+ messages in thread
From: Henry Martin @ 2025-04-01 14:53 UTC (permalink / raw)
  To: m.grzeschik, andrew+netdev, davem, edumazet, kuba, pabeni
  Cc: netdev, linux-kernel, Henry Martin

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arcnet: Add NULL check in com20020pci_probe()
  2025-04-01 14:53 [PATCH v2] arcnet: Add NULL check in com20020pci_probe() Henry Martin
@ 2025-04-01 15:37 ` Markus Elfring
  2025-04-02  9:07   ` Simon Horman
  0 siblings, 1 reply; 6+ messages in thread
From: Markus Elfring @ 2025-04-01 15:37 UTC (permalink / raw)
  To: Henry Martin, netdev
  Cc: LKML, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Michael Grzeschik, Paolo Abeni, Simon Horman

> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v2] arcnet: Add NULL check in com20020pci_probe()
  2025-04-01 15:37 ` Markus Elfring
@ 2025-04-02  9:07   ` Simon Horman
  2025-04-02 13:50     ` [PATCH v3] " Henry Martin
  0 siblings, 1 reply; 6+ messages in thread
From: Simon Horman @ 2025-04-02  9:07 UTC (permalink / raw)
  To: Markus Elfring
  Cc: Henry Martin, netdev, LKML, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Michael Grzeschik, Paolo Abeni

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3] arcnet: Add NULL check in com20020pci_probe()
  2025-04-02  9:07   ` Simon Horman
@ 2025-04-02 13:50     ` Henry Martin
  2025-04-02 18:46       ` Markus Elfring
  2025-04-04 14:40       ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 6+ messages in thread
From: Henry Martin @ 2025-04-02 13:50 UTC (permalink / raw)
  To: m.grzeschik, andrew+netdev, davem, edumazet, kuba, pabeni
  Cc: netdev, linux-kernel, Henry Martin

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


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] arcnet: Add NULL check in com20020pci_probe()
  2025-04-02 13:50     ` [PATCH v3] " Henry Martin
@ 2025-04-02 18:46       ` Markus Elfring
  2025-04-04 14:40       ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: Markus Elfring @ 2025-04-02 18:46 UTC (permalink / raw)
  To: Henry Martin, netdev
  Cc: LKML, Andrew Lunn, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Michael Grzeschik, Paolo Abeni, Simon Horman

…
> 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3] arcnet: Add NULL check in com20020pci_probe()
  2025-04-02 13:50     ` [PATCH v3] " Henry Martin
  2025-04-02 18:46       ` Markus Elfring
@ 2025-04-04 14:40       ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 6+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-04-04 14:40 UTC (permalink / raw)
  To: Henry Martin
  Cc: m.grzeschik, andrew+netdev, davem, edumazet, kuba, pabeni, netdev,
	linux-kernel

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed,  2 Apr 2025 21:50:36 +0800 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [v3] arcnet: Add NULL check in com20020pci_probe()
    https://git.kernel.org/netdev/net/c/fda8c491db2a

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2025-04-04 14:39 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-01 14:53 [PATCH v2] arcnet: Add NULL check in com20020pci_probe() Henry Martin
2025-04-01 15:37 ` Markus Elfring
2025-04-02  9:07   ` Simon Horman
2025-04-02 13:50     ` [PATCH v3] " Henry Martin
2025-04-02 18:46       ` Markus Elfring
2025-04-04 14:40       ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).