| Message ID | 1293471094-28976-1-git-send-email-dsterba@suse.cz |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | David Miller |
| Headers | show |
On Mon, Dec 27, 2010 at 06:31:34PM +0100, David Sterba wrote: > CC: Matt Carlson <mcarlson@broadcom.com> > CC: Michael Chan <mchan@broadcom.com> > Signed-off-by: David Sterba <dsterba@suse.cz> > --- Your fix is obviously correct, but could you describe the symptoms in your changelog instead of leaving it blank? In the original code, negative error values are ignored so we never goto out_not_found. Can pci_read_vpd() return any errors we care about besides -ENODEV? What I mean is, did you find this through analysing the code or did it cause a bug at runtime? regards, dan carpenter -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi, On Monday 27 December 2010 21:19:27 Dan Carpenter wrote: > Your fix is obviously correct, but could you describe the symptoms > in your changelog instead of leaving it blank? In the original code, > negative error values are ignored so we never goto out_not_found. Can > pci_read_vpd() return any errors we care about besides -ENODEV? What I > mean is, did you find this through analysing the code or did it cause a > bug at runtime? Although pci_read_vpd returns ENODEV directly, there is a call to vpd->read function which may return other error values and is eg. set to pci_vpd_pci22_read() : drivers/pci/access.c::pci_vpd_pci22_read() may return -EINVAL or -EINTR directly or -ETIMEDOUT or -EINTR via pci_vpd_pci22_wait() Yes, other negative values besides ETIMEDOUT and EINTR are ignored and after 3 attempts the pos != TG3_NVM_VPD_LEN check goes outwards. The fixed version allows to jump out immediately. So this does not manifest itself as a misbehaviour at runtime. It was found by code analysis. I'll post updated patch. dave -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/net/tg3.c b/drivers/net/tg3.c index 30ccbb6..6f97b7b 100644 --- a/drivers/net/tg3.c +++ b/drivers/net/tg3.c @@ -12658,7 +12658,7 @@ static void __devinit tg3_read_vpd(struct tg3 *tp) cnt = pci_read_vpd(tp->pdev, pos, TG3_NVM_VPD_LEN - pos, &vpd_data[pos]); - if (cnt == -ETIMEDOUT || -EINTR) + if (cnt == -ETIMEDOUT || cnt == -EINTR) cnt = 0; else if (cnt < 0) goto out_not_found;
CC: Matt Carlson <mcarlson@broadcom.com> CC: Michael Chan <mchan@broadcom.com> Signed-off-by: David Sterba <dsterba@suse.cz> --- drivers/net/tg3.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)