[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <Y5GtMk64Zg+tnbMS@unreal>
Date: Thu, 8 Dec 2022 11:24:02 +0200
From: Leon Romanovsky <leon@...nel.org>
To: Jiasheng Jiang <jiasheng@...as.ac.cn>
Cc: jiri@...nulli.us, jesse.brandeburg@...el.com,
anthony.l.nguyen@...el.com, davem@...emloft.net,
edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
intel-wired-lan@...ts.osuosl.org, netdev@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH net v2] ice: Add check for kzalloc
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@...as.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
>
Powered by blists - more mailing lists