Wireshark mailing list archives
Re: Possible bug in dissect_pipe_dcerpc() (packet-smb-pipe.c)
From: Pascal Quantin <pascal.quantin () gmail com>
Date: Sat, 28 Jul 2012 00:32:19 +0200
2012/7/28 Evan Huus <eapache () gmail com>
On Fri, Jul 27, 2012 at 6:14 PM, Pascal Quantin <pascal.quantin () gmail com> 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() functionisnever 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 ifthedissector 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
Hi Evan, thanks for the reminder: I missed this bug ;) I guess Guy would be the best able to answer this question (if he remembers the code he wrote in 2003!). Regards, Pascal.
___________________________________________________________________________ Sent via: Wireshark-dev mailing list <wireshark-dev () wireshark org> Archives: http://www.wireshark.org/lists/wireshark-dev Unsubscribe: https://wireshark.org/mailman/options/wireshark-dev mailto:wireshark-dev-request () wireshark org?subject=unsubscribe
Current thread:
- Possible bug in dissect_pipe_dcerpc() (packet-smb-pipe.c) Pascal Quantin (Jul 27)
- Re: Possible bug in dissect_pipe_dcerpc() (packet-smb-pipe.c) Evan Huus (Jul 27)
- Re: Possible bug in dissect_pipe_dcerpc() (packet-smb-pipe.c) Pascal Quantin (Jul 27)
- Re: Possible bug in dissect_pipe_dcerpc() (packet-smb-pipe.c) Evan Huus (Jul 27)