linux-kernel - Re: [PATCH] watchdog: dw_wdt: Fix buffer overflow when get timeout

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]

Date:   Tue, 28 Jun 2022 06:55:29 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Schspa Shi <schspa@...il.com>, wim@...ux-watchdog.org
Cc:     linux-watchdog@...r.kernel.org, linux-kernel@...r.kernel.org,
        zhaohui.shi@...izon.ai
Subject: Re: [PATCH] watchdog: dw_wdt: Fix buffer overflow when get timeout

On 6/27/22 22:45, Schspa Shi wrote:
> The top_val can be obtained from device-tree, if it is not configured
> correctly, there will be buffer overflow.
> 
> Signed-off-by: Schspa Shi <schspa@...il.com>
> ---
>   drivers/watchdog/dw_wdt.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/watchdog/dw_wdt.c b/drivers/watchdog/dw_wdt.c
> index cd578843277e..1f8605c0d712 100644
> --- a/drivers/watchdog/dw_wdt.c
> +++ b/drivers/watchdog/dw_wdt.c
> @@ -155,6 +155,9 @@ static unsigned int dw_wdt_get_min_timeout(struct dw_wdt *dw_wdt)
>   			break;
>   	}
>   
> +	if (WARN_ON_ONCE(idx == DW_WDT_NUM_TOPS))
> +		idx = DW_WDT_NUM_TOPS - 1;
> +
dw_wdt_get_min_timeout() returns the lowest non-0 configurable timeout.
The  last entry in the timeout array must not be 0, meaning there must
be at least one entry in the array where the timeout is not 0. Therefore
this situation can not happen.

>   	return dw_wdt->timeouts[idx].sec;
>   }
>   
> @@ -178,6 +181,9 @@ static unsigned int dw_wdt_get_timeout(struct dw_wdt *dw_wdt)
>   			break;
>   	}
>   
> +	if (WARN_ON_ONCE(idx == DW_WDT_NUM_TOPS))
> +		idx = DW_WDT_NUM_TOPS - 1;
> +

idx is derived from a top_val value written into WDOG_TIMEOUT_RANGE_REG_OFFSET,
and the value written is derived from an entry in the timeouts array.
This array contains an entry for each possible top_val. While the array is not
sorted by top_val, dw_wdt_handle_tops() still guarantees that an entry exists.

I do not see how bad devicetree data can circumvent that. If it does, please
provide an example and explain.

Thanks,
Guenter