Re: [PATCH] ata: pata_hpt37x: fix potential forever loop

[Sergey Shtylyov <s.shtylyov@xxxxxx>]

Hello!

On 2/1/23 5:10 PM, Dan Carpenter wrote:

> This code accidentally reuses the "tries" iterator for both the inside
> and outside loops.  It could result in a forever loop if the "tries"
> variable gets reset to 0x1000 and never reaches 0x5000.
> 
> Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.")
> Signed-off-by: Dan Carpenter <error27@xxxxxxxxx>
> ---
>  drivers/ata/pata_hpt37x.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/ata/pata_hpt37x.c b/drivers/ata/pata_hpt37x.c
> index ce3c5eaa7e76..35be9a095b18 100644
> --- a/drivers/ata/pata_hpt37x.c
> +++ b/drivers/ata/pata_hpt37x.c
> @@ -621,14 +621,14 @@ static int hpt37x_calibrate_dpll(struct pci_dev *dev)
>  {
>  	u8 reg5b;
>  	u32 reg5c;
> -	int tries;
> +	int tries, tries2;
>  
>  	for (tries = 0; tries < 0x5000; tries++) {
>  		udelay(50);
>  		pci_read_config_byte(dev, 0x5b, &reg5b);
>  		if (reg5b & 0x80) {
>  			/* See if it stays set */
> -			for (tries = 0; tries < 0x1000; tries++) {
> +			for (tries2 = 0; tries2 < 0x1000; tries2++) {
>  				pci_read_config_byte(dev, 0x5b, &reg5b);
>  				/* Failed ? */
>  				if ((reg5b & 0x80) == 0)

   Looks like Alan tried to "fix" the DPLL calibration code imported from drivers/ide/hpt366.c
and failed at that... :-) The iriginal code has 2 sequential loops (thus 'tries' could be used
for both). In principle, I agree with a simplistic patch:

Reviewed-by: Sergey Shtylyov <s.shtylyov@xxxxxx>

MBR, Sergey