SCSI dissector does not parse PERSISTENT RESERVE commands correctly. Bug 9012 (https...

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

git-svn-id: http://anonsvn.wireshark.org/wireshark/trunk@51152 f5534014-38df-0310-8fa8-9805f1628bb7

diff --git a/AUTHORS b/AUTHORS
index acdc95e234e599fc9704ff9602265b3571ba1df6..5e06b5a1f1737b8741e74531680db06a06a43775 100644 (file)
--- a/AUTHORS
+++ b/AUTHORS
@@ -3751,6 +3751,7 @@ Javier Godoy              <uce[AT]rjgodoy.com.ar>
 Matt Texier            <mtexier[AT]arbor.net>
 Linas Vepstas          <linasvepstas[AT]gmail.com>
 Simon Zhong                    <szhong[AT]juniper.net>
+Bart Van Assche                <bvanassche[AT]acm.org>
 
 Dan Lasley <dlasley[AT]promus.com> 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 faed0606d51904d79a5a8dce4224bbd3158937bc..79126aaa7c5d610b23b2ff00bc0f3d5e24244c97 100644 (file)
--- 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}},