the way we pass dcerpc strings from deep down in helpers to high level dissector functions (dcv->private_data) for things such as strings and sids is a mess and very difficult to handle without a lot of memory leakage.

the biggest problem in changing this is the dcv->private_data usage.


add a dcv->se_data which can keep data around from a request to a response and use this to change the LSA/OpenPolicy2 servername passing from request to response as a test pattern of moving all users of dcv->private data over to use dcv->se_data.

once all users are migrated over we can then change the dcv->private data pointer to be of ep scope and thus not need an explicit free (which is quite difficult and it is quite difficult in the old semantics to know WHEN we need to free this pointer)

this will eventually make the usage more clean and at the same time close down quite a few memory leaks.


eventually this will make dissect_ndr_nt_SID return a pointer to ep allocated memory that need not be explicitely freed.



svn path=/trunk/; revision=19226

3 files changed, 51 insertions, 11 deletions

diff --git a/epan/dissectors/packet-dcerpc-lsa.c b/epan/dissectors/packet-dcerpc-lsa.c
index 07c4aa0bf2..9c406961db 100644
--- a/epan/dissectors/packet-dcerpc-lsa.c
+++ b/epan/dissectors/packet-dcerpc-lsa.c
@@ -623,6 +623,18 @@ lsa_dissect_lsaropenpolicy2_rqst(tvbuff_t *tvb, int offset,
 		hf_lsa_server, cb_wstr_postprocess, 
 		GINT_TO_POINTER(CB_STR_COL_INFO | CB_STR_SAVE | 1));
 
+	/* OpenPolicy2() stores the servername string in se_data */
+	if(!pinfo->fd->flags.visited){
+		dcerpc_info *di = (dcerpc_info *)pinfo->private_data;
+		dcerpc_call_value *dcv = (dcerpc_call_value *)di->call_data;
+
+		/* did we get a string for the server name above? */
+		if(dcv->private_data && !dcv->se_data){
+			dcv->se_data=se_strdup(dcv->private_data);
+		}
+	}	
+
+
 	offset = dissect_ndr_pointer(tvb, offset, pinfo, tree, drep,
 		lsa_dissect_LSA_OBJECT_ATTRIBUTES, NDR_POINTER_REF,
 		"OBJECT_ATTRIBUTES", -1);
@@ -651,16 +663,18 @@ lsa_dissect_lsaropenpolicy2_reply(tvbuff_t *tvb, int offset,
 	offset = dissect_ntstatus(
 		tvb, offset, pinfo, tree, drep, hf_lsa_rc, &status);
 
-	if (status == 0) {
-		if (dcv->private_data)
+	if( status == 0 ){
+		if (dcv->se_data){
 			pol_name = ep_strdup_printf(
-				"OpenPolicy2(%s)", (char *)dcv->private_data);
-		else
-			pol_name = ep_strdup("OpenPolicy2 handle");
-
-		dcerpc_smb_store_pol_name(&policy_hnd, pinfo, pol_name);
-
-		if (hnd_item != NULL)
+				"OpenPolicy2(%s)", (char *)dcv->se_data);
+		} else {
+			pol_name = "Unknown OpenPolicy2() handle";
+		}
+		if(!pinfo->fd->flags.visited){
+			dcerpc_smb_store_pol_name(&policy_hnd, pinfo, pol_name);
+		}
+
+		if(hnd_item)
 			proto_item_append_text(hnd_item, ": %s", pol_name);
 	}
 
diff --git a/epan/dissectors/packet-dcerpc.c b/epan/dissectors/packet-dcerpc.c
index 5da2bf28fd..dec7fb3c25 100644
--- a/epan/dissectors/packet-dcerpc.c
+++ b/epan/dissectors/packet-dcerpc.c
@@ -3292,6 +3292,7 @@ dissect_dcerpc_cn_rqst (tvbuff_t *tvb, gint offset, packet_info *pinfo,
 				call_value->req_time=pinfo->fd->abs_ts;
 				call_value->rep_frame=0;
 				call_value->max_ptr=0;
+				call_value->se_data = NULL;
 				call_value->private_data = NULL;
 				g_hash_table_insert (dcerpc_cn_calls, call_key, call_value);
 
@@ -4497,6 +4498,7 @@ dissect_dcerpc_dg_rqst (tvbuff_t *tvb, int offset, packet_info *pinfo,
 	call_value->req_time=pinfo->fd->abs_ts;
 	call_value->rep_frame=0;
 	call_value->max_ptr=0;
+	call_value->se_data = NULL;
 	call_value->private_data = NULL;
 	g_hash_table_insert (dcerpc_dg_calls, call_key, call_value);
 
@@ -4517,6 +4519,7 @@ dissect_dcerpc_dg_rqst (tvbuff_t *tvb, int offset, packet_info *pinfo,
         v.req_frame = pinfo->fd->num;
         v.rep_frame = 0;
         v.max_ptr = 0;
+        v.se_data=NULL;
         v.private_data=NULL;
         value = &v;
     }
@@ -4580,6 +4583,7 @@ dissect_dcerpc_dg_resp (tvbuff_t *tvb, int offset, packet_info *pinfo,
         v.opnum = hdr->opnum;
         v.req_frame=0;
         v.rep_frame=pinfo->fd->num;
+        v.se_data=NULL;
         v.private_data=NULL;
         value = &v;
     }
diff --git a/epan/dissectors/packet-dcerpc.h b/epan/dissectors/packet-dcerpc.h
index ff61a038a9..cef43e5faa 100644
--- a/epan/dissectors/packet-dcerpc.h
+++ b/epan/dissectors/packet-dcerpc.h
@@ -255,7 +255,18 @@ dcerpc_sub_dissector *dcerpc_get_proto_sub_dissector(e_uuid_t *uuid, guint16 ver
 
 value_string *value_string_from_subdissectors(dcerpc_sub_dissector *sd);
 
-/* Private data passed to subdissectors from the main DCERPC dissector. */
+/* Private data passed to subdissectors from the main DCERPC dissector.
+ * One unique instance of this structure is created for each 
+ * DCERPC request/response transaction when we see the initial request
+ * of the transaction.
+ * These instances are persistent and will remain available until the
+ * capture file is closed and a new one is read.
+ *
+ * For transactions where we never saw the request (missing from the trace)
+ * the dcerpc runtime will create a temporary "fake" such structure to pass
+ * to the response dissector. These fake structures are not persistent
+ * and can not be used to keep data hanging around.
+ */
 typedef struct _dcerpc_call_value {
     e_uuid_t uuid;          /* interface UUID */
     guint16 ver;            /* interface version */
@@ -265,7 +276,18 @@ typedef struct _dcerpc_call_value {
     nstime_t req_time;
     guint32 rep_frame;
     guint32 max_ptr;
-    void *private_data;
+    void *se_data;          /* This holds any data with se allocation scope
+                             * that we might want to keep
+                             * for this request/response transaction.
+                             * The pointer is initialized to NULL and must be
+                             * checked before being dereferenced.
+                             * This is useful for such things as when we
+                             * need to pass persistent data from the request
+                             * to the reply, such as LSA/OpenPolicy2() that
+                             * uses this to pass the domain name from the
+                             * request to the reply.
+                             */
+    void *private_data;      /* XXX This will later be renamed as ep_data */
 } dcerpc_call_value;
 
 typedef struct _dcerpc_info {