[lwip-devel] [bug #46467] ip_frag() shouldn't modify pbuf in case of a retransmission

[Zach Smith]

Follow-up Comment #4, bug #46467 (project lwip):

The IP frag modification is during TX of a TCP packet. To describe more
clearly, ip_frag is modifying the payload. I understand that the lower layers
do modify (decrement) the payload to add on IP/ETH headers but in ip_frag the
payload is being incremented and modified to point further into the data
payload as data is sent out. Specifically these lines:

    /* Can just adjust p directly for needed offset. */
    p->payload = (u8_t *)p->payload + poff;
    p->len -= poff;

This seems like a problem because TCP references the pbuf and may try to
re-transmit it later. When the retry happens, I think that the problem occurs
when tcp_output_segment() checks the seg->p->payload and compares it against
the seg->tcphdr. It tries to adjust the payload and len members but it is
going on the assumption that the payload may have been *decremented* to add on
headers. But it does not take into account that payload may have been
*incremented* to point into the data. That is where I think the pbuf gets
messed up.

I guess I don't know enough to say this is a problem with TCP (shouldn't ref
the pbuf later) or with ip_frag (shouldn't modify the pbuf)

Specifically, we were able to catch this in a specific case where in
ip_output_if_opt() before calling ip_frag() the p->tot_len != p->len AND
p->next was NULL! The call stack showed it came from a tcp_slowtmr retransmit.
The code assumes that if tot_len > len then there is another pbuf in the chain
and it ended up referencing a null pointer. We got looking in ip_frag and
figured that this was happening because the payload pointer was modified.

This fix is to make sure the structure (p->len and p->payload) are not changed
upon exiting from what they were when entering



    _______________________________________________________

Reply to this item at:

  <http://savannah.nongnu.org/bugs/?46467>

_______________________________________________
  Message sent via/by Savannah
  http://savannah.nongnu.org/