From aef08cc2434b5ba5aee4422fbcf481004c62583a Mon Sep 17 00:00:00 2001 From: Peter Wu Date: Sun, 6 Jul 2014 16:29:03 +0200 Subject: [WIP] packet: handle return value of new-style dissectors Before this patch, any non-zero return value from a new-style dissector would be treated as success. The actual meaning of the return value ("need more bytes" or "accepted X bytes for PDU") was ignored. After this patch, non-zero values will set desegment_offset and desegment_len (if these were not set before and if reassembly is allowed by the parent protocol). Regression alert: many dissectors return bogus values. These fall in the categories: - Calculate offset + value, return this. Bug: the data may actually be less (or more). - Return less than tvb_captured_length(tvb). Bug: if padding/junk follows a PDU, then it needs to be dropped. When a smaller value is returned, the new code assumes that it is part of the next segment. Warning: currently, dissectors need to handle cases where multiple PDUs are present in a single segment (otherwise the remainder gets ignored). The HTTP dissector handles this with an internal loop for example. To be done: fix dissectors to return a negative value rather than setting desegment_len. DO NOT MERGE: two getenv() for debugging. Change-Id: Id6d5ea195d620ab2bfb18c39f86b598b7b2b4797 --- epan/packet.c | 33 +++++++++++++++++++++++++++++++-- 1 file changed, 31 insertions(+), 2 deletions(-) diff --git a/epan/packet.c b/epan/packet.c index e938884126..7fba6fb9d7 100644 --- a/epan/packet.c +++ b/epan/packet.c @@ -597,8 +597,11 @@ struct dissector_handle { * old style dissector : * length of the payload or 1 of the payload is empty * new dissector : - * >0 this protocol was successfully dissected and this was this protocol. + * >0 the protocol accepted the data, this is the number of bytes that were + * processed. If not equal to tvb_captured_length(), then the remaining + * data will be used for the next PDU (if reassembly is enabled). * 0 this packet did not match this protocol. + * <0 this protocol accepted the data, but needs more for a PDU. * * The only time this function will return 0 is if it is a new style dissector * and if the dissector rejected the packet. @@ -621,11 +624,37 @@ call_dissector_through_handle(dissector_handle_t handle, tvbuff_t *tvb, EP_CHECK_CANARY(("before calling handle->dissector.new_d for %s",handle->name)); ret = (*handle->dissector.new_d)(tvb, pinfo, tree, data); EP_CHECK_CANARY(("after calling handle->dissector.new_d for %s",handle->name)); + /* If the parent protocol can handle desegmentation (such as + * TCP), then set params according to the dissector return + * value, but *only* if desegment_len was not already set */ + if (!getenv("WS_DISABLE_EXPERIMENT")) + if (pinfo->can_desegment && pinfo->desegment_len == 0) { + if (ret < 0) { + /* need more bytes, so request another segment. + * Note that desegment_offset may be set if the + * dissector could already extract some PDUs */ + pinfo->desegment_len = DESEGMENT_ONE_MORE_SEGMENT; + } else if (ret > 0) { + /* succesfully dissected, check for more + * segments or verify boundary */ + guint caplen = tvb_captured_length(tvb); + + if (getenv("WS_BULLSHIT") && (guint) ret < caplen) { + /* not all data was consumed, assume + * that more data is necessary */ + pinfo->desegment_offset = ret; + pinfo->desegment_len = DESEGMENT_ONE_MORE_SEGMENT; + } else { + /* ret must not be larger than available */ + DISSECTOR_ASSERT((guint) ret <= caplen); + } + } + } } else { EP_CHECK_CANARY(("before calling handle->dissector.old for %s",handle->name)); (*handle->dissector.old)(tvb, pinfo, tree); EP_CHECK_CANARY(("after calling handle->dissector.old for %s",handle->name)); - ret = tvb_length(tvb); + ret = tvb_captured_length(tvb); if (ret == 0) { /* * XXX - a tvbuff can have 0 bytes of data in -- cgit v1.2.1