Intel-Wired-Lan Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-wired-lan] [PATCH net v2] ice: Add check for kzalloc
@ 2022-12-08  1:19 Jiasheng Jiang
  2022-12-08  9:24 ` Leon Romanovsky
  0 siblings, 1 reply; 5+ messages in thread
From: Jiasheng Jiang @ 2022-12-08  1:19 UTC (permalink / raw)
  To: jiri
  Cc: intel-wired-lan, Jiasheng Jiang, jesse.brandeburg, linux-kernel,
	edumazet, anthony.l.nguyen, netdev, kuba, pabeni, davem

As kzalloc may return NULL pointer, the return value should
be checked and return error if fails in order to avoid the
NULL pointer dereference.
Moreover, use the goto-label to share the clean code.

Fixes: d6b98c8d242a ("ice: add write functionality for GNSS TTY")
Signed-off-by: Jiasheng Jiang <jiasheng@iscas.ac.cn>
---
Changelog:

v1 -> v2:

1. Use goto-label to share the clean code.
---
 drivers/net/ethernet/intel/ice/ice_gnss.c | 25 ++++++++++++++---------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice_gnss.c b/drivers/net/ethernet/intel/ice/ice_gnss.c
index b5a7f246d230..7bd3452a16d2 100644
--- a/drivers/net/ethernet/intel/ice/ice_gnss.c
+++ b/drivers/net/ethernet/intel/ice/ice_gnss.c
@@ -421,7 +421,7 @@ static struct tty_driver *ice_gnss_create_tty_driver(struct ice_pf *pf)
 	const int ICE_TTYDRV_NAME_MAX = 14;
 	struct tty_driver *tty_driver;
 	char *ttydrv_name;
-	unsigned int i;
+	unsigned int i, j;
 	int err;
 
 	tty_driver = tty_alloc_driver(ICE_GNSS_TTY_MINOR_DEVICES,
@@ -462,6 +462,9 @@ static struct tty_driver *ice_gnss_create_tty_driver(struct ice_pf *pf)
 					       GFP_KERNEL);
 		pf->gnss_serial[i] = NULL;
 
+		if (!pf->gnss_tty_port[i])
+			goto err_out;
+
 		tty_port_init(pf->gnss_tty_port[i]);
 		tty_port_link_device(pf->gnss_tty_port[i], tty_driver, i);
 	}
@@ -469,21 +472,23 @@ static struct tty_driver *ice_gnss_create_tty_driver(struct ice_pf *pf)
 	err = tty_register_driver(tty_driver);
 	if (err) {
 		dev_err(dev, "Failed to register TTY driver err=%d\n", err);
-
-		for (i = 0; i < ICE_GNSS_TTY_MINOR_DEVICES; i++) {
-			tty_port_destroy(pf->gnss_tty_port[i]);
-			kfree(pf->gnss_tty_port[i]);
-		}
-		kfree(ttydrv_name);
-		tty_driver_kref_put(pf->ice_gnss_tty_driver);
-
-		return NULL;
+		goto err_out;
 	}
 
 	for (i = 0; i < ICE_GNSS_TTY_MINOR_DEVICES; i++)
 		dev_info(dev, "%s%d registered\n", ttydrv_name, i);
 
 	return tty_driver;
+
+err_out:
+	for (j = 0; j < i; j++) {
+		tty_port_destroy(pf->gnss_tty_port[j]);
+		kfree(pf->gnss_tty_port[j]);
+	}
+	kfree(ttydrv_name);
+	tty_driver_kref_put(pf->ice_gnss_tty_driver);
+
+	return NULL;
 }
 
 /**
-- 
2.25.1

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] ice: Add check for kzalloc
@ 2022-12-08 10:06 Jiasheng Jiang
  2022-12-08 11:17 ` Jiri Pirko
  0 siblings, 1 reply; 5+ messages in thread
From: Jiasheng Jiang @ 2022-12-08 10:06 UTC (permalink / raw)
  To: leon
  Cc: jiri, intel-wired-lan, Jiasheng Jiang, jesse.brandeburg,
	linux-kernel, edumazet, anthony.l.nguyen, netdev, kuba, pabeni,
	davem

On Thu, Dec 08, 2022 at 05:25:02PM +0800, Leon Romanovsky wrote:
>> +err_out:
>> +	for (j = 0; j < i; j++) {
> 
> You don't need an extra variable, "while(i--)" will do the trick.

No, the right range is [0, i - 1], but the "while(i--)" is [1, i].
If using "while(i--)", the code should be "tty_port_destroy(pf->gnss_tty_port[i - 1]);".
It will be more complex.
Therefore, it is worthwhile to use an extra varaible.

Thanks,
Jiang

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [Intel-wired-lan] [PATCH net v2] ice: Add check for kzalloc
@ 2022-12-08 12:52 Jiasheng Jiang
  0 siblings, 0 replies; 5+ messages in thread
From: Jiasheng Jiang @ 2022-12-08 12:52 UTC (permalink / raw)
  To: jiri
  Cc: leon, intel-wired-lan, Jiasheng Jiang, jesse.brandeburg,
	linux-kernel, edumazet, anthony.l.nguyen, netdev, kuba, pabeni,
	davem

On Thu, Dec 08, 2022 at 07:17:59PM +0800, Leon Romanovsky wrote:
>>>> +err_out:
>>>> +	for (j = 0; j < i; j++) {
>>> 
>>> You don't need an extra variable, "while(i--)" will do the trick.
>>
>>No, the right range is [0, i - 1], but the "while(i--)" is [1, i].
> 
> Are you sure??

Yes, you are right.
I will submit a v3 to add this change.

Thanks,
Jiang

_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-12-08 16:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-08  1:19 [Intel-wired-lan] [PATCH net v2] ice: Add check for kzalloc Jiasheng Jiang
2022-12-08  9:24 ` Leon Romanovsky
  -- strict thread matches above, loose matches on Subject: below --
2022-12-08 10:06 Jiasheng Jiang
2022-12-08 11:17 ` Jiri Pirko
2022-12-08 12:52 Jiasheng Jiang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox