[Bugme-new] [Bug 32942] New: (ath5k) Not all elements of chinfo[pier].pd_curves[] are freed
bugzilla-daemon at bugzilla.kernel.org
bugzilla-daemon at bugzilla.kernel.org
Sat Apr 9 05:10:44 PDT 2011
https://bugzilla.kernel.org/show_bug.cgi?id=32942
Summary: (ath5k) Not all elements of chinfo[pier].pd_curves[]
are freed
Product: Drivers
Version: 2.5
Kernel Version: 2.6.39-rc2
Platform: All
OS/Version: Linux
Tree: Mainline
Status: NEW
Severity: normal
Priority: P1
Component: network-wireless
AssignedTo: drivers_network-wireless at kernel-bugs.osdl.org
ReportedBy: dame_eugene at mail.ru
Regression: No
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.
--
Configure bugmail: https://bugzilla.kernel.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are on the CC list for the bug.
More information about the Bugme-new
mailing list