linux-kernel - Re: [PATCH v3] drm/ast: astdp: fix loop timeout check

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: <9ab2e0bb-3c9a-4197-ab73-e0cc82c3efb6@suse.de>
Date: Mon, 12 Aug 2024 12:43:51 +0200
From: Thomas Zimmermann <tzimmermann@...e.de>
To: Dan Carpenter <dan.carpenter@...aro.org>
Cc: Dave Airlie <airlied@...hat.com>, Jocelyn Falempe <jfalempe@...hat.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, David Airlie <airlied@...il.com>,
 Daniel Vetter <daniel@...ll.ch>, dri-devel@...ts.freedesktop.org,
 linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org
Subject: Re: [PATCH v3] drm/ast: astdp: fix loop timeout check

Hi

Am 12.08.24 um 10:29 schrieb Dan Carpenter:
> This code has an issue because it loops until "i" is set to UINT_MAX but
> the test for failure assumes that "i" is set to zero.  The result is that
> it will only print an error message if we succeed on the very last try.
> Reformat the loop to count forwards instead of backwards.
>
> Fixes: 2281475168d2 ("drm/ast: astdp: Perform link training during atomic_enable")
> Signed-off-by: Dan Carpenter <dan.carpenter@...aro.org>

Reviewed-by: Thomas Zimmermann <tzimmermann@...e.de>

Thanks a lot for the fix. I'll merge it during the week if no other 
comments come in.

Best regards
Thomas

> ---
> v3: V2 had the same bug but just without the always true if (i) statement.
>      Remove the final sleep.
> v2: In V1, I introduced a bug where it would msleep(100) after failure
>      and that is a pointless thing to do.  Also change the loop to a for loop.
> ---
>   drivers/gpu/drm/ast/ast_dp.c | 15 ++++++++-------
>   1 file changed, 8 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/ast/ast_dp.c b/drivers/gpu/drm/ast/ast_dp.c
> index 5d07678b502c..ca022c287785 100644
> --- a/drivers/gpu/drm/ast/ast_dp.c
> +++ b/drivers/gpu/drm/ast/ast_dp.c
> @@ -146,18 +146,19 @@ void ast_dp_power_on_off(struct drm_device *dev, bool on)
>   void ast_dp_link_training(struct ast_device *ast)
>   {
>   	struct drm_device *dev = &ast->base;
> -	unsigned int i = 10;
> +	int i;
>   
> -	while (i--) {
> -		u8 vgacrdc = ast_get_index_reg(ast, AST_IO_VGACRI, 0xdc);
> +	for (i = 0; i < 10; i++) {
> +		u8 vgacrdc;
>   
> -		if (vgacrdc & AST_IO_VGACRDC_LINK_SUCCESS)
> -			break;
>   		if (i)
>   			msleep(100);
> +
> +		vgacrdc = ast_get_index_reg(ast, AST_IO_VGACRI, 0xdc);
> +		if (vgacrdc & AST_IO_VGACRDC_LINK_SUCCESS)
> +			return;
>   	}
> -	if (!i)
> -		drm_err(dev, "Link training failed\n");
> +	drm_err(dev, "Link training failed\n");
>   }
>   
>   void ast_dp_set_on_off(struct drm_device *dev, bool on)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)