linux-kernel - Re: [PATCH v2] watchdog: wdat_wdt: Fix ACPI table leak in probe function

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]

Message-ID: <c4c1a84e-ee2d-459a-84dd-6dec3ee8e152@roeck-us.net>
Date: Mon, 17 Nov 2025 11:06:10 -0800
From: Guenter Roeck <linux@...ck-us.net>
To: Haotian Zhang <vulab@...as.ac.cn>, wim@...ux-watchdog.org
Cc: linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] watchdog: wdat_wdt: Fix ACPI table leak in probe
 function

On 11/12/25 18:30, Haotian Zhang wrote:
> wdat_wdt_probe() calls acpi_get_table() to obtain the WDAT ACPI table but
> never calls acpi_put_table() on any paths. This causes a permanent ACPI
> table memory leak.
> 
> Add a single cleanup path which calls acpi_put_table() to ensure
> the ACPI table is always released.
> 
> Fixes: 058dfc767008 ("ACPI / watchdog: Add support for WDAT hardware watchdog")
> Suggested-by: Guenter Roeck <linux@...ck-us.net>
> Signed-off-by: Haotian Zhang <vulab@...as.ac.cn>

Reviewed-by: Guenter Roeck <linux@...ck-us.net>

> ---
> Changes in v2:
>    -Remove unnecessary initialization.
>    -Correct the patch description, since the v1 patch already
>     free table on both error and success paths.
> ---
>   drivers/watchdog/wdat_wdt.c | 64 +++++++++++++++++++++++++------------
>   1 file changed, 43 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/watchdog/wdat_wdt.c b/drivers/watchdog/wdat_wdt.c
> index 650fdc7996e1..dd3c2d69c9df 100644
> --- a/drivers/watchdog/wdat_wdt.c
> +++ b/drivers/watchdog/wdat_wdt.c
> @@ -326,19 +326,27 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>   		return -ENODEV;
>   
>   	wdat = devm_kzalloc(dev, sizeof(*wdat), GFP_KERNEL);
> -	if (!wdat)
> -		return -ENOMEM;
> +	if (!wdat) {
> +		ret = -ENOMEM;
> +		goto out_put_table;
> +	}
>   
>   	regs = devm_kcalloc(dev, pdev->num_resources, sizeof(*regs),
>   			    GFP_KERNEL);
> -	if (!regs)
> -		return -ENOMEM;
> +	if (!regs) {
> +		ret = -ENOMEM;
> +		goto out_put_table;
> +	}
>   
>   	/* WDAT specification wants to have >= 1ms period */
> -	if (tbl->timer_period < 1)
> -		return -EINVAL;
> -	if (tbl->min_count > tbl->max_count)
> -		return -EINVAL;
> +	if (tbl->timer_period < 1) {
> +		ret = -EINVAL;
> +		goto out_put_table;
> +	}
> +	if (tbl->min_count > tbl->max_count) {
> +		ret = -EINVAL;
> +		goto out_put_table;
> +	}
>   
>   	wdat->period = tbl->timer_period;
>   	wdat->wdd.min_timeout = DIV_ROUND_UP(wdat->period * tbl->min_count, 1000);
> @@ -355,15 +363,20 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>   		res = &pdev->resource[i];
>   		if (resource_type(res) == IORESOURCE_MEM) {
>   			reg = devm_ioremap_resource(dev, res);
> -			if (IS_ERR(reg))
> -				return PTR_ERR(reg);
> +			if (IS_ERR(reg)) {
> +				ret = PTR_ERR(reg);
> +				goto out_put_table;
> +			}
>   		} else if (resource_type(res) == IORESOURCE_IO) {
>   			reg = devm_ioport_map(dev, res->start, 1);
> -			if (!reg)
> -				return -ENOMEM;
> +			if (!reg) {
> +				ret = -ENOMEM;
> +				goto out_put_table;
> +			}
>   		} else {
>   			dev_err(dev, "Unsupported resource\n");
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_put_table;
>   		}
>   
>   		regs[i] = reg;
> @@ -385,8 +398,10 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>   		}
>   
>   		instr = devm_kzalloc(dev, sizeof(*instr), GFP_KERNEL);
> -		if (!instr)
> -			return -ENOMEM;
> +		if (!instr) {
> +			ret = -ENOMEM;
> +			goto out_put_table;
> +		}
>   
>   		INIT_LIST_HEAD(&instr->node);
>   		instr->entry = entries[i];
> @@ -417,7 +432,8 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>   
>   		if (!instr->reg) {
>   			dev_err(dev, "I/O resource not found\n");
> -			return -EINVAL;
> +			ret = -EINVAL;
> +			goto out_put_table;
>   		}
>   
>   		instructions = wdat->instructions[action];
> @@ -425,8 +441,10 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>   			instructions = devm_kzalloc(dev,
>   						    sizeof(*instructions),
>   						    GFP_KERNEL);
> -			if (!instructions)
> -				return -ENOMEM;
> +			if (!instructions) {
> +				ret = -ENOMEM;
> +				goto out_put_table;
> +			}
>   
>   			INIT_LIST_HEAD(instructions);
>   			wdat->instructions[action] = instructions;
> @@ -443,7 +461,7 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>   
>   	ret = wdat_wdt_enable_reboot(wdat);
>   	if (ret)
> -		return ret;
> +		goto out_put_table;
>   
>   	platform_set_drvdata(pdev, wdat);
>   
> @@ -460,12 +478,16 @@ static int wdat_wdt_probe(struct platform_device *pdev)
>   
>   	ret = wdat_wdt_set_timeout(&wdat->wdd, timeout);
>   	if (ret)
> -		return ret;
> +		goto out_put_table;
>   
>   	watchdog_set_nowayout(&wdat->wdd, nowayout);
>   	watchdog_stop_on_reboot(&wdat->wdd);
>   	watchdog_stop_on_unregister(&wdat->wdd);
> -	return devm_watchdog_register_device(dev, &wdat->wdd);
> +	ret = devm_watchdog_register_device(dev, &wdat->wdd);
> +
> +out_put_table:
> +	acpi_put_table((struct acpi_table_header *)tbl);
> +	return ret;
>   }
>   
>   static int wdat_wdt_suspend_noirq(struct device *dev)