* Re: [ath5k] OOPS bisected - ath5k_eeprom_init_header() - "XX: This value is likely too big still, waiting on a better value" [not found] <4DFF2156.4000100@googlemail.com> @ 2011-06-20 10:34 ` Alan Jenkins 0 siblings, 0 replies; 2+ messages in thread From: Alan Jenkins @ 2011-06-20 10:34 UTC (permalink / raw) To: linux-wireless@vger.kernel.org, linux-kernel Cc: Luis R. Rodriguez, ath5k-devel@lists.ath5k.org [Removed "XXX" from subject in order to get past VGER spam filter] On 20/06/11 11:30, Alan Jenkins wrote: > ## Summary of bug report ## > Confidence: high. > (Bisected. Crash message captured, see end of message). > > Severity: low. > (Users can easily avoid the trigger for this crash). > > > ## Scenario: ## > Asus EeePC 701. > Wireless-toggle button, implemented in firmware, disables / enables > the entire PCI slot. (See platform driver eeepc-laptop). > > ## Expected results: ## > Ath5k (like any other PCI driver), must tolerate removal of PCI > devices without warning, and *never crash*. > > ## Actual results: ## > Ath5k generally handles removal very well, but crashes in a corner > case. This is a regression introduced by v2.6.33 (commit commit > 359207c687cc8f4f9845c8dadd0d6dabad44e584 "ath5k: Fix eeprom checksum > check for custom sized eeproms"). > > ## How to reproduce: ## > Repeatedly press the wireless-toggle button as fast as you can. It > shouldn't take more than about 20 presses before the OOPS occurs. > > > ## Results of git-bisect ## > > commit 359207c687cc8f4f9845c8dadd0d6dabad44e584 > Author: Luis R. Rodriguez <lrodriguez@atheros.com> > Date: Mon Jan 4 10:40:39 2010 -0500 > > ath5k: Fix eeprom checksum check for custom sized eeproms > > 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: stable@kernel.org > 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> > > ... > > - 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++) { > > > ### Example OOPS message ### > > pci 0000:01:00.0: reg 10: [mem 0x00000000-0x0000ffff 64bit] > pci 0000:01:00.0: BAR 0: assigned [mem 0xf8000000-0xf800ffff 64bit] > pci 0000:01:00.0: BAR 0: set to [mem 0xf8000000-0xf800ffff 64bit] (PCI > address [0xf8000000-0xf800ffff] > ath5k 0000:01:00.0: enabling device (0000 -> 0002) > ath5k 0000:01:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18 > ath5k 0000:01:00.0: setting latency timer to 64 > ath5k 0000:01:00.0: registered as 'phy9' > BUG: unable to handle kernel paging request at 1ef700e0 > IP: [<c05a098c>] iowrite32+0xd/0x30 > *pde = 00000000 > Oops: 0002 [#1] SMP > last sysfs file: /sys/devices/platform/eeepc/rfkill/rfkill0/uevent > Modules linked in: aes_i586 aes_generic snd_hda_codec_realtek arc4 ecb > snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device > ip6t_REJECT ath5k snd_pcm nf_conntrack_ipv6 mac80211 uvcvideo iTCO_wdt > ip6table_filter ath iTCO_vendor_support videodev snd_timer > eeepc_laptop microcode ip6_tables v4l1_compat joydev i2c_i801 cfg80211 > snd sparse_keymap atl2 rfkill soundcore snd_page_alloc ipv6 autofs4 > i915 drm_kms_helper drm usb_storage i2c_algo_bit i2c_core video output > > Pid: 17, comm: kacpi_notify Not tainted 2.6.33.3-85.fc13.i686 #1 701/701 > EIP: 0060:[<c05a098c>] EFLAGS: 00010216 CPU: 0 > EIP is at iowrite32+0xd/0x30 > EAX: 00000020 EBX: de324000 ECX: 1ef700e0 EDX: 1ef700e0 > ESI: 00000020 EDI: decabdba EBP: decabd90 ESP: decabd90 > DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 > Process kacpi_notify (pid: 17, ti=decaa000 task=dec84c80 > task.ti=decaa000) > Stack: > decabda4 e08af61e de0e0a40 00000007 decabdba decabdc8 e08af6df decabe3a > <0> de324000 00000000 00000000 de0e0a40 00000007 decabe38 decabe54 > e08c041c > <0> 0000000c decabe1c 0000000c e09a0000 de325680 def6a500 1ef6a500 > de324000 > Call Trace: > [<e08af61e>] ? ath5k_hw_eeprom_read+0x80/0x116 [ath5k] > [<e08af6df>] ? ath5k_eeprom_read_mac+0x2b/0x8f [ath5k] > [<e08c041c>] ? ath5k_pci_probe+0xbd4/0x1115 [ath5k] > [<c05ab416>] ? local_pci_probe+0xe/0x10 > [<c05abdef>] ? pci_device_probe+0x43/0x66 > [<c062e3f6>] ? driver_probe_device+0xc5/0x1cd > [<c062e587>] ? __device_attach+0x2a/0x2e > [<c062d832>] ? bus_for_each_drv+0x3d/0x67 > [<c062e5f6>] ? device_attach+0x4c/0x6c > [<c062e55d>] ? __device_attach+0x0/0x2e > [<c062d697>] ? bus_probe_device+0x18/0x2d > [<c062c31f>] ? device_add+0x30d/0x49c > [<c075f8ea>] ? pci_bus_assign_resources+0xac/0x3a7 > [<c05a723d>] ? pci_device_add+0xca/0xce > [<c05a6f26>] ? pci_bus_add_device+0xf/0x30 > [<e07480df>] ? eeepc_rfkill_hotplug+0x87/0xc0 [eeepc_laptop] > [<e0748126>] ? eeepc_rfkill_notify+0xe/0x10 [eeepc_laptop] > [<c05df721>] ? acpi_ev_notify_dispatch+0x4f/0x5a > [<c05d205d>] ? acpi_os_execute_deferred+0x1d/0x28 > [<c0448d63>] ? worker_thread+0x13b/0x1b4 > [<c05d2040>] ? acpi_os_execute_deferred+0x0/0x28 > [<c044bd89>] ? autoremove_wake_function+0x0/0x2f > [<c0448c28>] ? worker_thread+0x0/0x1b4 > [<c044ba47>] ? kthread+0x5f/0x64 > [<c044b9e8>] ? kthread+0x0/0x64 > [<c040383e>] ? kernel_thread_helper+0x6/0x10 > Code: 00 76 0d 25 ff ff 00 00 89 d7 89 c2 f3 6c eb 0a ba ad 60 8b c0 > e8 4e fe ff ff 5b 5f 5d c3 55 81 fa ff ff 03 00 89 e5 89 d1 76 04 <89> > 02 eb 1d 81 fa 00 00 01 00 76 09 81 e2 ff ff 00 00 ef eb 0c > EIP: [<c05a098c>] iowrite32+0xd/0x30 SS:ESP 0068:decabd90 > CR2: 000000001ef700e0 > ---[ end trace 2761b2bae95de764 ]--- > > ## Analysis ## > > When the card is disabled, reads will return with all bits set. So we > think the EEPROM is of maximum size, and we end up reading outside the > memory mapping for the device? > > There are two obvious solutions - > > 1. Bounds-check the size read from the EEPROM against the appropriate > memory mapping. > 2. Check for "all ones" when reading the size of the EEPROM. > > > I have a feeling the bounds-checking is more annoying than it sounds. > When I looked at ath5k_hw_eeprom_read(), I saw two types of hardware. > In one, the EEPROM was mapped directly into PCI space, which is what > seems to be happening here. In other hardware, the access was > indirected through just a couple of registers at fixed addresses - in > this case the ROM could be bigger than the PCI space. > > The second option is a bit of a hack though, especially considering > the "XXX:" comment. > > > Best wishes > Alan ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [ath5k] OOPS bisected - ath5k_eeprom_init_header() - "XX: This value is likely too big still, waiting on a better value" @ 2011-06-20 10:34 ` Alan Jenkins 0 siblings, 0 replies; 2+ messages in thread From: Alan Jenkins @ 2011-06-20 10:34 UTC (permalink / raw) To: linux-wireless@vger.kernel.org, linux-kernel Cc: Luis R. Rodriguez, ath5k-devel@lists.ath5k.org [Removed "XXX" from subject in order to get past VGER spam filter] On 20/06/11 11:30, Alan Jenkins wrote: > ## Summary of bug report ## > Confidence: high. > (Bisected. Crash message captured, see end of message). > > Severity: low. > (Users can easily avoid the trigger for this crash). > > > ## Scenario: ## > Asus EeePC 701. > Wireless-toggle button, implemented in firmware, disables / enables > the entire PCI slot. (See platform driver eeepc-laptop). > > ## Expected results: ## > Ath5k (like any other PCI driver), must tolerate removal of PCI > devices without warning, and *never crash*. > > ## Actual results: ## > Ath5k generally handles removal very well, but crashes in a corner > case. This is a regression introduced by v2.6.33 (commit commit > 359207c687cc8f4f9845c8dadd0d6dabad44e584 "ath5k: Fix eeprom checksum > check for custom sized eeproms"). > > ## How to reproduce: ## > Repeatedly press the wireless-toggle button as fast as you can. It > shouldn't take more than about 20 presses before the OOPS occurs. > > > ## Results of git-bisect ## > > commit 359207c687cc8f4f9845c8dadd0d6dabad44e584 > Author: Luis R. Rodriguez <lrodriguez@atheros.com> > Date: Mon Jan 4 10:40:39 2010 -0500 > > ath5k: Fix eeprom checksum check for custom sized eeproms > > 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: stable@kernel.org > 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> > > ... > > - 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++) { > > > ### Example OOPS message ### > > pci 0000:01:00.0: reg 10: [mem 0x00000000-0x0000ffff 64bit] > pci 0000:01:00.0: BAR 0: assigned [mem 0xf8000000-0xf800ffff 64bit] > pci 0000:01:00.0: BAR 0: set to [mem 0xf8000000-0xf800ffff 64bit] (PCI > address [0xf8000000-0xf800ffff] > ath5k 0000:01:00.0: enabling device (0000 -> 0002) > ath5k 0000:01:00.0: PCI INT A -> GSI 18 (level, low) -> IRQ 18 > ath5k 0000:01:00.0: setting latency timer to 64 > ath5k 0000:01:00.0: registered as 'phy9' > BUG: unable to handle kernel paging request at 1ef700e0 > IP: [<c05a098c>] iowrite32+0xd/0x30 > *pde = 00000000 > Oops: 0002 [#1] SMP > last sysfs file: /sys/devices/platform/eeepc/rfkill/rfkill0/uevent > Modules linked in: aes_i586 aes_generic snd_hda_codec_realtek arc4 ecb > snd_hda_intel snd_hda_codec snd_hwdep snd_seq snd_seq_device > ip6t_REJECT ath5k snd_pcm nf_conntrack_ipv6 mac80211 uvcvideo iTCO_wdt > ip6table_filter ath iTCO_vendor_support videodev snd_timer > eeepc_laptop microcode ip6_tables v4l1_compat joydev i2c_i801 cfg80211 > snd sparse_keymap atl2 rfkill soundcore snd_page_alloc ipv6 autofs4 > i915 drm_kms_helper drm usb_storage i2c_algo_bit i2c_core video output > > Pid: 17, comm: kacpi_notify Not tainted 2.6.33.3-85.fc13.i686 #1 701/701 > EIP: 0060:[<c05a098c>] EFLAGS: 00010216 CPU: 0 > EIP is at iowrite32+0xd/0x30 > EAX: 00000020 EBX: de324000 ECX: 1ef700e0 EDX: 1ef700e0 > ESI: 00000020 EDI: decabdba EBP: decabd90 ESP: decabd90 > DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068 > Process kacpi_notify (pid: 17, ti=decaa000 task=dec84c80 > task.ti=decaa000) > Stack: > decabda4 e08af61e de0e0a40 00000007 decabdba decabdc8 e08af6df decabe3a > <0> de324000 00000000 00000000 de0e0a40 00000007 decabe38 decabe54 > e08c041c > <0> 0000000c decabe1c 0000000c e09a0000 de325680 def6a500 1ef6a500 > de324000 > Call Trace: > [<e08af61e>] ? ath5k_hw_eeprom_read+0x80/0x116 [ath5k] > [<e08af6df>] ? ath5k_eeprom_read_mac+0x2b/0x8f [ath5k] > [<e08c041c>] ? ath5k_pci_probe+0xbd4/0x1115 [ath5k] > [<c05ab416>] ? local_pci_probe+0xe/0x10 > [<c05abdef>] ? pci_device_probe+0x43/0x66 > [<c062e3f6>] ? driver_probe_device+0xc5/0x1cd > [<c062e587>] ? __device_attach+0x2a/0x2e > [<c062d832>] ? bus_for_each_drv+0x3d/0x67 > [<c062e5f6>] ? device_attach+0x4c/0x6c > [<c062e55d>] ? __device_attach+0x0/0x2e > [<c062d697>] ? bus_probe_device+0x18/0x2d > [<c062c31f>] ? device_add+0x30d/0x49c > [<c075f8ea>] ? pci_bus_assign_resources+0xac/0x3a7 > [<c05a723d>] ? pci_device_add+0xca/0xce > [<c05a6f26>] ? pci_bus_add_device+0xf/0x30 > [<e07480df>] ? eeepc_rfkill_hotplug+0x87/0xc0 [eeepc_laptop] > [<e0748126>] ? eeepc_rfkill_notify+0xe/0x10 [eeepc_laptop] > [<c05df721>] ? acpi_ev_notify_dispatch+0x4f/0x5a > [<c05d205d>] ? acpi_os_execute_deferred+0x1d/0x28 > [<c0448d63>] ? worker_thread+0x13b/0x1b4 > [<c05d2040>] ? acpi_os_execute_deferred+0x0/0x28 > [<c044bd89>] ? autoremove_wake_function+0x0/0x2f > [<c0448c28>] ? worker_thread+0x0/0x1b4 > [<c044ba47>] ? kthread+0x5f/0x64 > [<c044b9e8>] ? kthread+0x0/0x64 > [<c040383e>] ? kernel_thread_helper+0x6/0x10 > Code: 00 76 0d 25 ff ff 00 00 89 d7 89 c2 f3 6c eb 0a ba ad 60 8b c0 > e8 4e fe ff ff 5b 5f 5d c3 55 81 fa ff ff 03 00 89 e5 89 d1 76 04 <89> > 02 eb 1d 81 fa 00 00 01 00 76 09 81 e2 ff ff 00 00 ef eb 0c > EIP: [<c05a098c>] iowrite32+0xd/0x30 SS:ESP 0068:decabd90 > CR2: 000000001ef700e0 > ---[ end trace 2761b2bae95de764 ]--- > > ## Analysis ## > > When the card is disabled, reads will return with all bits set. So we > think the EEPROM is of maximum size, and we end up reading outside the > memory mapping for the device? > > There are two obvious solutions - > > 1. Bounds-check the size read from the EEPROM against the appropriate > memory mapping. > 2. Check for "all ones" when reading the size of the EEPROM. > > > I have a feeling the bounds-checking is more annoying than it sounds. > When I looked at ath5k_hw_eeprom_read(), I saw two types of hardware. > In one, the EEPROM was mapped directly into PCI space, which is what > seems to be happening here. In other hardware, the access was > indirected through just a couple of registers at fixed addresses - in > this case the ROM could be bigger than the PCI space. > > The second option is a bit of a hack though, especially considering > the "XXX:" comment. > > > Best wishes > Alan ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-06-20 10:34 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <4DFF2156.4000100@googlemail.com>
2011-06-20 10:34 ` [ath5k] OOPS bisected - ath5k_eeprom_init_header() - "XX: This value is likely too big still, waiting on a better value" Alan Jenkins
2011-06-20 10:34 ` Alan Jenkins
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.