| Message ID | 1264447561-15472-1-git-send-email-stefan.bader@canonical.com |
|---|---|
| State | Accepted |
| Delegated to: | Stefan Bader |
| Headers | show |
Stefan Bader wrote: > SRU justification: > > Impact: Upstream 2.6.31.9 contained a patch that added a check for bogus > EEPROM checksums to the ath5k driver but it seems to have missed the fact > that custom EEPROMs might be different in size. > > Fix: Upstream patch which was added to 2.6.32.4 but not carried over to > 2.6.31.y adds code to query and work with differently sized EEPROMS. > > Testcase: Bring up certain hw with custom EEPROM. Verified in the report. > > -Stefan > > --- > > From e6efac7b7c4ce45d40f5e07d3105e07704e95673 Mon Sep 17 00:00:00 2001 > From: Luis R. Rodriguez <lrodriguez@atheros.com> > Date: Mon, 4 Jan 2010 10:40:39 -0500 > Subject: [PATCH] ath5k: Fix eeprom checksum check for custom sized eeproms > > BugLink: http://bugs.launchpad.net/bugs/506180 > > commit 359207c687cc8f4f9845c8dadd0d6dabad44e584 upstream. > > Commit 8bf3d79bc401ca417ccf9fc076d3295d1a71dbf5 enabled EEPROM > checksum checks to avoid bogus bug reports but failed to address > updating the code to consider devices with custom EEPROM sizes. > Devices with custom sized EEPROMs have the upper limit size stuffed > in the EEPROM. Use this as the upper limit instead of the static > default size. In case of a checksum error also provide back the > max size and whether or not this was the default size or a custom > one. If the EEPROM is busted we add a failsafe check to ensure > we don't loop forever or try to read bogus areas of hardware. > > This closes bug 14874 > > http://bugzilla.kernel.org/show_bug.cgi?id=14874 > > Cc: David Quan <david.quan@atheros.com> > Cc: Stephen Beahm <stephenbeahm@comcast.net> > Reported-by: Joshua Covington <joshuacov@googlemail.com> > Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com> > Signed-off-by: John W. Linville <linville@tuxdriver.com> > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/net/wireless/ath/ath5k/eeprom.c | 32 ++++++++++++++++++++++++++++-- > drivers/net/wireless/ath/ath5k/eeprom.h | 8 +++++++ > 2 files changed, 37 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c > index 7918852..9a96550 100644 > --- a/drivers/net/wireless/ath/ath5k/eeprom.c > +++ b/drivers/net/wireless/ath/ath5k/eeprom.c > @@ -97,7 +97,7 @@ ath5k_eeprom_init_header(struct ath5k_hw *ah) > struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom; > int ret; > u16 val; > - u32 cksum, offset; > + u32 cksum, offset, eep_max = AR5K_EEPROM_INFO_MAX; > > /* > * Read values from EEPROM and store them in the capability structure > @@ -116,12 +116,38 @@ ath5k_eeprom_init_header(struct ath5k_hw *ah) > * Validate the checksum of the EEPROM date. There are some > * devices with invalid EEPROMs. > */ > - for (cksum = 0, offset = 0; offset < AR5K_EEPROM_INFO_MAX; offset++) { > + AR5K_EEPROM_READ(AR5K_EEPROM_SIZE_UPPER, val); > + if (val) { > + eep_max = (val & AR5K_EEPROM_SIZE_UPPER_MASK) << > + AR5K_EEPROM_SIZE_ENDLOC_SHIFT; > + AR5K_EEPROM_READ(AR5K_EEPROM_SIZE_LOWER, val); > + eep_max = (eep_max | val) - AR5K_EEPROM_INFO_BASE; > + > + /* > + * Fail safe check to prevent stupid loops due > + * to busted EEPROMs. XXX: This value is likely too > + * big still, waiting on a better value. > + */ > + if (eep_max > (3 * AR5K_EEPROM_INFO_MAX)) { > + ATH5K_ERR(ah->ah_sc, "Invalid max custom EEPROM size: " > + "%d (0x%04x) max expected: %d (0x%04x)\n", > + eep_max, eep_max, > + 3 * AR5K_EEPROM_INFO_MAX, > + 3 * AR5K_EEPROM_INFO_MAX); > + return -EIO; > + } > + } > + > + for (cksum = 0, offset = 0; offset < eep_max; offset++) { > AR5K_EEPROM_READ(AR5K_EEPROM_INFO(offset), val); > cksum ^= val; > } > if (cksum != AR5K_EEPROM_INFO_CKSUM) { > - ATH5K_ERR(ah->ah_sc, "Invalid EEPROM checksum 0x%04x\n", cksum); > + ATH5K_ERR(ah->ah_sc, "Invalid EEPROM " > + "checksum: 0x%04x eep_max: 0x%04x (%s)\n", > + cksum, eep_max, > + eep_max == AR5K_EEPROM_INFO_MAX ? > + "default size" : "custom size"); > return -EIO; > } > > diff --git a/drivers/net/wireless/ath/ath5k/eeprom.h b/drivers/net/wireless/ath/ath5k/eeprom.h > index 0123f35..473a483 100644 > --- a/drivers/net/wireless/ath/ath5k/eeprom.h > +++ b/drivers/net/wireless/ath/ath5k/eeprom.h > @@ -37,6 +37,14 @@ > #define AR5K_EEPROM_RFKILL_POLARITY_S 1 > > #define AR5K_EEPROM_REG_DOMAIN 0x00bf /* EEPROM regdom */ > + > +/* FLASH(EEPROM) Defines for AR531X chips */ > +#define AR5K_EEPROM_SIZE_LOWER 0x1b /* size info -- lower */ > +#define AR5K_EEPROM_SIZE_UPPER 0x1c /* size info -- upper */ > +#define AR5K_EEPROM_SIZE_UPPER_MASK 0xfff0 > +#define AR5K_EEPROM_SIZE_UPPER_SHIFT 4 > +#define AR5K_EEPROM_SIZE_ENDLOC_SHIFT 12 > + > #define AR5K_EEPROM_CHECKSUM 0x00c0 /* EEPROM checksum */ > #define AR5K_EEPROM_INFO_BASE 0x00c0 /* EEPROM header */ > #define AR5K_EEPROM_INFO_MAX (0x400 - AR5K_EEPROM_INFO_BASE) Acked-by: Tim Gardner <tim.gardner@canonical.com>
On Mon, 2010-01-25 at 17:39 -0700, Tim Gardner wrote: > Stefan Bader wrote: > > SRU justification: > > > > Impact: Upstream 2.6.31.9 contained a patch that added a check for bogus > > EEPROM checksums to the ath5k driver but it seems to have missed the fact > > that custom EEPROMs might be different in size. > > > > Fix: Upstream patch which was added to 2.6.32.4 but not carried over to > > 2.6.31.y adds code to query and work with differently sized EEPROMS. > > > > Testcase: Bring up certain hw with custom EEPROM. Verified in the report. > > > > -Stefan > > > > --- > > > > From e6efac7b7c4ce45d40f5e07d3105e07704e95673 Mon Sep 17 00:00:00 2001 > > From: Luis R. Rodriguez <lrodriguez@atheros.com> > > Date: Mon, 4 Jan 2010 10:40:39 -0500 > > Subject: [PATCH] ath5k: Fix eeprom checksum check for custom sized eeproms > > > > BugLink: http://bugs.launchpad.net/bugs/506180 > > > > commit 359207c687cc8f4f9845c8dadd0d6dabad44e584 upstream. > > > > Commit 8bf3d79bc401ca417ccf9fc076d3295d1a71dbf5 enabled EEPROM > > checksum checks to avoid bogus bug reports but failed to address > > updating the code to consider devices with custom EEPROM sizes. > > Devices with custom sized EEPROMs have the upper limit size stuffed > > in the EEPROM. Use this as the upper limit instead of the static > > default size. In case of a checksum error also provide back the > > max size and whether or not this was the default size or a custom > > one. If the EEPROM is busted we add a failsafe check to ensure > > we don't loop forever or try to read bogus areas of hardware. > > > > This closes bug 14874 > > > > http://bugzilla.kernel.org/show_bug.cgi?id=14874 > > > > Cc: David Quan <david.quan@atheros.com> > > Cc: Stephen Beahm <stephenbeahm@comcast.net> > > Reported-by: Joshua Covington <joshuacov@googlemail.com> > > Signed-off-by: Luis R. Rodriguez <lrodriguez@atheros.com> > > Signed-off-by: John W. Linville <linville@tuxdriver.com> > > Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de> > > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > > --- > > drivers/net/wireless/ath/ath5k/eeprom.c | 32 ++++++++++++++++++++++++++++-- > > drivers/net/wireless/ath/ath5k/eeprom.h | 8 +++++++ > > 2 files changed, 37 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c > > index 7918852..9a96550 100644 > > --- a/drivers/net/wireless/ath/ath5k/eeprom.c > > +++ b/drivers/net/wireless/ath/ath5k/eeprom.c > > @@ -97,7 +97,7 @@ ath5k_eeprom_init_header(struct ath5k_hw *ah) > > struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom; > > int ret; > > u16 val; > > - u32 cksum, offset; > > + u32 cksum, offset, eep_max = AR5K_EEPROM_INFO_MAX; > > > > /* > > * Read values from EEPROM and store them in the capability structure > > @@ -116,12 +116,38 @@ ath5k_eeprom_init_header(struct ath5k_hw *ah) > > * Validate the checksum of the EEPROM date. There are some > > * devices with invalid EEPROMs. > > */ > > - for (cksum = 0, offset = 0; offset < AR5K_EEPROM_INFO_MAX; offset++) { > > + AR5K_EEPROM_READ(AR5K_EEPROM_SIZE_UPPER, val); > > + if (val) { > > + eep_max = (val & AR5K_EEPROM_SIZE_UPPER_MASK) << > > + AR5K_EEPROM_SIZE_ENDLOC_SHIFT; > > + AR5K_EEPROM_READ(AR5K_EEPROM_SIZE_LOWER, val); > > + eep_max = (eep_max | val) - AR5K_EEPROM_INFO_BASE; > > + > > + /* > > + * Fail safe check to prevent stupid loops due > > + * to busted EEPROMs. XXX: This value is likely too > > + * big still, waiting on a better value. > > + */ > > + if (eep_max > (3 * AR5K_EEPROM_INFO_MAX)) { > > + ATH5K_ERR(ah->ah_sc, "Invalid max custom EEPROM size: " > > + "%d (0x%04x) max expected: %d (0x%04x)\n", > > + eep_max, eep_max, > > + 3 * AR5K_EEPROM_INFO_MAX, > > + 3 * AR5K_EEPROM_INFO_MAX); > > + return -EIO; > > + } > > + } > > + > > + for (cksum = 0, offset = 0; offset < eep_max; offset++) { > > AR5K_EEPROM_READ(AR5K_EEPROM_INFO(offset), val); > > cksum ^= val; > > } > > if (cksum != AR5K_EEPROM_INFO_CKSUM) { > > - ATH5K_ERR(ah->ah_sc, "Invalid EEPROM checksum 0x%04x\n", cksum); > > + ATH5K_ERR(ah->ah_sc, "Invalid EEPROM " > > + "checksum: 0x%04x eep_max: 0x%04x (%s)\n", > > + cksum, eep_max, > > + eep_max == AR5K_EEPROM_INFO_MAX ? > > + "default size" : "custom size"); > > return -EIO; > > } > > > > diff --git a/drivers/net/wireless/ath/ath5k/eeprom.h b/drivers/net/wireless/ath/ath5k/eeprom.h > > index 0123f35..473a483 100644 > > --- a/drivers/net/wireless/ath/ath5k/eeprom.h > > +++ b/drivers/net/wireless/ath/ath5k/eeprom.h > > @@ -37,6 +37,14 @@ > > #define AR5K_EEPROM_RFKILL_POLARITY_S 1 > > > > #define AR5K_EEPROM_REG_DOMAIN 0x00bf /* EEPROM regdom */ > > + > > +/* FLASH(EEPROM) Defines for AR531X chips */ > > +#define AR5K_EEPROM_SIZE_LOWER 0x1b /* size info -- lower */ > > +#define AR5K_EEPROM_SIZE_UPPER 0x1c /* size info -- upper */ > > +#define AR5K_EEPROM_SIZE_UPPER_MASK 0xfff0 > > +#define AR5K_EEPROM_SIZE_UPPER_SHIFT 4 > > +#define AR5K_EEPROM_SIZE_ENDLOC_SHIFT 12 > > + > > #define AR5K_EEPROM_CHECKSUM 0x00c0 /* EEPROM checksum */ > > #define AR5K_EEPROM_INFO_BASE 0x00c0 /* EEPROM header */ > > #define AR5K_EEPROM_INFO_MAX (0x400 - AR5K_EEPROM_INFO_BASE) > > Acked-by: Tim Gardner <tim.gardner@canonical.com> Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com>
Applied to Karmic and pushed.
diff --git a/drivers/net/wireless/ath/ath5k/eeprom.c b/drivers/net/wireless/ath/ath5k/eeprom.c index 7918852..9a96550 100644 --- a/drivers/net/wireless/ath/ath5k/eeprom.c +++ b/drivers/net/wireless/ath/ath5k/eeprom.c @@ -97,7 +97,7 @@ ath5k_eeprom_init_header(struct ath5k_hw *ah) struct ath5k_eeprom_info *ee = &ah->ah_capabilities.cap_eeprom; int ret; u16 val; - u32 cksum, offset; + u32 cksum, offset, eep_max = AR5K_EEPROM_INFO_MAX; /* * Read values from EEPROM and store them in the capability structure @@ -116,12 +116,38 @@ ath5k_eeprom_init_header(struct ath5k_hw *ah) * Validate the checksum of the EEPROM date. There are some * devices with invalid EEPROMs. */ - for (cksum = 0, offset = 0; offset < AR5K_EEPROM_INFO_MAX; offset++) { + AR5K_EEPROM_READ(AR5K_EEPROM_SIZE_UPPER, val); + if (val) { + eep_max = (val & AR5K_EEPROM_SIZE_UPPER_MASK) << + AR5K_EEPROM_SIZE_ENDLOC_SHIFT; + AR5K_EEPROM_READ(AR5K_EEPROM_SIZE_LOWER, val); + eep_max = (eep_max | val) - AR5K_EEPROM_INFO_BASE; + + /* + * Fail safe check to prevent stupid loops due + * to busted EEPROMs. XXX: This value is likely too + * big still, waiting on a better value. + */ + if (eep_max > (3 * AR5K_EEPROM_INFO_MAX)) { + ATH5K_ERR(ah->ah_sc, "Invalid max custom EEPROM size: " + "%d (0x%04x) max expected: %d (0x%04x)\n", + eep_max, eep_max, + 3 * AR5K_EEPROM_INFO_MAX, + 3 * AR5K_EEPROM_INFO_MAX); + return -EIO; + } + } + + for (cksum = 0, offset = 0; offset < eep_max; offset++) { AR5K_EEPROM_READ(AR5K_EEPROM_INFO(offset), val); cksum ^= val; } if (cksum != AR5K_EEPROM_INFO_CKSUM) { - ATH5K_ERR(ah->ah_sc, "Invalid EEPROM checksum 0x%04x\n", cksum); + ATH5K_ERR(ah->ah_sc, "Invalid EEPROM " + "checksum: 0x%04x eep_max: 0x%04x (%s)\n", + cksum, eep_max, + eep_max == AR5K_EEPROM_INFO_MAX ? + "default size" : "custom size"); return -EIO; } diff --git a/drivers/net/wireless/ath/ath5k/eeprom.h b/drivers/net/wireless/ath/ath5k/eeprom.h index 0123f35..473a483 100644 --- a/drivers/net/wireless/ath/ath5k/eeprom.h +++ b/drivers/net/wireless/ath/ath5k/eeprom.h @@ -37,6 +37,14 @@ #define AR5K_EEPROM_RFKILL_POLARITY_S 1 #define AR5K_EEPROM_REG_DOMAIN 0x00bf /* EEPROM regdom */ + +/* FLASH(EEPROM) Defines for AR531X chips */ +#define AR5K_EEPROM_SIZE_LOWER 0x1b /* size info -- lower */ +#define AR5K_EEPROM_SIZE_UPPER 0x1c /* size info -- upper */ +#define AR5K_EEPROM_SIZE_UPPER_MASK 0xfff0 +#define AR5K_EEPROM_SIZE_UPPER_SHIFT 4 +#define AR5K_EEPROM_SIZE_ENDLOC_SHIFT 12 + #define AR5K_EEPROM_CHECKSUM 0x00c0 /* EEPROM checksum */ #define AR5K_EEPROM_INFO_BASE 0x00c0 /* EEPROM header */ #define AR5K_EEPROM_INFO_MAX (0x400 - AR5K_EEPROM_INFO_BASE)