Re: [lwip-devel] Particular size of ping can halt lwIP -- a pbuf_copy bug?

[David Empson]

On 2012-04-27 18:47, narke wrote:
> Hi,
>
> When ppp receive data (in pppInProc function), it allocate pbuf chains
> to hold received bytes including the last two bytes checksum. It's
> possible that a particular size of incoming ppp frame makes the
> pppInProc to allocate a kind of pbuf chains where the last pbuf just
> hold the two bytes checksum and with lenght 2.  Before ppp forward the
> pbuf chains to upper layer, it will remove the ppp checksum by
> reducing the length field of the last pbuf by 2.  So, in this case, it
> will forward a pbuf chains with n + 1 pbuf's and the last pbuf is zero
> length.

What happens in the case where the checksum occupies the last byte of 
the second to last pbuf and the first byte of the last pbuf? (e.g. ping 
length is one byte shorter than the case you describe.)

From a quick glance at ppp.c, it looks like this case is mishandled. 
Upon receiving the closing FLAG, the length of the last pbuf is checked, 
and if at least two, the length is reduced by two.

If the last pbuf length is one, then the checksum bytes will remain 
attached to the packet and will be delivered to higher layers. Is this a 
problem?

I'd expect this would work like Ethernet padding, so it should be 
ignored by higher layers in the case of IP packets, but a ping may be 
echoing those bytes.

If the code is modified to correctly trim the checksum bytes in this 
case, it would have to locate the final pbuf in pcrx->inHead and reduce 
its length by one, then free pcrx->inTail rather than calling pbuf_cat() 
to attach pcrx->inTail to pcrx->inHead. If this code was implemented, it 
would be easiest to handle the checksum by itself in the final pbuf as 
part of the same code.

Alternatively, if the extra checksum bytes are not a problem then the 
case where the checksum is by itself in the final pbuf could be dealt 
with by not reducing the pbuf length in that case, i.e. change the trim 
check to

if (pcrx->inTail->len > 2)

rather than

if (pcrx->inTail->len >= 2)