Get rid of boilerplate.

Use "tvb_ensure_bytes_exist()" to check for some invalid lengths.

Add some comments about possible problems in the code.

Get rid of an unnecessary length check (the length is the sum of a small
constant and a value extracted from a single byte, so the sum won't
overflow).

For a FCP_RSP, make the top-level protocol tree item run to the end of
the tvbuff and then set its length when we finish dissecting it (if we
throw an exception and don't get around to setting the length, that
means that we hit the end of the tvbuff before we hit the end of the
item).  Add some checks to catch too-large length fields.


svn path=/trunk/; revision=13917
This commit is contained in:
Guy Harris 2005-03-26 03:56:54 +00:00
parent afd5ea96b1
commit 43558fba8a
1 changed files with 35 additions and 42 deletions

View File

@ -8,12 +8,6 @@
* By Gerald Combs <gerald@ethereal.com>
* 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);
}