Great! This fix should be applied to all dissectors using reassembly. The
"wicked crash" I mentioned in another mail today is very likely to be linked
to an empty tvbuff_t because of a retransmitted PDU. This however requires a
similar fix in the WTP dissector.
Should the README.developer be updated so the "common code" from tvb =
process_reassembled_data() is used instead of the tvb = tvb_new_real_data()
and other calls?
Regards,
Olivier
PS see also some comments below :)
| -----Original Message-----
| From: Guy Harris
|
| On Dec 19, 2003, at 3:35 AM, Biot Olivier wrote:
|
| > Attached packet causes Ethereal to crash in malloc() in cygwin
| > run-time DLLs,or to exit upon a failed assertion:
|
| I checked in a change to fix some potential problems in the SMPP
| dissector that I found when looking at this (none of which were the
| real problem), and to use "process_reassembled_data()" for
| reassembled
| packets, which eliminates the crash (and uses common code).
|
| The underlying problem is that
|
| 1) some 802.11 network interfaces do reassembly of
| fragmented 802.11
| packets even when you're doing a monitor-mode capture;
|
| 2) when they do that, they supply the packet without the "more
| fragments" indicator set *and* with a non-zero fragment
| number, so the
| packet is indistinguishable from the last fragment of an
| *unreassembled* packet;
|
| 3) the reassembly code works around this by, if the
| first fragment
| seen for a packet is the last fragment, treating that as the only
| fragment even if the fragment number is non-zero;
|
| 4) your single packet was the last fragment of a
| fragmented SMPP message.
Correct: it was the 2nd SMS message of a 2-SMS WSP SDU containing an MMS
notification.
| The code wasn't checking for "fd_sm->len" being zero, which it is in
| the special case that the reassembly code does for 802.11;
| that created
| a tvbuff with a zero length, which caused a crash in the GUI code.
| (Such a tvbuff is treated as legal so that we can arrange to throw a
| "short frame" exception if a packet was cut off by the
| snapshot length
| at the beginning of some protocol's data, so that there was data for
| that protocol but the amount of it that we captured is zero.)
|
| "process_reassembled_data()" does that check, thus avoiding
| the crash.
| (Using "process_reassembled_data()" also means we use common code.)
|
| However, that means it treats the packet as reassembled and
| hands it to the WSP dissector.
I have noticed this; I'd need to prevent this. Does the "common code"
provide a means of doing this?
| We should arrange that the aforementioned hack is
| done *ONLY* for 802.11; I'll look at doing that.