pinfo->private_data was a quite subptimal idea

change the decodeas for dcerpc so that it actually works again for dcerpc over smb



svn path=/trunk/; revision=13515

5 files changed, 44 insertions, 62 deletions

diff --git a/epan/dissectors/packet-dcerpc.c b/epan/dissectors/packet-dcerpc.c
index 7516243821..87d0ca56b2 100644
--- a/epan/dissectors/packet-dcerpc.c
+++ b/epan/dissectors/packet-dcerpc.c
@@ -285,7 +285,7 @@ static const value_string reject_status_vals[] = {
 
 
 /* we need to keep track of what transport were used, ie what handle we came
- * in through so we know what kind of pinfo->private_data was passed to us.
+ * in through so we know what kind of pinfo->dce_smb_fid was passed to us.
  */
 /* Value of -1 is reserved for "not DCE packet" in packet_info.dcetransporttype. */
 #define DCE_TRANSPORT_UNKNOWN		0
@@ -810,7 +810,10 @@ static guint
 dcerpc_bind_hash (gconstpointer k)
 {
     const dcerpc_bind_key *key = (const dcerpc_bind_key *)k;
-    return GPOINTER_TO_UINT(key->conv) + key->ctx_id + key->smb_fid;
+    guint hash;
+
+    hash=GPOINTER_TO_UINT(key->conv) + key->ctx_id + key->smb_fid;
+    return hash;
 
 }
 
@@ -1089,7 +1092,7 @@ dissect_dcerpc_double(tvbuff_t *tvb, gint offset, packet_info *pinfo _U_,
 
 
 int
-dissect_dcerpc_uuid_t (tvbuff_t *tvb, gint offset, packet_info *pinfo,
+dissect_dcerpc_uuid_t (tvbuff_t *tvb, gint offset, packet_info *pinfo _U_,
                     proto_tree *tree, char *drep,
                     int hfindex, e_uuid_t *pdata)
 {
@@ -2455,17 +2458,12 @@ dissect_dcerpc_cn_auth (tvbuff_t *tvb, int stub_offset, packet_info *pinfo,
  * as well in the future.
  */
 
-guint16 dcerpc_get_transport_salt (packet_info *pinfo, int transport_type)
+guint16 dcerpc_get_transport_salt (packet_info *pinfo)
 {
-    dcerpc_private_info *priv = (dcerpc_private_info *)pinfo->private_data;
-
-    if (!priv)
-        return 0;	/* Nothing to see here */
-
-    switch(transport_type){
+    switch(pinfo->dcetransporttype){
 	case DCE_CN_TRANSPORT_SMBPIPE:
 	    /* DCERPC over smb */
-	    return priv->fid;
+	    return pinfo->dcetransportsalt;
     }
 
     /* Some other transport... */
@@ -2478,8 +2476,7 @@ guint16 dcerpc_get_transport_salt (packet_info *pinfo, int transport_type)
 
 static void
 dissect_dcerpc_cn_bind (tvbuff_t *tvb, gint offset, packet_info *pinfo, 
-			proto_tree *dcerpc_tree, e_dce_cn_common_hdr_t *hdr,
-			int transport_type)
+			proto_tree *dcerpc_tree, e_dce_cn_common_hdr_t *hdr)
 {
     conversation_t *conv = NULL;
     guint8 num_ctx_items = 0;
@@ -2524,7 +2521,6 @@ dissect_dcerpc_cn_bind (tvbuff_t *tvb, gint offset, packet_info *pinfo,
       /* (if we have multiple contexts, this might cause "decode as"
        *  to behave unpredictably) */
       pinfo->dcectxid = ctx_id;
-      pinfo->dcetransporttype = transport_type;
 
       if (dcerpc_tree) {
 	      proto_item *ctx_item;
@@ -2602,7 +2598,7 @@ dissect_dcerpc_cn_bind (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 	        key = g_mem_chunk_alloc (dcerpc_bind_key_chunk);
         	key->conv = conv;
         	key->ctx_id = ctx_id;
-        	key->smb_fid = dcerpc_get_transport_salt(pinfo, transport_type);
+        	key->smb_fid = dcerpc_get_transport_salt(pinfo);
 
         	value = g_mem_chunk_alloc (dcerpc_bind_value_chunk);
         	value->uuid = if_id;
@@ -3143,7 +3139,7 @@ dcerpc_add_conv_to_bind_table(decode_dcerpc_bind_values_t *binding)
 static void
 dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo, 
 			proto_tree *dcerpc_tree, proto_tree *tree,
-			e_dce_cn_common_hdr_t *hdr, int transport_type)
+			e_dce_cn_common_hdr_t *hdr)
 {
     conversation_t *conv;
     guint16 ctx_id;
@@ -3166,7 +3162,6 @@ dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
     /* save context ID for use with dcerpc_add_conv_to_bind_table() */
     pinfo->dcectxid = ctx_id;
-    pinfo->dcetransporttype = transport_type;
 
     if (check_col (pinfo->cinfo, COL_INFO)) {
         col_append_fstr (pinfo->cinfo, COL_INFO, " opnum: %u ctx_id: %u",
@@ -3224,7 +3219,7 @@ dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
 		bind_key.conv=conv;
 		bind_key.ctx_id=ctx_id;
-		bind_key.smb_fid=dcerpc_get_transport_salt(pinfo, transport_type);
+		bind_key.smb_fid=dcerpc_get_transport_salt(pinfo);
 
 		if((bind_value=g_hash_table_lookup(dcerpc_binds, &bind_key)) ){
 			if(!(hdr->flags&PFC_FIRST_FRAG)){
@@ -3233,7 +3228,7 @@ dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
 				call_key.conv=conv;
 				call_key.call_id=hdr->call_id;
-				call_key.smb_fid=dcerpc_get_transport_salt(pinfo, transport_type);
+				call_key.smb_fid=dcerpc_get_transport_salt(pinfo);
 				if((call_value=g_hash_table_lookup(dcerpc_cn_calls, &call_key))){
 					new_matched_key = g_mem_chunk_alloc(dcerpc_matched_key_chunk);
 					*new_matched_key = matched_key;
@@ -3252,7 +3247,7 @@ dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 				call_key=g_mem_chunk_alloc (dcerpc_cn_call_key_chunk);
 				call_key->conv=conv;
 				call_key->call_id=hdr->call_id;
-				call_key->smb_fid=dcerpc_get_transport_salt(pinfo, transport_type);
+				call_key->smb_fid=dcerpc_get_transport_salt(pinfo);
 
 				/* if there is already a matching call in the table
 				   remove it so it is replaced with the new one */
@@ -3287,7 +3282,7 @@ dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo,
             /* handoff this call */
 	    di->conv = conv;
 	    di->call_id = hdr->call_id;
-	    di->smb_fid = dcerpc_get_transport_salt(pinfo, transport_type);
+	    di->smb_fid = dcerpc_get_transport_salt(pinfo);
 	    di->ptype = PDU_REQ;
 	    di->call_data = value;
 		di->hf_index = -1;
@@ -3313,7 +3308,7 @@ dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 static void
 dissect_dcerpc_cn_resp (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 			proto_tree *dcerpc_tree, proto_tree *tree,
-			e_dce_cn_common_hdr_t *hdr, int transport_type)
+			e_dce_cn_common_hdr_t *hdr)
 {
     dcerpc_call_value *value = NULL;
     conversation_t *conv;
@@ -3330,7 +3325,6 @@ dissect_dcerpc_cn_resp (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
     /* save context ID for use with dcerpc_add_conv_to_bind_table() */
     pinfo->dcectxid = ctx_id;
-    pinfo->dcetransporttype = transport_type;
 
     if (check_col (pinfo->cinfo, COL_INFO)) {
         col_append_fstr (pinfo->cinfo, COL_INFO, " ctx_id: %u", ctx_id);
@@ -3370,7 +3364,7 @@ dissect_dcerpc_cn_resp (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
 		call_key.conv=conv;
 		call_key.call_id=hdr->call_id;
-		call_key.smb_fid=dcerpc_get_transport_salt(pinfo, transport_type);
+		call_key.smb_fid=dcerpc_get_transport_salt(pinfo);
 
 		if((call_value=g_hash_table_lookup(dcerpc_cn_calls, &call_key))){
 			/* extra sanity check,  only match them if the reply
@@ -3394,7 +3388,7 @@ dissect_dcerpc_cn_resp (tvbuff_t *tvb, gint offset, packet_info *pinfo,
             /* handoff this call */
 	    di->conv = conv;
 	    di->call_id = hdr->call_id;
-	    di->smb_fid = dcerpc_get_transport_salt(pinfo, transport_type);
+	    di->smb_fid = dcerpc_get_transport_salt(pinfo);
 	    di->ptype = PDU_RESP;
 	    di->call_data = value;
 
@@ -3427,8 +3421,7 @@ dissect_dcerpc_cn_resp (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
 static void
 dissect_dcerpc_cn_fault (tvbuff_t *tvb, gint offset, packet_info *pinfo,
-                         proto_tree *dcerpc_tree, e_dce_cn_common_hdr_t *hdr,
-			 int transport_type)
+                         proto_tree *dcerpc_tree, e_dce_cn_common_hdr_t *hdr)
 {
     dcerpc_call_value *value = NULL;
     conversation_t *conv;
@@ -3454,7 +3447,6 @@ dissect_dcerpc_cn_fault (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
     /* save context ID for use with dcerpc_add_conv_to_bind_table() */
     pinfo->dcectxid = ctx_id;
-    pinfo->dcetransporttype = transport_type;
 
     if (check_col (pinfo->cinfo, COL_INFO)) {
         col_append_fstr (pinfo->cinfo, COL_INFO,
@@ -3493,7 +3485,7 @@ dissect_dcerpc_cn_fault (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 
 		call_key.conv=conv;
 		call_key.call_id=hdr->call_id;
-		call_key.smb_fid=dcerpc_get_transport_salt(pinfo, transport_type);
+		call_key.smb_fid=dcerpc_get_transport_salt(pinfo);
 
 		if((call_value=g_hash_table_lookup(dcerpc_cn_calls, &call_key))){
 			new_matched_key = g_mem_chunk_alloc(dcerpc_matched_key_chunk);
@@ -3515,7 +3507,7 @@ dissect_dcerpc_cn_fault (tvbuff_t *tvb, gint offset, packet_info *pinfo,
             /* handoff this call */
 	    di->conv = conv;
 	    di->call_id = hdr->call_id;
-	    di->smb_fid = dcerpc_get_transport_salt(pinfo, transport_type);
+	    di->smb_fid = dcerpc_get_transport_salt(pinfo);
 	    di->ptype = PDU_FAULT;
 	    di->call_data = value;
 
@@ -3690,8 +3682,7 @@ dissect_dcerpc_cn_fault (tvbuff_t *tvb, gint offset, packet_info *pinfo,
  */
 static gboolean
 dissect_dcerpc_cn (tvbuff_t *tvb, int offset, packet_info *pinfo,
-                   proto_tree *tree, gboolean can_desegment, int *pkt_len,
-		   int transport_type)
+                   proto_tree *tree, gboolean can_desegment, int *pkt_len)
 {
     static const guint8 nulls[4] = { 0 };
     int start_offset;
@@ -3837,7 +3828,7 @@ dissect_dcerpc_cn (tvbuff_t *tvb, int offset, packet_info *pinfo,
     switch (hdr.ptype) {
     case PDU_BIND:
     case PDU_ALTER:
-        dissect_dcerpc_cn_bind (tvb, offset, pinfo, dcerpc_tree, &hdr, transport_type);
+        dissect_dcerpc_cn_bind (tvb, offset, pinfo, dcerpc_tree, &hdr);
         break;
 
     case PDU_BIND_ACK:
@@ -3854,15 +3845,15 @@ dissect_dcerpc_cn (tvbuff_t *tvb, int offset, packet_info *pinfo,
         break;
 
     case PDU_REQ:
-        dissect_dcerpc_cn_rqst (tvb, offset, pinfo, dcerpc_tree, tree, &hdr, transport_type);
+        dissect_dcerpc_cn_rqst (tvb, offset, pinfo, dcerpc_tree, tree, &hdr);
         break;
 
     case PDU_RESP:
-        dissect_dcerpc_cn_resp (tvb, offset, pinfo, dcerpc_tree, tree, &hdr, transport_type);
+        dissect_dcerpc_cn_resp (tvb, offset, pinfo, dcerpc_tree, tree, &hdr);
         break;
 
     case PDU_FAULT:
-        dissect_dcerpc_cn_fault (tvb, offset, pinfo, dcerpc_tree, &hdr, transport_type);
+        dissect_dcerpc_cn_fault (tvb, offset, pinfo, dcerpc_tree, &hdr);
         break;
 
     case PDU_BIND_NAK:
@@ -3906,7 +3897,8 @@ dissect_dcerpc_cn_pk (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
      * Only one PDU per transport packet, and only one transport
      * packet per PDU.
      */
-    if (!dissect_dcerpc_cn (tvb, 0, pinfo, tree, FALSE, NULL, DCE_TRANSPORT_UNKNOWN)) {
+    pinfo->dcetransporttype=DCE_TRANSPORT_UNKNOWN;
+    if (!dissect_dcerpc_cn (tvb, 0, pinfo, tree, FALSE, NULL)) {
         /*
          * It wasn't a DCERPC PDU.
          */
@@ -3926,7 +3918,7 @@ dissect_dcerpc_cn_pk (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
  * to be able to know what kind of private_data structure to expect.
  */
 static gboolean
-dissect_dcerpc_cn_bs_body (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, int transport_type)
+dissect_dcerpc_cn_bs_body (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
     volatile int offset = 0;
     int pdu_len;
@@ -3950,8 +3942,7 @@ dissect_dcerpc_cn_bs_body (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
         is_dcerpc_pdu = FALSE;
         TRY {
             is_dcerpc_pdu = dissect_dcerpc_cn (tvb, offset, pinfo, tree,
-                                               dcerpc_cn_desegment, &pdu_len,
-					       transport_type);
+                                               dcerpc_cn_desegment, &pdu_len);
         } CATCH(BoundsError) {
             RETHROW;
         } CATCH(ReportedBoundsError) {
@@ -3988,13 +3979,15 @@ dissect_dcerpc_cn_bs_body (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree,
 static gboolean
 dissect_dcerpc_cn_bs (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-	return dissect_dcerpc_cn_bs_body(tvb, pinfo, tree, DCE_TRANSPORT_UNKNOWN);
+	pinfo->dcetransporttype=DCE_TRANSPORT_UNKNOWN;
+	return dissect_dcerpc_cn_bs_body(tvb, pinfo, tree);
 }
 
 static gboolean
 dissect_dcerpc_cn_smbpipe (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree)
 {
-	return dissect_dcerpc_cn_bs_body(tvb, pinfo, tree, DCE_CN_TRANSPORT_SMBPIPE);
+	pinfo->dcetransporttype=DCE_CN_TRANSPORT_SMBPIPE;
+	return dissect_dcerpc_cn_bs_body(tvb, pinfo, tree);
 }
 
 
@@ -4770,8 +4763,11 @@ dcerpc_init_protocol (void)
 	/* structures and data for BIND */
 	if (dcerpc_binds){
 		g_hash_table_destroy (dcerpc_binds);
+		dcerpc_binds=NULL;
+	} 
+	if(!dcerpc_binds){
+		dcerpc_binds = g_hash_table_new (dcerpc_bind_hash, dcerpc_bind_equal);
 	}
-	dcerpc_binds = g_hash_table_new (dcerpc_bind_hash, dcerpc_bind_equal);
 
 	if (dcerpc_bind_key_chunk){
 		g_mem_chunk_destroy (dcerpc_bind_key_chunk);
diff --git a/epan/dissectors/packet-dcerpc.h b/epan/dissectors/packet-dcerpc.h
index f3931c13ac..1e4e8427e6 100644
--- a/epan/dissectors/packet-dcerpc.h
+++ b/epan/dissectors/packet-dcerpc.h
@@ -247,17 +247,6 @@ value_string *value_string_from_subdissectors(dcerpc_sub_dissector *sd);
 /* try to get protocol name registered for this uuid */
 gchar *dcerpc_get_uuid_name(e_uuid_t *uuid, guint16 ver);
 
-/* Private data structure to pass to DCERPC dissector. This is used to
-   pass transport specific information down to the dissector from the
-   dissector that parsed this encapsulated calls. 
-   When it comes to DCERPC over SMB the only thing we really want to pass
-   on is the FID.
-*/
-
-typedef struct _dcerpc_private_info {
-	guint16 fid;
-} dcerpc_private_info;
-
 /* Private data passed to subdissectors from the main DCERPC dissector. */
 typedef struct _dcerpc_call_value {
     e_uuid_t uuid;
@@ -361,7 +350,7 @@ struct _dcerpc_bind_value *
 dcerpc_add_conv_to_bind_table(decode_dcerpc_bind_values_t *binding);
 
 guint16 
-dcerpc_get_transport_salt (packet_info *pinfo, int transport_type);
+dcerpc_get_transport_salt (packet_info *pinfo);
 
 /* Authentication services */
 
diff --git a/epan/dissectors/packet-smb-pipe.c b/epan/dissectors/packet-smb-pipe.c
index 8d7ca90ee3..bc1ffea3ed 100644
--- a/epan/dissectors/packet-smb-pipe.c
+++ b/epan/dissectors/packet-smb-pipe.c
@@ -3240,7 +3240,6 @@ gboolean
 dissect_pipe_dcerpc(tvbuff_t *d_tvb, packet_info *pinfo, proto_tree *parent_tree,
     proto_tree *tree, guint32 fid)
 {
-	dcerpc_private_info dcerpc_priv;
 	smb_info_t *smb_priv = (smb_info_t *)pinfo->private_data;
 	gboolean result=0;
 	gboolean save_fragmented;
@@ -3249,10 +3248,7 @@ dissect_pipe_dcerpc(tvbuff_t *d_tvb, packet_info *pinfo, proto_tree *parent_tree
 	fragment_data *fd_head;
 	tvbuff_t *new_tvb;
 
-	dcerpc_priv.fid = fid;
-
-	pinfo->private_data = &dcerpc_priv;
-
+	pinfo->dcetransportsalt = fid;
 
 	/*
 	 * Offer desegmentation service to DCERPC if we have all the
diff --git a/epan/packet_info.h b/epan/packet_info.h
index c37ae13511..624314b893 100644
--- a/epan/packet_info.h
+++ b/epan/packet_info.h
@@ -124,9 +124,10 @@ typedef struct _packet_info {
   guint16 dst_idx;              /* Dest port index (Cisco MDS-specific) */
   guint16 vsan;                 /* Fibre channel/Cisco MDS-specific */
   guint16 dcectxid;             /* Context ID (DCERPC-specific) */
-  int     dcetransporttype;     /* Transport type, tag for private_data (DCERPC-specific)
+  guint16 dcetransporttype;     /* Transport type
                                  * Value -1 means "not a DCERPC packet"
                                  */
+  guint16 dcetransportsalt;	/* fid: if transporttype==DCE_CN_TRANSPORT_SMBPIPE */
   guint32 ppid[MAX_NUMBER_OF_PPIDS]; /* The first NUMBER_OF_PPIDS PPIDS which are present
                                       * in the SCTP packet
                                       */
diff --git a/gtk/decode_as_dcerpc.c b/gtk/decode_as_dcerpc.c
index a58c838938..f5e91efb66 100644
--- a/gtk/decode_as_dcerpc.c
+++ b/gtk/decode_as_dcerpc.c
@@ -377,7 +377,7 @@ decode_dcerpc_add_page (packet_info *pinfo)
     binding->port_a = pinfo->srcport;
     binding->port_b = pinfo->destport;
     binding->ctx_id = pinfo->dcectxid;
-    binding->smb_fid = dcerpc_get_transport_salt(pinfo, pinfo->dcetransporttype);
+    binding->smb_fid = dcerpc_get_transport_salt(pinfo);
     binding->ifname = NULL;
     /*binding->uuid = NULL;*/
     binding->ver = 0;