Some of the memory blocks kcalloc'ed in ath5k_eeprom_convert_pcal_info_*() are not freed even when module ath5k is unloaded. To reproduce the problem, it is enough to load ath5k.ko module and then unload it again. Here is the trace for one iteration of the outer 'for' loop with my comments. ath5k_eeprom_convert_pcal_info_2413(), drivers/net/wireless/ath/ath5k/eeprom.c:1148: // Allocation of chinfo[pier].pd_curves, line 1153. Room for 4 elements of // type struct ath5k_pdgain_info is created. called___kmalloc: (.text+0x212f) arguments: (48, 80d0), result: ab3ad780 // The inner loop, iter #0, allocation of pd->pd_step and pd->pd_pwr // for chinfo[pier].pd_curves[pdgain_idx[0]] (lines 1176 and 1182) called___kmalloc: (.text+0x212f) arguments: (4, 80d0), result: ab1c3f68 called___kmalloc: (.text+0x212f) arguments: (8, 80d0), result: ab1c3f70 // The inner loop, iter #1, allocation of pd->pd_step and pd->pd_pwr // for chinfo[pier].pd_curves[pdgain_idx[1]] (lines 1176 and 1182) called___kmalloc: (.text+0x212f) arguments: (5, 80d0), result: ab1c3f78 called___kmalloc: (.text+0x212f) arguments: (10, 80d0), result: ab17ce80 <...> Here is the trace for one iteration of the outer 'for' loop, ath5k_eeprom_free_pcal_info(), drivers/net/wireless/ath/ath5k/eeprom.c:1564 // The inner loop, iter #0, line 1573 called_kfree: (.text+0x20be) arguments: ((null)) called_kfree: (.text+0x20c6) arguments: ((null)) // The inner loop, iter #1, line 1573 called_kfree: (.text+0x20be) arguments: (ab1c3f78) called_kfree: (.text+0x20c6) arguments: (ab17ce80) // Deallocation of chinfo[pier].pd_curves array called_kfree: (.text+0x20de) arguments: (ab3ad780) <...> Same for other iterations of the outer 'for' loops in ath5k_eeprom_convert_pcal_info_*() and ath5k_eeprom_free_pcal_info(). It looks like ath5k_eeprom_free_pcal_info() tries to free 'pd_step' and 'pd_pwr' fields of wrong elements. On my system, chinfo[pier].pd_curves[] is an array of 4 (AR5K_EEPROM_N_PD_CURVES) elements, only 2 of which are actually used, #1 and #2, which is probably OK. The elements #0 and #3 remain filled with zeros. Consider line 1162: for (pdg = 0; pdg < ee->ee_pd_gains[mode]; pdg++) { u8 idx = pdgain_idx[pdg]; struct ath5k_pdgain_info *pd = &chinfo[pier].pd_curves[idx]; ee->ee_pd_gains[mode] is 2 on my system and pdgain_idx[] array is {1; 2}. From the code in eeprom.c, it looks like only these elements of chinfo[pier].pd_curves are actually used for calibration, etc. Only ath5k_eeprom_free_pcal_info() works in a different way. It tries to free 'pd_step' and 'pd_pwr' for the first two elements of chinfo[pier].pd_curves instead of the elements that were actually allocated before: for (pdg = 0; pdg < ee->ee_pd_gains[mode]; pdg++) { struct ath5k_pdgain_info *pd = &chinfo[pier].pd_curves[pdg]; if (pd != NULL) { kfree(pd->pd_step); kfree(pd->pd_pwr); } } That is, it processes pd_curves[0] and pd_curves[1] rather than pd_curves[pdgain_idx[0]] and pd_curves[pdgain_idx[1]]. There are probably two ways to fix this issue: we can just process all 4 elements of pd_curves[] or we can process only the elements actually used (like it is done in other places in eeprom.c). Here is the patch for the first variant (I would also remove the inner conditional as 'pd' is never NULL anyway): eeprom.c:1568: - for (pdg = 0; pdg < ee->ee_pd_gains[mode]; pdg++) { + for (pdg = 0; pdg < AR5K_EEPROM_N_PD_CURVES; pdg++) { struct ath5k_pdgain_info *pd = &chinfo[pier].pd_curves[pdg]; - if (pd != NULL) { kfree(pd->pd_step); kfree(pd->pd_pwr); - } } Here is the patch for the second variant: eeprom.c:1541: struct ath5k_chan_pcal_info *chinfo; + u8 *pdgain_idx = ee->ee_pdc_to_idx[mode]; u8 pier, pdg; eeprom.c:1568: for (pdg = 0; pdg < ee->ee_pd_gains[mode]; pdg++) { - struct ath5k_pdgain_info *pd = - &chinfo[pier].pd_curves[pdg]; + u8 idx = pdgain_idx[pdg]; + struct ath5k_pdgain_info *pd = + &chinfo[pier].pd_curves[idx]; - if (pd != NULL) { kfree(pd->pd_step); kfree(pd->pd_pwr); - } } I guess the result is the same in both these variants.
Nick and Bob, any comment on this one?
Created attachment 61212 [details] eugene's patch 1 Agreed. I lean towards the first variant for simplicity despite the few extra calls to kfree. Here's a patch against latest w-t. Eugene, let me know if the attribution is satisfactory (email address correct etc) and I'll send it on to the ML. Thanks for the thorough bug report!
I'm glad it was helpful. Yes, the attribution is OK.
Bob, send the patch to linux-wireless? :-)
Weird, I never got the June 8th response in email. Ok, rebasing and sending it on now.