Wireshark · Wireshark-dev: [Wireshark-dev] Possible bug in dissect_pipe_dcerpc() (packet-smb-pipe.c)

[Pascal Quantin <pascal.quantin@xxxxxxxxx>]

Hi all,

while working on fixing a few Clang warnings in packet-smb-pipe.c, I discovered that the hash_key variable in dissect_pipe_dcerpc() function is never used.
According to the nice comment Guy put in revision 7777 above the variable assignment, I have the feeling that it is actually useful in case of reassembly in both directions and that the following patch should be applied:

Index: packet-smb-pipe.c
===================================================================
--- packet-smb-pipe.c   (révision 44082)
+++ packet-smb-pipe.c   (copie de travail)
@@ -3348,7 +3348,7 @@
                 * in this direction, by searching for its reassembly

                 * structure.
                 */
-               fd_head=fragment_get(pinfo, fid, dcerpc_fragment_table);
+               fd_head=fragment_get(pinfo, hash_key, dcerpc_fragment_table);
                if(!fd_head){

                        /* No reassembly, so this is a new pdu. check if the
                           dissector wants us to reassemble it or if we
@@ -3370,11 +3370,11 @@
                           more data ?

                        */
                        if(pinfo->desegment_len){
-                               fragment_add_check(d_tvb, 0, pinfo, fid,
+                               fragment_add_check(d_tvb, 0, pinfo, hash_key,

                                        dcerpc_fragment_table,
                                        dcerpc_reassembled_table,
                                        0, reported_len, TRUE);
-                               fragment_set_tot_len(pinfo, fid,

+                               fragment_set_tot_len(pinfo, hash_key,
                                        dcerpc_fragment_table,
                                        pinfo->desegment_len+reported_len);

                        }
@@ -3392,7 +3392,7 @@
                while(fd_head->next){
                        fd_head=fd_head->next;
                }
-               fd_head=fragment_add_check(d_tvb, 0, pinfo, fid,

+               fd_head=fragment_add_check(d_tvb, 0, pinfo, hash_key,
                        dcerpc_fragment_table, dcerpc_reassembled_table,
                        fd_head->offset+fd_head->len,
                        reported_len, TRUE);

@@ -3426,7 +3426,7 @@
         * up so that we don't have to distinguish between the first
         * pass and subsequent passes?
         */
-       fd_head=fragment_add_check(d_tvb, 0, pinfo, fid, dcerpc_fragment_table,

+       fd_head=fragment_add_check(d_tvb, 0, pinfo, hash_key, dcerpc_fragment_table,
            dcerpc_reassembled_table, 0, 0, TRUE);
        if(!fd_head){
                /* we didnt find it, try any of the heuristic dissectors

Am I right or should we remove the hash_key variable?

Regards,
Pascal.