linux-kernel - Re: [PATCH] gpio: tegra186: Check PMC driver status before any request

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <Y1aa3/oJA2ElSGp0@orome>
Date:   Mon, 24 Oct 2022 16:02:07 +0200
From:   Thierry Reding <thierry.reding@...il.com>
To:     Prathamesh Shete <pshete@...dia.com>
Cc:     linus.walleij@...aro.org, bgolaszewski@...libre.com,
        linux-gpio@...r.kernel.org, linux-tegra@...r.kernel.org,
        linux-kernel@...r.kernel.org, jonathanh@...dia.com,
        smangipudi@...dia.com, Manish Bhardwaj <mbhardwaj@...dia.com>
Subject: Re: [PATCH] gpio: tegra186: Check PMC driver status before any
 request

On Mon, Sep 12, 2022 at 07:03:09PM +0530, Prathamesh Shete wrote:
> This patch fixes the issue where even if pmc driver
> status is disabled still we are invoking pmc driver
> to process some request

s/pmc/PMC/ twice above. Also, this is slightly misleading. We're not
"invoking" the PMC driver, but rather we're trying to look up the IRQ
domain that the PMC driver would've registered if it had been enabled.
So perhaps reword this to more accurately reflect that.

> 
> Signed-off-by: Manish Bhardwaj <mbhardwaj@...dia.com>
> Signed-off-by: Prathamesh Shete <pshete@...dia.com>
> ---
>  drivers/gpio/gpio-tegra186.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpio-tegra186.c b/drivers/gpio/gpio-tegra186.c
> index 54d9fa7da9c1..efd508ba07a6 100644
> --- a/drivers/gpio/gpio-tegra186.c
> +++ b/drivers/gpio/gpio-tegra186.c
> @@ -895,7 +895,7 @@ static int tegra186_gpio_probe(struct platform_device *pdev)
>  		tegra186_gpio_init_route_mapping(gpio);
>  
>  	np = of_find_matching_node(NULL, tegra186_pmc_of_match);
> -	if (np) {
> +	if (of_device_is_available(np)) {
>  		irq->parent_domain = irq_find_host(np);
>  		of_node_put(np);

This now leaks a reference to np if np is found but not available. So
this should be something like:

	if (np) {
		if (of_device_is_available(np)) {
			irq->parent_domain = irq_find_host(np);
			of_node_put(np);

			if (!irq->parent_domain)
				return -EPROBE_DEFER;
		} else {
			of_node_put(np);
		}
	}

or:

	if (np) {
		if (of_device_is_available(np))
			irq->parent_domain = irq_find_host(np);

		of_node_put(np);

		if (of_device_is_available(np) && !irq->parent_domain)
			return -EPROBE_DEFER;
	}

The former is a little nicer because it doesn't check availability
twice.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)