From 86472b67dfb532804a2da68a7db41061c7400fe5 Mon Sep 17 00:00:00 2001 From: Michael Mann Date: Mon, 5 Aug 2013 15:55:10 +0000 Subject: SCSI dissector does not parse PERSISTENT RESERVE commands correctly. Bug 9012 (https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=9012). From Bart Van Assche. Changes: - Add REGISTER AND MOVE and REPLACE LOST RESERVATION service actions. - Decode the PARAMETER LIST LENGTH field correctly - this is a four byte field instead of a two byte field. - For the REGISTER AND MOVE service action, add support for decoding the RELATIVE TARGET PORT IDENTIFIER, TRANSPORT ID LENGTH and TransportID fields. - Fix parsing of the SERVICE ACTION field - this field is five bits wide instead of four. - Move the definition of the "scsi.persresv.control.unreg" field just below the other REGISTER AND MOVE service action parameter list fields. See also http://www.t10.org/cgi-bin/ac.pl?t=f&f=spc4r36h.pdf. - Only display persistent reservation information in a PERSISTENT RESERVE IN response if the ALLOCATION LENGTH field in the request was not zero. - Correct the offset of the (SPC-2) SCOPE-SPECIFIC ADDRESS field. This field starts at offset 16 and not at offset 8. - Correct the offsets of the SCOPE and TYPE fields. These fields are both contained in the byte at offset 21. - Correct the base of the TRANSPORTID LENGTH field from BASE_HEX into BASE_NONE since this is the base required by non-numeric types. For more information, see also: * http://www.t10.org/cgi-bin/ac.pl?t=f&f=spc4r36h.pdf * http://www.t10.org/cgi-bin/ac.pl?t=f&f=spc2r20.pdf svn path=/trunk/; revision=51152 --- AUTHORS | 1 + epan/dissectors/packet-scsi.c | 44 ++++++++++++++++++++++++++++++++++--------- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/AUTHORS b/AUTHORS index acdc95e234..5e06b5a1f1 100644 --- a/AUTHORS +++ b/AUTHORS @@ -3751,6 +3751,7 @@ Javier Godoy Matt Texier Linas Vepstas Simon Zhong +Bart Van Assche Dan Lasley gave permission for his dumpit() hex-dump routine to be used. diff --git a/epan/dissectors/packet-scsi.c b/epan/dissectors/packet-scsi.c index faed0606d5..79126aaa7c 100644 --- a/epan/dissectors/packet-scsi.c +++ b/epan/dissectors/packet-scsi.c @@ -148,6 +148,9 @@ static int hf_scsi_persresvout_reskey = -1; static int hf_scsi_persresvout_sareskey = -1; static int hf_scsi_persresvout_obsolete = -1; static int hf_scsi_persresvout_control = -1; +static int hf_scsi_persresvout_rel_tpi = -1; +static int hf_scsi_persresvout_transportid_len = -1; +static int hf_scsi_persresvout_transportid = -1; static int hf_scsi_persresv_control_rsvd = -1; static int hf_scsi_persresv_control_rsvd1 = -1; static int hf_scsi_persresv_control_rsvd2 = -1; @@ -1118,6 +1121,8 @@ static const value_string scsi_persresvout_svcaction_val[] = { {4, "Preempt"}, {5, "Preempt & Abort"}, {6, "Register & Ignore Existing Key"}, + {7, "Register & Move"}, + {8, "Replace Lost Reservation"}, {0, NULL}, }; @@ -4373,14 +4378,14 @@ dissect_spc_persistentreservein(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tre offset += 8; } } - else if ((flags & 0x1F) == SCSI_SPC_RESVIN_SVCA_RDRESV) { + else if ((flags & 0x1F) == SCSI_SPC_RESVIN_SVCA_RDRESV && len) { proto_tree_add_item(tree, hf_scsi_persresv_key, tvb, offset+8, 8, ENC_NA); proto_tree_add_item(tree, hf_scsi_persresv_scopeaddr, tvb, - offset+8, 4, ENC_NA); - proto_tree_add_item(tree, hf_scsi_persresv_scope, tvb, offset+13, + offset+16, 4, ENC_NA); + proto_tree_add_item(tree, hf_scsi_persresv_scope, tvb, offset+21, 1, ENC_BIG_ENDIAN); - proto_tree_add_item(tree, hf_scsi_persresv_type, tvb, offset+13, + proto_tree_add_item(tree, hf_scsi_persresv_type, tvb, offset+21, 1, ENC_BIG_ENDIAN); } } @@ -4398,7 +4403,7 @@ dissect_spc_persistentreserveout(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tr proto_tree_add_item(tree, hf_scsi_persresvout_svcaction, tvb, offset, 1, ENC_BIG_ENDIAN); proto_tree_add_item(tree, hf_scsi_persresv_scope, tvb, offset+1, 1, ENC_BIG_ENDIAN); proto_tree_add_item(tree, hf_scsi_persresv_type, tvb, offset+1, 1, ENC_BIG_ENDIAN); - proto_tree_add_item(tree, hf_scsi_paramlen16, tvb, offset+6, 2, ENC_BIG_ENDIAN); + proto_tree_add_item(tree, hf_scsi_paramlen16, tvb, offset+4, 4, ENC_BIG_ENDIAN); proto_tree_add_bitmask(tree, tvb, offset+8, hf_scsi_control, ett_scsi_control, cdb_control_fields, ENC_BIG_ENDIAN); /* We store the service action since we want to interpret the params */ @@ -4410,19 +4415,29 @@ dissect_spc_persistentreserveout(tvbuff_t *tvb, packet_info *pinfo _U_, proto_tr proto_tree_add_item(tree, hf_scsi_persresvout_sareskey, tvb, offset +8, 8, ENC_NA); if (cdata->itlq->flags == 0x07) { + /* Service action REGISTER AND MOVE */ const int *persresv_fields[] = { &hf_scsi_persresv_control_rsvd, &hf_scsi_persresv_control_unreg, &hf_scsi_persresv_control_aptpl, NULL }; + guint32 tid_len = tvb_get_ntohl(tvb, offset+20); + proto_tree_add_item(tree, hf_scsi_persresvout_obsolete, tvb, offset+16, 1, ENC_NA); proto_tree_add_bitmask(tree, tvb, offset+17, hf_scsi_persresvout_control, ett_persresv_control, persresv_fields, ENC_BIG_ENDIAN); + proto_tree_add_item(tree, hf_scsi_persresvout_rel_tpi, tvb, + offset+18, 2, ENC_NA); + proto_tree_add_item(tree, hf_scsi_persresvout_transportid_len, tvb, + offset+20, 4, ENC_NA); + proto_tree_add_item(tree, hf_scsi_persresvout_transportid, tvb, + offset+24, tid_len, ENC_NA); } else { + /* Other service actions than REGISTER AND MOVE. */ const int *persresv_fields[] = { &hf_scsi_persresv_control_rsvd1, &hf_scsi_persresv_control_spec_i_pt, @@ -5818,7 +5833,7 @@ proto_register_scsi(void) VALS(scsi_persresvin_svcaction_val), 0x1F, NULL, HFILL}}, { &hf_scsi_persresvout_svcaction, {"Service Action", "scsi.persresvout.svcaction", FT_UINT8, BASE_HEX, - VALS(scsi_persresvout_svcaction_val), 0x0F, NULL, HFILL}}, + VALS(scsi_persresvout_svcaction_val), 0x1F, NULL, HFILL}}, { &hf_scsi_persresv_scope, {"Reservation Scope", "scsi.persresv.scope", FT_UINT8, BASE_HEX, VALS(scsi_persresv_scope_val), 0xF0, NULL, HFILL}}, @@ -5837,9 +5852,14 @@ proto_register_scsi(void) { &hf_scsi_persresvout_control, {"Control", "scsi.presresv.control", FT_UINT8, BASE_HEX, NULL, 0x0, NULL, HFILL}}, + /* Service action REGISTER AND MOVE */ { &hf_scsi_persresv_control_rsvd, {"Reserved", "scsi.persresv.control.reserved", FT_UINT8, BASE_HEX, NULL, 0xFC, NULL, HFILL}}, + { &hf_scsi_persresv_control_unreg, + {"unreg", "scsi.persresv.control.unreg", FT_BOOLEAN, 8, + NULL, 0x02, NULL, HFILL}}, + /* Other service actions than REGISTER AND MOVE */ { &hf_scsi_persresv_control_rsvd1, {"Reserved", "scsi.persresv.control.reserved1", FT_UINT8, BASE_HEX, NULL, 0xF0, NULL, HFILL}}, @@ -5855,9 +5875,15 @@ proto_register_scsi(void) { &hf_scsi_persresv_control_aptpl, {"aptpl", "scsi.persresv.control.aptpl", FT_BOOLEAN, 8, TFS(&scsi_aptpl_tfs), 0x01, NULL, HFILL}}, - { &hf_scsi_persresv_control_unreg, /* XXX: originally missing; OK ? */ - {"unreg", "scsi.persresv.control.unreg", FT_BOOLEAN, 8, - NULL, 0x02, NULL, HFILL}}, + { &hf_scsi_persresvout_rel_tpi, + {"rel_tpi", "scsi.persresv.rel_tpi", FT_UINT16, BASE_DEC, + NULL, 0x0, NULL, HFILL}}, + { &hf_scsi_persresvout_transportid_len, + {"transportid_len", "scsi.persresv.transportid_len", + FT_UINT32, BASE_DEC, NULL, 0x0, NULL, HFILL}}, + { &hf_scsi_persresvout_transportid, + {"transportid_len", "scsi.persresv.transportid", + FT_BYTES, BASE_NONE, NULL, 0x0, NULL, HFILL}}, { &hf_scsi_release_flags, {"Release Flags", "scsi.release.flags", FT_UINT8, BASE_HEX, NULL, 0x0, NULL, HFILL}}, -- cgit v1.2.1