Wireshark-dev: Re: [Wireshark-dev] Possible bug in dissect_pipe_dcerpc() (packet-smb-pipe.c)
On Fri, Jul 27, 2012 at 6:14 PM, Pascal Quantin
<pascal.quantin@xxxxxxxxx> wrote:
> 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.
I'm not sure what the correct course of action is, but there's already
a bug for it. I found the same issue with CppCheck a few weeks ago :)
https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=7451
Cheers,
Evan
