USB MSC BOT: Workaround USBLL reassembly limitations

Mass Storage Bulk Only Transport prohibits ending data transfers with
zero length packet. This is generally not problematic when capturing at
OS URB level, but it does raise issues when capturing at USB Link Layer.

USBLL dissector has no idea where the transfer ends. It will concatenate
SCSI Data IN with CSW and SCSI Data OUT with next CBW whenever SCSI Data
length is multiple of bulk endpoint max packet size (virtually all Read
and Write commands because most common sector sizes are 512 and 4096).

CBW and CSW always end transfer reassembly because they must start at
packet boundary and their size is not equal to bulk max packet size.

Merging Data IN with CSW poses no problems at all. The only end user
visible difference is that Data IN and CSW appear in single packet (the
packet where reassembly ends).

Merging Data OUT with next CBW is ok for practical purposes, because
host periodically issues TEST UNIT READY (which does not have data
transfer and thus is not subject to the issue). While the CSW (and thus
SCSI status) will appear before Data OUT (and next CBW), the packets
will be correctly linked.

Workaround USBLL reassembly limitation by anticipating that SCSI Data
can be concatenated with Bulk Only Transport wrappers. Proper solution
would involve implementing a framework to allow USB class dissectors to
signal expected transfer length on Bulk IN or Bulk OUT endpoint whenever
CBW is encountered.

1 files changed, 64 insertions, 18 deletions

diff --git a/epan/dissectors/packet-usbms-bot.c b/epan/dissectors/packet-usbms-bot.c
index 6496e39b0d..27abe36faf 100644
--- a/epan/dissectors/packet-usbms-bot.c
+++ b/epan/dissectors/packet-usbms-bot.c
@@ -164,9 +164,22 @@ dissect_usbms_bot_control(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_
     return tvb_captured_length(tvb);
 }
 
+static proto_tree *
+create_usbms_bot_protocol_tree(tvbuff_t *tvb, proto_tree *parent_tree)
+{
+    proto_tree *tree;
+    proto_item *ti;
+
+    ti = proto_tree_add_protocol_format(parent_tree, proto_usbms_bot, tvb, 0, -1, "USB Mass Storage");
+    tree = proto_item_add_subtree(ti, ett_usbms_bot);
+
+    return tree;
+}
+
 static int
-dissect_usbms_bot_cbw(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, proto_tree *tree, usbms_bot_conv_info_t *usbms_bot_conv_info)
+dissect_usbms_bot_cbw(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, usbms_bot_conv_info_t *usbms_bot_conv_info)
 {
+    proto_tree *tree = create_usbms_bot_protocol_tree(tvb, parent_tree);
     tvbuff_t *cdb_tvb;
     int offset=0;
     int cdbrlen, cdblen;
@@ -250,8 +263,9 @@ dissect_usbms_bot_cbw(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree
 }
 
 static int
-dissect_usbms_bot_csw(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, proto_tree *tree, usbms_bot_conv_info_t *usbms_bot_conv_info)
+dissect_usbms_bot_csw(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, usbms_bot_conv_info_t *usbms_bot_conv_info)
 {
+    proto_tree *tree = create_usbms_bot_protocol_tree(tvb, parent_tree);
     int offset=0;
     guint8 status;
     itl_nexus_t *itl;
@@ -294,19 +308,31 @@ dissect_usbms_bot_csw(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree
     return tvb_captured_length(tvb);
 }
 
+static gboolean
+usbms_bot_bulk_is_cbw(tvbuff_t *tvb, int offset, gboolean is_request)
+{
+    return is_request && (tvb_reported_length(tvb)==(guint)offset+31) &&
+           tvb_get_letohl(tvb, offset) == 0x43425355;
+}
+
+static gboolean
+usbms_bot_bulk_is_csw(tvbuff_t *tvb, int offset, gboolean is_request)
+{
+    return !is_request && (tvb_reported_length(tvb)==(guint)offset+13) &&
+           tvb_get_letohl(tvb, offset) == 0x53425355;
+}
+
 /* dissector for mass storage bulk data */
 static int
 dissect_usbms_bot_bulk(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tree, void* data)
 {
     usb_conv_info_t *usb_conv_info;
     usbms_bot_conv_info_t *usbms_bot_conv_info;
-    proto_tree *tree;
-    proto_item *ti;
-    guint32 signature=0;
     int offset=0;
     gboolean is_request;
     itl_nexus_t *itl;
     itlq_nexus_t *itlq;
+    tvbuff_t *payload_tvb;
 
     /* Reject the packet if data is NULL */
     if (data == NULL)
@@ -333,42 +359,62 @@ dissect_usbms_bot_bulk(tvbuff_t *tvb, packet_info *pinfo, proto_tree *parent_tre
     col_clear(pinfo->cinfo, COL_INFO);
 
 
-    ti = proto_tree_add_protocol_format(parent_tree, proto_usbms_bot, tvb, 0, -1, "USB Mass Storage");
-    tree = proto_item_add_subtree(ti, ett_usbms_bot);
-
-    signature=tvb_get_letohl(tvb, offset);
-
-
     /*
      * SCSI CDB inside CBW
      */
-    if(is_request&&(signature==0x43425355)&&(tvb_reported_length(tvb)==31)){
-        return dissect_usbms_bot_cbw(tvb, pinfo, parent_tree, tree, usbms_bot_conv_info);
+    if (usbms_bot_bulk_is_cbw(tvb, offset, is_request)) {
+        return dissect_usbms_bot_cbw(tvb, pinfo, parent_tree, usbms_bot_conv_info);
     }
 
 
     /*
      * SCSI RESPONSE inside CSW
      */
-    if((!is_request)&&(signature==0x53425355)&&(tvb_reported_length(tvb)==13)){
-        return dissect_usbms_bot_csw(tvb, pinfo, parent_tree, tree, usbms_bot_conv_info);
+    if (usbms_bot_bulk_is_csw(tvb, offset, is_request)) {
+        return dissect_usbms_bot_csw(tvb, pinfo, parent_tree, usbms_bot_conv_info);
     }
 
     /*
      * Ok it was neither CDB not STATUS so just assume it is either data in/out
      */
-    itlq=(itlq_nexus_t *)wmem_tree_lookup32_le(usbms_bot_conv_info->itlq, pinfo->num);
+    itlq=(itlq_nexus_t *)wmem_tree_lookup32_le(usbms_bot_conv_info->itlq, pinfo->num-1);
     if(!itlq){
+        create_usbms_bot_protocol_tree(tvb, parent_tree);
         return tvb_captured_length(tvb);
     }
 
     itl=(itl_nexus_t *)wmem_tree_lookup32(usbms_bot_conv_info->itl, itlq->lun);
     if(!itl){
+        create_usbms_bot_protocol_tree(tvb, parent_tree);
         return tvb_captured_length(tvb);
     }
 
-    dissect_scsi_payload(tvb, pinfo, parent_tree, is_request, itlq, itl, 0);
-    return tvb_captured_length(tvb);
+    /*
+     * Workaround USBLL reassembly limitations by anticipating concatenated
+     * SCSI Data IN with CSW and SCSI Data OUT with next CBW. Proper would
+     * involve implementing a framework to allow USB class dissectors to signal
+     * expected transfer length on Bulk IN or Bulk OUT endpoint whenever CBW is
+     * encountered.
+     */
+    payload_tvb = tvb_new_subset_length(tvb, 0, itlq->data_length);
+    if (usbms_bot_bulk_is_cbw(tvb, itlq->data_length, is_request)) {
+        tvbuff_t *cbw_tvb = tvb_new_subset_length(tvb, itlq->data_length, 31);
+
+        dissect_scsi_payload(payload_tvb, pinfo, parent_tree, is_request, itlq, itl, 0);
+        dissect_usbms_bot_cbw(cbw_tvb, pinfo, parent_tree, usbms_bot_conv_info);
+        return tvb_captured_length(tvb);
+    } else if (usbms_bot_bulk_is_csw(tvb, itlq->data_length, is_request)) {
+        tvbuff_t *csw_tvb = tvb_new_subset_length(tvb, itlq->data_length, 13);
+
+        dissect_scsi_payload(payload_tvb, pinfo, parent_tree, is_request, itlq, itl, 0);
+        dissect_usbms_bot_csw(csw_tvb, pinfo, parent_tree, usbms_bot_conv_info);
+        return tvb_captured_length(tvb);
+    }
+
+    /* Create empty protocol tree so "usbms" filter displays this packet */
+    create_usbms_bot_protocol_tree(tvb, parent_tree);
+    dissect_scsi_payload(payload_tvb, pinfo, parent_tree, is_request, itlq, itl, 0);
+    return tvb_captured_length(payload_tvb);
 }
 
 static gboolean