Bill Meier
changed
bug 8912
What |
Removed |
Added |
Status |
RESOLVED
|
INCOMPLETE
|
Resolution |
FIXED
|
---
|
Comment # 14
on bug 8912
from Bill Meier
Some notes about the following part of the patch for wiretap/vwr.c in
vwr_get_fpga_version():
if ((rec_size > vVW510021_W_STATS_LEN) && (fpga_version == 1000)) {
/* Check the version of the FPGA */
if (header[8] == 48)
fpga_version = S3_W_FPGA;
}
if ((rec_size > vVW510021_W_STATS_LEN) && (fpga_version == 1000)) {
/* Check the version of the FPGA */
if (header[8] == 48)
fpga_version = S3_W_FPGA;
}
1. The code is duplicated. Can the duplicated code be removed or was the second
if (...) intended to be something different ?
2. More importantly: with the addition of this test, at least 1 capture file
I have is incorrectly identified as a vwr capture file
(and then a crash occurs (see below) when the file is read for dissection).
Looking at the code in vwr.c: vwr_get_fpga_version(): this test seems
a bit weak.
Essentially: Test for one of hex 21/31/C1/8B/FE in the 0th byte
and hex 30 in the 8th byte of a 16 byte 'header'.
Looking a bit more deeply at the code: A search is done through the
file looking for certain patterns and skipping stuff (headers/data) which
don't match the pattern. Would it be possible to do some initial validation
of each "header" (e.g., checking for valid 'cmd' values) so that there would
be less likelihood of having to read a lot of data from the file before
deciding that it's not a vwr file.
3. The crash occurred in 'vwr_read_rec_data_wlan' in the following code:
if ( rec_size < ((int)common_fields.vw_msdu_length + (int)vwr->STATS_LEN) )
/*something's been truncated, DUMP AS-IS*/
memcpy(&data_ptr[bytes_written], &rec[mpdu_offset],
common_fields.vw_msdu_length);
I think the problem is copying something greater than 'rec_size' is ng.
(I'll see if I can share the capture file I have which causes the crash).
Any help/patches/... you can provide will be much appreciated.
You are receiving this mail because:
- You are watching all bug changes.