All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH net v2] ice: Add check for kzalloc
@ 2022-12-08 10:06 ` Jiasheng Jiang
  0 siblings, 0 replies; 7+ messages in thread
From: Jiasheng Jiang @ 2022-12-08 10:06 UTC (permalink / raw)
  To: leon
  Cc: jiri, jesse.brandeburg, anthony.l.nguyen, davem, edumazet, kuba,
	pabeni, intel-wired-lan, netdev, linux-kernel, Jiasheng Jiang

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


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

Thu, Dec 08, 2022 at 11:06:03AM CET, jiasheng@iscas.ac.cn wrote:
>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].

Are you sure??


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

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

Thu, Dec 08, 2022 at 11:06:03AM CET, jiasheng@iscas.ac.cn wrote:
>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].

Are you sure??


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

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


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

On Thu, Dec 08, 2022 at 09:19:36AM +0800, Jiasheng Jiang wrote:
> 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++) {

You don't need an extra variable, "while(i--)" will do the trick.

Thanks

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

^ permalink raw reply	[flat|nested] 7+ messages in thread
* [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; 7+ messages in thread
From: Jiasheng Jiang @ 2022-12-08  1:19 UTC (permalink / raw)
  To: jiri
  Cc: jesse.brandeburg, anthony.l.nguyen, davem, edumazet, kuba,
	pabeni, intel-wired-lan, netdev, linux-kernel, Jiasheng Jiang

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


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

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.