| Message ID | Y9pyzLUShZJeLDq7@kili |
|---|---|
| State | New |
| Headers | show |
| Series | ata: pata_hpt3x2n: prevent potential forever loop in hpt3xn_calibrate_dpll() | expand |
On 2/1/23 5:10 PM, Dan Carpenter wrote: > This code accidentally reuses "tries" as the iterator for both the inside > and outside loops. It means that the potentially the "tries" could be > reset to 0x1000 and never reach 0x5000. > > Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.") > Signed-off-by: Dan Carpenter <error27@gmail.com> [...] Again, I'm OK with this simplistic fix: Reviewed-by: Sergey Shtylyov <s.shtylyov@omp.ru> [...] MBR, Sergey
On 2/1/23 23:10, Dan Carpenter wrote: > This code accidentally reuses "tries" as the iterator for both the inside > and outside loops. It means that the potentially the "tries" could be > reset to 0x1000 and never reach 0x5000. > > Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.") > Signed-off-by: Dan Carpenter <error27@gmail.com> > --- > drivers/ata/pata_hpt3x2n.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/ata/pata_hpt3x2n.c b/drivers/ata/pata_hpt3x2n.c > index 617c95522f43..447dc287a2d4 100644 > --- a/drivers/ata/pata_hpt3x2n.c > +++ b/drivers/ata/pata_hpt3x2n.c > @@ -380,14 +380,14 @@ static int hpt3xn_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, ®5b); > 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, ®5b); > /* Failed ? */ > if ((reg5b & 0x80) == 0) I am assuming this one is the same as for pata_hpt37x: a false positive ?
On Tue, Feb 14, 2023 at 12:47:52PM +0900, Damien Le Moal wrote: > On 2/1/23 23:10, Dan Carpenter wrote: > > This code accidentally reuses "tries" as the iterator for both the inside > > and outside loops. It means that the potentially the "tries" could be > > reset to 0x1000 and never reach 0x5000. > > > > Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.") > > Signed-off-by: Dan Carpenter <error27@gmail.com> > > --- > > drivers/ata/pata_hpt3x2n.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/ata/pata_hpt3x2n.c b/drivers/ata/pata_hpt3x2n.c > > index 617c95522f43..447dc287a2d4 100644 > > --- a/drivers/ata/pata_hpt3x2n.c > > +++ b/drivers/ata/pata_hpt3x2n.c > > @@ -380,14 +380,14 @@ static int hpt3xn_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, ®5b); > > 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, ®5b); > > /* Failed ? */ > > if ((reg5b & 0x80) == 0) > > I am assuming this one is the same as for pata_hpt37x: a false positive ? Yes, sorry. regards, dan carpenter
diff --git a/drivers/ata/pata_hpt3x2n.c b/drivers/ata/pata_hpt3x2n.c index 617c95522f43..447dc287a2d4 100644 --- a/drivers/ata/pata_hpt3x2n.c +++ b/drivers/ata/pata_hpt3x2n.c @@ -380,14 +380,14 @@ static int hpt3xn_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, ®5b); 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, ®5b); /* Failed ? */ if ((reg5b & 0x80) == 0)
This code accidentally reuses "tries" as the iterator for both the inside and outside loops. It means that the potentially the "tries" could be reset to 0x1000 and never reach 0x5000. Fixes: 669a5db411d8 ("[libata] Add a bunch of PATA drivers.") Signed-off-by: Dan Carpenter <error27@gmail.com> --- drivers/ata/pata_hpt3x2n.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)