We're now a non-profit! Support open source packet analysis by making a donation.

Wireshark-dev: Re: [Wireshark-dev] How should we solve bug #2202

From: Sake Blok <[email protected]>
Date: Sat, 2 Feb 2008 10:18:25 +0100
On Fri, Feb 01, 2008 at 07:15:52PM -0700, Stephen Fisher wrote:
> We have bug #2202 open for a problem where Wireshark is reporting an 
> incorrect PPPoE payload length (as read from the PPPoE protocol data) 
> when it is compared to the entire tvb length.  The problem is that there 
> is an Ethernet trailer at the end that isn't being recognized as an 
> Ethernet trailer.  One fix I have come up with is to call the 
> tvb_set_reported_length() function to "chop off" the Ethernet trailer so 
> the Ethernet dissector recognizes it as such.  However, this would 
> defeat the check of PPPoE payload length vs. actual length available.  
> Can anyone think of a better fix?

I wanted to suggest to treat the case where the ethernet frame was
padded (to make it 64 bytes) differently, but then I saw that it
should already do that:

 * The only reason why the payload length from the header
 * should differ from the remaining data in the packet
 * would be if the total packet length, including Ethernet
 * CRC, were < 64 bytes, so that padding was required.
 * That means that you have 14 bytes of Ethernet header,
 * 4 bytes of FCS, and fewer than 46 bytes of PPPoE packet.
 * If that's not the case, we report a difference between
 * the payload length in the packet, and the amount of
 * data following the PPPoE header, as an error.

However, the code has a one-off error in it:

if (tvb_reported_length(tvb) >= 46 &&
   reported_payload_length != actual_payload_length) {
       proto_item_append_text(ti, " [incorrect, should be %u]",
       expert_add_info_format(pinfo, ti, PI_MALFORMED,
	   PI_WARN, "Possible bad payload length %u != %u",
	   reported_payload_length, actual_payload_length);

In case of a minimal length ethernet frame, the reported_length 
is actually 46, so the ">=" should have been a ">".

I'll check in a fix as I have it ready anyways... Sorry to steal
your project for today ;-)