diff --git a/epan/dissectors/packet-fcp.c b/epan/dissectors/packet-fcp.c index a76e122bae..6c959eb9e0 100644 --- a/epan/dissectors/packet-fcp.c +++ b/epan/dissectors/packet-fcp.c @@ -8,12 +8,6 @@ * By Gerald Combs * Copyright 1998 Gerald Combs * - * Copied from WHATEVER_FILE_YOU_USED (where "WHATEVER_FILE_YOU_USED" - * is a dissector file; if you just copied this from README.developer, - * don't bother with the "Copied from" - you don't even need to put - * in a "Copied from" if you copied an existing dissector, especially - * if the bulk of the code in the new dissector is your code) - * * This program is free software; you can redistribute it and/or * modify it under the terms of the GNU General Public License * as published by the Free Software Foundation; either version 2 @@ -279,6 +273,17 @@ dissect_fcp_cmnd (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) cdata = (fcp_conv_data_t *)g_hash_table_lookup (fcp_req_hash, &ckey); + /* + * XXX - the fetch of the fcp_dl value will throw an exception on + * a short frame before we get a chance to dissect the stuff before + * it. + * + * XXX - this doesn't appear to store the data length with the + * FCP packet with the data, so this might not work correctly + * if you select a command packet, select the corresponding data + * packet, and then select another data packet with a different + * length. + */ if (cdata) { /* Since we never free the memory used by an exchange, this maybe a * case of another request using the same exchange as a previous @@ -306,13 +311,6 @@ dissect_fcp_cmnd (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) */ if (tree) { - /* XXX - Should we check for an invalid length using a method - * other than a cast? */ - if ((gint) len <= 0) { - proto_tree_add_text(tree, tvb, 0, 0, "Invalid length: %u", len); - return; - } - ti = proto_tree_add_protocol_format (tree, proto_fcp, tvb, 0, len, "FCP_CMND"); fcp_tree = proto_item_add_subtree (ti, ett_fcp); @@ -403,9 +401,8 @@ dissect_fcp_rsp (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) { guint32 offset = 0, del_usecs = 0; - guint len = 0, - add_len = 0, - rsplen = 0; + guint32 snslen = 0, + rsplen = 0; gchar str[128]; guint8 flags; proto_item *ti; @@ -437,27 +434,7 @@ dissect_fcp_rsp (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) } if (tree) { - /* Determine the length of the FCP part of the packet */ - len = FCP_DEF_RSP_LEN; - - flags = tvb_get_guint8 (tvb, offset+10); - if (flags & 0x2) { - add_len = tvb_get_ntohl (tvb, offset+20); - len += add_len; - } - if (flags & 0x1) { - add_len = tvb_get_ntohl (tvb, offset+16); - len += add_len; - } - - /* XXX - Should we check for an invalid length using a method - * other than a cast? */ - if ((gint) len <= 0) { - proto_tree_add_text(tree, tvb, 0, 0, "Invalid length: %u", len); - return; - } - - ti = proto_tree_add_protocol_format (tree, proto_fcp, tvb, 0, len, + ti = proto_tree_add_protocol_format (tree, proto_fcp, tvb, 0, -1, "FCP_RSP"); fcp_tree = proto_item_add_subtree (ti, ett_fcp); proto_tree_add_uint_hidden (fcp_tree, hf_fcp_type, tvb, offset, 0, 0); @@ -477,26 +454,42 @@ dissect_fcp_rsp (tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree) proto_tree_add_uint_hidden (fcp_tree, hf_fcp_singlelun, tvb, offset, 0, cdata->fcp_lun); } + flags = tvb_get_guint8 (tvb, offset+10); proto_tree_add_uint_format (fcp_tree, hf_fcp_rspflags, tvb, offset+10, 1, flags, "Flags: 0x%02x (%s)", flags, rspflags_to_str (flags, str)); proto_tree_add_item (fcp_tree, hf_fcp_scsistatus, tvb, offset+11, 1, 0); if (flags & 0xC) proto_tree_add_item (fcp_tree, hf_fcp_resid, tvb, offset+12, 4, 0); - if (flags & 0x2) - proto_tree_add_item (fcp_tree, hf_fcp_snslen, tvb, offset+16, 4, 0); + if (flags & 0x2) { + snslen = tvb_get_ntohl (tvb, offset+16); + proto_tree_add_uint (fcp_tree, hf_fcp_snslen, tvb, offset+16, 4, + snslen); + } if (flags & 0x1) { rsplen = tvb_get_ntohl (tvb, offset+20); - proto_tree_add_item (fcp_tree, hf_fcp_rsplen, tvb, offset+20, 4, 0); + proto_tree_add_uint (fcp_tree, hf_fcp_rsplen, tvb, offset+20, 4, + rsplen); + /* XXX - must rsplen be >= 4? What other than the code is there? */ proto_tree_add_item (fcp_tree, hf_fcp_rspcode, tvb, offset+27, 1, 0); } + /* This handles too-large rsplen values (including ones > 2^31-1) */ + tvb_ensure_bytes_exist (tvb, offset+24, rsplen); + offset += 24+rsplen; if (flags & 0x2) { - dissect_scsi_snsinfo (tvb, pinfo, tree, offset+24+rsplen, - tvb_get_ntohl (tvb, offset+16), + dissect_scsi_snsinfo (tvb, pinfo, tree, offset, + snslen, (guint16) (cdata?cdata->fcp_lun:0xffff) ); } + /* This handles too-large snslen values (including ones > 2^31-1) */ + tvb_ensure_bytes_exist (tvb, offset, snslen); + offset += snslen; + proto_item_set_end (ti, tvb, offset); if (cdata) { + /* + * XXX - this isn't done if an exception is thrown. + */ g_mem_chunk_free (fcp_req_vals, cdata); g_hash_table_remove (fcp_req_hash, &ckey); }