EPL: Fix segmented transfer complete detection

 In case of a segmented SDO transfer, the transfer complete response can
 contain additional data that should not be evaluated by the dissector.

Change-Id: I7016eb88b93aac8c318e703fe60a90c3adbf9eeb
Reviewed-on: https://code.wireshark.org/review/14692
Petri-Dish: Roland Knall <rknall@gmail.com>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Roland Knall <rknall@gmail.com>

1 files changed, 96 insertions, 53 deletions

diff --git a/epan/dissectors/packet-epl.c b/epan/dissectors/packet-epl.c
index 2203fc643a..97e67a588e 100644
--- a/epan/dissectors/packet-epl.c
+++ b/epan/dissectors/packet-epl.c
@@ -446,6 +446,8 @@ static struct _epl_segmentation{
 
 static epl_sdo_reassembly epl_asnd_sdo_reassembly_write;
 static epl_sdo_reassembly epl_asnd_sdo_reassembly_read;
+static gboolean first_read = TRUE;
+static gboolean first_write = TRUE;
 
 /* Priority values for EPL message type "ASnd", "", "", field PR */
 #define EPL_PR_GENERICREQUEST   0x03
@@ -1650,6 +1652,10 @@ static void
 cleanup_dissector(void)
 {
 	reassembly_table_destroy(&epl_reassembly_table);
+	count = 0;
+	ct = 0;
+	first_read = TRUE;
+	first_write = TRUE;
 }
 
 /* preference whether or not display the SoC flags in info column */
@@ -2820,14 +2826,15 @@ dissect_epl_sdo_command(proto_tree *epl_tree, tvbuff_t *tvb, packet_info *pinfo,
 	gboolean response, abort_flag;
 	guint32 abort_code = 0;
 	guint32 fragmentId = 0, remlength = 0;
-	guint16 segment_size;
+	guint16 segment_size = 0;
 	proto_tree *sdo_cmd_tree = NULL;
-	proto_tree *payload = NULL;
 	proto_item *item;
-	fragment_head *frag_msg = NULL;
+	guint8 sendCon = 0;
 
 	offset += 1;
 
+	sendCon = tvb_get_guint8(tvb, 5) & EPL_ASND_SDO_SEQ_SEND_CON_ERROR_VALID_ACK_REQ;
+
 	command_id = tvb_get_guint8(tvb, offset + 2);
 	abort_flag = tvb_get_guint8(tvb, offset + 1) & EPL_ASND_SDO_CMD_ABORT_FILTER;
 
@@ -2840,6 +2847,7 @@ dissect_epl_sdo_command(proto_tree *epl_tree, tvbuff_t *tvb, packet_info *pinfo,
 		transaction_id = tvb_get_guint8(tvb, offset);
 		response   = tvb_get_guint8(tvb, offset + 1) & EPL_ASND_SDO_CMD_RESPONSE_FILTER;
 		segmented  = (tvb_get_guint8(tvb, offset + 1) & EPL_ASND_SDO_CMD_SEGMENTATION_FILTER) >> 4;
+
 		segment_size = tvb_get_letohs(tvb, offset + 3);
 
 		col_append_fstr(pinfo->cinfo, COL_INFO, "Cmd:%s,TID=%02d ",
@@ -2852,14 +2860,16 @@ dissect_epl_sdo_command(proto_tree *epl_tree, tvbuff_t *tvb, packet_info *pinfo,
 		proto_tree_add_item(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_abort,    tvb, offset, 1, ENC_LITTLE_ENDIAN);
 
 		proto_tree_add_item(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_segmentation, tvb, offset, 1, ENC_LITTLE_ENDIAN);
-		offset += 1;
 
-		proto_tree_add_item(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_command_id, tvb, offset, 1, ENC_LITTLE_ENDIAN);
-		offset += 1;
-
-		proto_tree_add_item(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_segment_size, tvb, offset, 2, ENC_LITTLE_ENDIAN);
-		offset += 4;
+		if (segment_size != 0)
+		{
+			offset += 1;
+			proto_tree_add_item(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_command_id, tvb, offset, 1, ENC_LITTLE_ENDIAN);
+			offset += 1;
 
+			proto_tree_add_item(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_segment_size, tvb, offset, 2, ENC_LITTLE_ENDIAN);
+			offset += 4;
+		}
 		/* adjust size of packet */
 		tvb_set_reported_length(tvb, offset + segment_size);
 
@@ -2867,12 +2877,15 @@ dissect_epl_sdo_command(proto_tree *epl_tree, tvbuff_t *tvb, packet_info *pinfo,
 		{
 			if((command_id == EPL_ASND_SDO_COMMAND_WRITE_BY_INDEX) || (command_id == EPL_ASND_SDO_COMMAND_READ_BY_INDEX))
 			{
-				/* if download => reset counter */
-				if(command_id == EPL_ASND_SDO_COMMAND_WRITE_BY_INDEX)
-					ct = 0x00;
-				/* if upload => reset counter */
-				else if(command_id == EPL_ASND_SDO_COMMAND_READ_BY_INDEX)
-					count = 0x00;
+				if (sendCon != EPL_ASND_SDO_SEQ_SEND_CON_ERROR_VALID_ACK_REQ)
+				{
+					/* if download => reset counter */
+					if(command_id == EPL_ASND_SDO_COMMAND_WRITE_BY_INDEX)
+						ct = 0x00;
+					/* if upload => reset counter */
+					else if(command_id == EPL_ASND_SDO_COMMAND_READ_BY_INDEX)
+						count = 0x00;
+				}
 				/* payload length */
 				payload_length = tvb_reported_length_remaining(tvb, offset);
 				/* create a key for reassembly => first 16 bit are src-address and
@@ -2880,20 +2893,21 @@ dissect_epl_sdo_command(proto_tree *epl_tree, tvbuff_t *tvb, packet_info *pinfo,
 				fragmentId = (guint32)((((guint32)epl_segmentation.src)<<16)+epl_segmentation.dest);
 				/* set fragmented flag */
 				pinfo->fragmented = TRUE;
-				frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
+				fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
 												fragmentId, NULL, 0, payload_length, TRUE );
 				fragment_add_seq_offset ( &epl_reassembly_table, pinfo, fragmentId, NULL, 0 );
+				if (command_id == EPL_ASND_SDO_COMMAND_WRITE_BY_INDEX)
+				{
+					first_write = FALSE;
+				}
+				else
+				{
+					first_read = FALSE;
+				}
 				/* if Segmentation = Initiate then print DataSize */
 				proto_tree_add_item(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_data_size, tvb, offset, 4, ENC_LITTLE_ENDIAN);
 				segmented = TRUE;
 
-				if(frag_msg != NULL)
-				{
-					item = proto_tree_add_uint_format(sdo_cmd_tree, hf_epl_asnd_sdo_cmd_reassembled, tvb, offset ,payload_length, 0,
-												"Reassembled: %d bytes total (%d bytes in this frame)",frag_msg->len,payload_length);
-					payload = proto_item_add_subtree(item, ett_epl_asnd_sdo_data_reassembled);
-					process_reassembled_data(tvb, 0, pinfo, "Reassembled Message", frag_msg, &epl_frag_items, NULL, payload );
-				}
 				offset += 4;
 			}
 			else
@@ -2965,7 +2979,7 @@ dissect_epl_sdo_command(proto_tree *epl_tree, tvbuff_t *tvb, packet_info *pinfo,
 gint
 dissect_epl_sdo_command_write_by_index(proto_tree *epl_tree, tvbuff_t *tvb, packet_info *pinfo, gint offset, guint8 segmented, gboolean response, guint16 segment_size)
 {
-	gint size, payload_length;
+	gint size, payload_length = 0;
 	guint16 idx = 0x00, nosub = 0x00, sod_index = 0x00, error = 0xFF, entries = 0x00, sub_val = 0x00;
 	guint8 subindex = 0x00;
 	guint32 fragmentId = 0;
@@ -3091,6 +3105,7 @@ dissect_epl_sdo_command_write_by_index(proto_tree *epl_tree, tvbuff_t *tvb, pack
 			fragmentId = (guint32)((((guint32)epl_segmentation.src)<<16)+epl_segmentation.dest);
 			/* set the fragmented flag */
 			pinfo->fragmented = TRUE;
+
 			/* get payloade size */
 			payload_length = tvb_reported_length_remaining(tvb, offset);
 			/* if the frame is the last frame */
@@ -3109,14 +3124,34 @@ dissect_epl_sdo_command_write_by_index(proto_tree *epl_tree, tvbuff_t *tvb, pack
 				frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
 							  fragmentId, NULL, ct, payload_length, end_segment ? FALSE : TRUE );
 			}
-			else if(epl_asnd_sdo_reassembly_write.frame[epl_segmentation.recv][epl_segmentation.send] == 0x00)
+			else
 			{
-				/* save the current frame and increase counter */
-				epl_asnd_sdo_reassembly_write.frame[epl_segmentation.recv][epl_segmentation.send] = frame;
-				ct += 1;
-				/* add the frame to reassembly_table */
-				frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
-								  fragmentId, NULL, ct, payload_length, end_segment ? FALSE : TRUE );
+				if(epl_asnd_sdo_reassembly_write.frame[epl_segmentation.recv][epl_segmentation.send] == 0x00)
+				{
+					/* save the current frame and increase counter */
+					epl_asnd_sdo_reassembly_write.frame[epl_segmentation.recv][epl_segmentation.send] = frame;
+					ct += 1;
+					/* add the frame to reassembly_table */
+					if (first_write)
+					{
+						frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
+							fragmentId, NULL, 0, payload_length, end_segment ? FALSE : TRUE );
+						fragment_add_seq_offset(&epl_reassembly_table, pinfo, fragmentId, NULL, ct);
+
+						first_write = FALSE;
+					}
+					else
+					{
+						frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
+							fragmentId, NULL, ct, payload_length, end_segment ? FALSE : TRUE );
+					}
+				}
+				else
+				{
+					frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
+						fragmentId, NULL, 0, payload_length, end_segment ? FALSE : TRUE);
+					epl_asnd_sdo_reassembly_write.frame[epl_segmentation.recv][epl_segmentation.send] = frame;
+				}
 			}
 
 			/* if the reassembly_table is not Null and the frame stored is the same as the current frame */
@@ -3133,8 +3168,6 @@ dissect_epl_sdo_command_write_by_index(proto_tree *epl_tree, tvbuff_t *tvb, pack
 					proto_tree_add_uint_format_value(payload_tree, hf_epl_asnd_sdo_cmd_reassembled, tvb, 0, 0,
 									payload_length, "%d bytes (over all fragments)", frag_msg->len);
 					col_append_str(pinfo->cinfo, COL_INFO, " (Message Reassembled)" );
-					/* reset memory */
-					memset(&epl_asnd_sdo_reassembly_write,0,sizeof(epl_sdo_reassembly));
 				}
 				else
 				{
@@ -3144,9 +3177,10 @@ dissect_epl_sdo_command_write_by_index(proto_tree *epl_tree, tvbuff_t *tvb, pack
 					/* add reassemble field => Reassembled in: */
 					process_reassembled_data(tvb, 0, pinfo, "Reassembled Message", frag_msg, &epl_frag_items, NULL, payload_tree );
 				}
+				first_write = TRUE;
+				ct = 0;
 			}
 		}
-
 		size = tvb_reported_length_remaining(tvb, offset);
 
 		/* if the frame is a PDO Mapping and the subindex is bigger than 0x00 */
@@ -3451,12 +3485,11 @@ dissect_epl_sdo_command_read_by_index(proto_tree *epl_tree, tvbuff_t *tvb, packe
 						 segment_size, idx, subindex);
 		col_append_fstr(pinfo->cinfo, COL_INFO, " (%s", val_to_str_ext_const(((guint32) (idx << 16)), &sod_index_names, "User Defined"));
 		col_append_fstr(pinfo->cinfo, COL_INFO, "/%s)",val_to_str_ext_const((subindex|(idx<<16)), &sod_index_names, "User Defined"));
-
 	}
 	else
 	{
-		/* upload */
-		if(segmented > 0x01)
+		/* upload and no response */
+		if(segmented > 0x01 && segment_size != 0)
 		{
 			/* get the fragmentId */
 			fragmentId = (guint32)((((guint32)epl_segmentation.src)<<16)+epl_segmentation.dest);
@@ -3468,28 +3501,34 @@ dissect_epl_sdo_command_read_by_index(proto_tree *epl_tree, tvbuff_t *tvb, packe
 			if(segmented == EPL_ASND_SDO_CMD_SEGMENTATION_TRANSFER_COMPLETE)
 				end_segment = TRUE;
 
-			/* if the send-sequence-number is at the end or the beginning of a sequence */
-			if(epl_segmentation.send == 0x3f || epl_segmentation.send <= 0x01 )
-			{
-				/* reset memory */
-				memset(&epl_asnd_sdo_reassembly_read,0,sizeof(epl_sdo_reassembly));
-				epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv][epl_segmentation.send] = frame;
-				count += 1;
-			}
-			else if(epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv][epl_segmentation.send] == 0x00)
+			if(epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv][epl_segmentation.send] == 0x00 ||
+				epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv][epl_segmentation.send] == frame)
 			{
+				if (epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv][epl_segmentation.send] == 0x00)
+					count += 1;
 				/* store the current frame and increase the counter */
 				epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv][epl_segmentation.send] = frame;
-				count += 1;
-			}
-			/* add the frame to reassembly_table */
-			frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
+
+				/* add the frame to reassembly_table */
+				if (first_read)
+				{
+					frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
+							fragmentId, NULL, 0, payload_length, end_segment ? FALSE : TRUE );
+					fragment_add_seq_offset(&epl_reassembly_table, pinfo, fragmentId, NULL, count);
+
+					first_read = FALSE;
+				}
+				else
+				{
+					frag_msg = fragment_add_seq_check(&epl_reassembly_table, tvb, offset, pinfo,
 							fragmentId, NULL, count, payload_length, end_segment ? FALSE : TRUE );
+				}
+			}
 
 			/* if the reassembly_table is not Null and the frame stored is the same as the current frame */
 			if(frag_msg != NULL && (epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv][epl_segmentation.send] == frame))
 			{
-				if(end_segment)
+				if(end_segment || payload_length > 0)
 				{
 					cmd_payload = proto_tree_add_uint_format(epl_tree, hf_epl_asnd_sdo_cmd_reassembled, tvb, offset, payload_length,0,
 															"Reassembled: %d bytes total (%d bytes in this frame)",frag_msg->len,payload_length);
@@ -3498,9 +3537,10 @@ dissect_epl_sdo_command_read_by_index(proto_tree *epl_tree, tvbuff_t *tvb, packe
 					process_reassembled_data(tvb, 0, pinfo, "Reassembled Message", frag_msg, &epl_frag_items, NULL, payload_tree );
 					proto_tree_add_uint_format_value(payload_tree, hf_epl_asnd_sdo_cmd_reassembled, tvb, 0, 0,
 									payload_length, "%d bytes (over all fragments)", frag_msg->len);
-					col_append_str(pinfo->cinfo, COL_INFO, " (Message Reassembled)" );
+					if (frag_msg->reassembled_in == frame)
+						col_append_str(pinfo->cinfo, COL_INFO, " (Message Reassembled)" );
 					/* reset memory */
-					memset(&epl_asnd_sdo_reassembly_read,0,sizeof(epl_sdo_reassembly));
+					memset(&epl_asnd_sdo_reassembly_read.frame[epl_segmentation.recv], 0, sizeof(guint32) * EPL_MAX_SEQUENCE);
 				}
 				else
 				{
@@ -3510,6 +3550,9 @@ dissect_epl_sdo_command_read_by_index(proto_tree *epl_tree, tvbuff_t *tvb, packe
 					/* add reassemble field => Reassembled in: */
 					process_reassembled_data(tvb, 0, pinfo, "Reassembled Message", frag_msg, &epl_frag_items, NULL, payload_tree );
 				}
+
+				first_read = TRUE;
+				count = 0;
 			}
 		}
 		/* response */