Wireshark · Wireshark-bugs: [Wireshark-bugs] [Bug 5117] New: tcp_dissect_pdus: Possible bug related to appending to COL_INFO ?

[bugzilla-daemon@xxxxxxxxxxxxx]

https://bugs.wireshark.org/bugzilla/show_bug.cgi?id=5117

           Summary: tcp_dissect_pdus: Possible bug related to appending to
                    COL_INFO ?
           Product: Wireshark
           Version: SVN
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: Minor
          Priority: Low
         Component: Wireshark
        AssignedTo: wireshark-bugs@xxxxxxxxxxxxx
        ReportedBy: wmeier@xxxxxxxxxxx


Build Information:
SVN
--
The following code fragment (based upon code in packet-fcgi.c) is written to
append some text to COL_INFO for each PDU encountered when dissecting a
protocol running over TCP.

static void dissect_record(...) {
   ...
   col_clear(pinfo->cinfo, COL_INFO);
   col_append_sep_str(pinfo->cinfo, COL_INFO, NULL,
                      val_to_str(type, record_types, "Unknown (%u)"));
   col_set_fence(pinfo->cinfo, COL_INFO);
   ...
}

tcp_dissect_pdus(...., dissect_record) {
   ...
}

When there are multiple PDUs in a TCP frame, only the col_append... for the
first PDU succeeds; The col_appends for the rest of the PDUs in the frame are
ignored.


My question: 
 Should the above code work or is there a different way to accomplish the
 objective ?

This issue can be seen with the new FCGI dissector recently committed.
Look at Frame #98 in the capture file attached to Bug #5067.
There should be 3 "record types" in the INFO column, not 1.


--------
Looking at packet-tcp.c it appears that col_set_writable(...,FALSE) is called
after the first PDU in a frame thus prevents further appending to COL_INFO.

I don't really understand this code so I'm filing this as a possible bug for
review by someone knowledgeable about this code.  :)


    ...
    if(another_pdu_follows){
        /* there was another pdu following this one. */
        pinfo->can_desegment=2;
        /* we also have to prevent the dissector from changing the
         * PROTOCOL and INFO colums since what follows may be an
         * incomplete PDU and we dont want it be changed back from
         *  <Protocol>   to <TCP>
         * XXX There is no good way to block the PROTOCOL column
         * from being changed yet so we set the entire row unwritable.
         * The flag cleared_writable stores the initial state.
         */
        col_set_fence(pinfo->cinfo, COL_INFO);
        cleared_writable |= col_get_writable(pinfo->cinfo);
        col_set_writable(pinfo->cinfo, FALSE);
        offset += another_pdu_follows;
        seq += another_pdu_follows;
        goto again;
    else
        ...

-- 
Configure bugmail: https://bugs.wireshark.org/bugzilla/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.