Wireshark-bugs: [Wireshark-bugs] [Bug 8787] 9P dissector - compute fids on first visit

Date: Wed, 31 Jul 2013 21:31:14 +0000

Comment # 18 on bug 8787 from
Sorry for the radio silence...  Been busy working on other things.

Yes, tcp_dissect_pdus() is probably what you want to use to handle, well, TCP
being TCP.

Other comments:

This change shouldn't be applied:

        struct _9p_hashval *val;
        val = (struct _9p_hashval *)g_malloc(sizeof(struct _9p_hashval));

-       val->data = ""
+       if (len)
+               val->data = ""

Since with it val->data could end up uninitialized.  (g_malloc() will return
NULL if len==0.)


Another thing which has changed since this started: the ep_ and se_ allocators
have been deprecated.  Ideally those allocations would be replaced with wmem
(see doc/README.wmem).


In general the changes look good, but when I fuzz tested them for a while I got
a core dump:

~~~
#0  0x00007f0aedc3b4c9 in emem_tree_lookup32_le (se_tree=0x1e1dc90, key=12) at
emem.c:1340
#1  0x00007f0aedca3e44 in conv_get_fid (pinfo=0x7fff66571148, fid=4294967295)
at packet-9p.c:1162
#2  0x00007f0aedca7992 in dissect_9P (tvb=0x1bfca80, pinfo=0x7fff66571148,
tree=0x1dd2330, data="" at packet-9p.c:2077
#3  0x00007f0aedc464ef in call_dissector_through_handle (handle=0x1a95c40,
tvb=0x1bfca80, pinfo=0x7fff66571148, tree=0x1dd2330, data="" at packet.c:432
#4  0x00007f0aedc46ccd in call_dissector_work (handle=0x1a95c40,
tvb=tvb@entry=0x1bfca80, pinfo_arg=pinfo_arg@entry=0x7fff66571148,
tree=tree@entry=0x1dd2330, add_proto_name=add_proto_name@entry=1, 
    data="" at packet.c:530
#5  0x00007f0aedc47520 in dissector_try_uint_new (sub_dissectors=<optimized
out>, uint_val=564, tvb=tvb@entry=0x1bfca80, pinfo=pinfo@entry=0x7fff66571148,
tree=tree@entry=0x1dd2330, 
    add_proto_name=add_proto_name@entry=1, data="" at packet.c:947
~~~

Valrind had this to say about the offending capture file:

~~~
==25503== Invalid read of size 8
==25503==    at 0x61E24C0: emem_tree_lookup32_le (emem.c:1338)
==25503==    by 0x624AE43: conv_get_fid (packet-9p.c:1162)
==25503==    by 0x624E991: dissect_9P (packet-9p.c:2077)
==25503==    by 0x61ED4EE: call_dissector_through_handle (packet.c:432)
==25503==    by 0x61EDCCC: call_dissector_work (packet.c:530)
==25503==    by 0x61EE51F: dissector_try_uint_new (packet.c:947)
==25503==    by 0x61EE576: dissector_try_uint (packet.c:973)
==25503==    by 0x685C6D4: decode_tcp_ports (packet-tcp.c:3848)
==25503==    by 0x685CB21: process_tcp_payload (packet-tcp.c:3921)
==25503==    by 0x685D0FC: dissect_tcp_payload (packet-tcp.c:1746)
==25503==    by 0x685EB54: dissect_tcp (packet-tcp.c:4758)
==25503==    by 0x61ED4A7: call_dissector_through_handle (packet.c:436)
==25503==  Address 0xbe7abc8 is 20 bytes after a block of size 4 alloc'd
==25503==    at 0x4A0887C: malloc (vg_replace_malloc.c:270)
==25503==    by 0x3D3904D68E: g_malloc (in /usr/lib64/libglib-2.0.so.0.3400.2)
==25503==    by 0x624AA4A: _9p_hash_new_val (packet-9p.c:1031)
==25503==    by 0x624AC30: conv_set_version (packet-9p.c:1100)
==25503==    by 0x624B3D4: dissect_9P (packet-9p.c:1286)
==25503==    by 0x61ED4EE: call_dissector_through_handle (packet.c:432)
==25503==    by 0x61EDCCC: call_dissector_work (packet.c:530)
==25503==    by 0x61EE51F: dissector_try_uint_new (packet.c:947)
==25503==    by 0x61EE576: dissector_try_uint (packet.c:973)
==25503==    by 0x685C6D4: decode_tcp_ports (packet-tcp.c:3848)
==25503==    by 0x685CB21: process_tcp_payload (packet-tcp.c:3921)
==25503==    by 0x685D0FC: dissect_tcp_payload (packet-tcp.c:1746)
~~~

I suspect the problem is coming from val->data either being an se_tree or a
_9p_version and it's getting mixed up.


You are receiving this mail because:
  • You are watching all bug changes.