netdev - RE: [PATCH] mlxbf_gige: Add missing check for platform_get

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date: Fri, 2 Jun 2023 14:56:49 +0000
From: David Thompson <davthompson@...dia.com>
To: Jiasheng Jiang <jiasheng@...as.ac.cn>, "davem@...emloft.net"
	<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com" <pabeni@...hat.com>,
	Asmaa Mnebhi <asmaa@...dia.com>, "mkl@...gutronix.de" <mkl@...gutronix.de>,
	Liming Sun <limings@...dia.com>
CC: "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] mlxbf_gige: Add missing check for platform_get_irq

> -----Original Message-----
> From: Jiasheng Jiang <jiasheng@...as.ac.cn>
> Sent: Thursday, June 1, 2023 2:19 AM
> To: davem@...emloft.net; edumazet@...gle.com; kuba@...nel.org;
> pabeni@...hat.com; David Thompson <davthompson@...dia.com>; Asmaa
> Mnebhi <asmaa@...dia.com>; mkl@...gutronix.de; Liming Sun
> <limings@...dia.com>
> Cc: netdev@...r.kernel.org; linux-kernel@...r.kernel.org; Jiasheng Jiang
> <jiasheng@...as.ac.cn>
> Subject: [PATCH] mlxbf_gige: Add missing check for platform_get_irq
> 
> Add the check for the return value of the platform_get_irq and return error if it
> fails.
> 
> Fixes: f92e1869d74e ("Add Mellanox BlueField Gigabit Ethernet driver")
> Signed-off-by: Jiasheng Jiang <jiasheng@...as.ac.cn>
> ---
>  drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> index 694de9513b9f..a38e1c68874f 100644
> --- a/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> +++ b/drivers/net/ethernet/mellanox/mlxbf_gige/mlxbf_gige_main.c
> @@ -427,6 +427,10 @@ static int mlxbf_gige_probe(struct platform_device
> *pdev)
>  	priv->error_irq = platform_get_irq(pdev,
> MLXBF_GIGE_ERROR_INTR_IDX);
>  	priv->rx_irq = platform_get_irq(pdev,
> MLXBF_GIGE_RECEIVE_PKT_INTR_IDX);
>  	priv->llu_plu_irq = platform_get_irq(pdev,
> MLXBF_GIGE_LLU_PLU_INTR_IDX);
> +	if (priv->error_irq < 0 || priv->rx_irq < 0 || priv->llu_plu_irq < 0) {
> +		err = -ENODEV;
> +		goto out;
> +	}
> 

It is preferred to maintain the error code from "platform_get_irq" and return
that instead of -ENODEV.  Please review the API for "platform_get_irq" :

/**
 * platform_get_irq - get an IRQ for a device
 * @dev: platform device
 * @num: IRQ number index
 *
 * Gets an IRQ for a platform device and prints an error message if finding the
 * IRQ fails. Device drivers should check the return value for errors so as to
 * not pass a negative integer value to the request_irq() APIs.
 *
 * For example::
 *
 *              int irq = platform_get_irq(pdev, 0);
 *              if (irq < 0)
 *                      return irq;
 *
 * Return: non-zero IRQ number on success, negative error number on failure.
 */

So, the code could look something like this, for each of 3 IRQs:
	
	priv->error_irq = platform_get_irq(...)
	if (priv->error_irq < 0) {
		err = priv->error_irq;
		goto out;
	}

Just a thought.

Thanks,
- Dave

>  	phy_irq = acpi_dev_gpio_irq_get_by(ACPI_COMPANION(&pdev->dev),
> "phy-gpios", 0);
>  	if (phy_irq < 0) {
> --
> 2.25.1