buglwIP - A Lightweight TCP/IP stack - Bugs: bug #34525, Queueing a packet for nd leaks...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

bug #34525: Queueing a packet for nd leaks memory

Submitted by:  Philipp Tölke <philipptoelke>
Submitted on:  Tue 11 Oct 2011 09:31:47 AM UTC  
 
Category: IPv6Severity: 3 - Normal
Item Group: Faulty BehaviourStatus: Invalid
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned Release: None
lwIP version: CVS Head

Tue 11 Oct 2011 10:52:44 AM UTC, comment #4:

Oh, that explains your "bug" then (and you would find the same bug in IPv4/etharp or when using raw PCBs).

In general, pbufs have a reference counter and the initial reference (after allocation) is not silently transferred from your application to lwIP. Instead, if the lwIP code keeps a pointer to a pbuf somewhere, it increases its ref-count (e.g. nd6.c:1562). The application is responsible for giving up its own reference when it does not need it any more (i.e. after sending the pbuf). Plus there's an extra rule that pbufs passed to a send-function that returns success may not be used any more (i.e. don't send the same pbuf twice).

The pbuf you saw freed in udp_sendto() is not the pbuf that you pass in for sending but an extra pbuf which is allocated for protocol headers (which is only allocated if the headers don't fit into your original pbuf - otherwise, nothing is freed on error).

Closing this as invalid.

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 11 Oct 2011 10:19:53 AM UTC, comment #3:

The way I understood the API the memory should be free()d by the linkoutput-function (I got this understanding from the fact, that udp_sendto frees the memory anytime it detects an error and returns !ERR_OK).

Is the expected behavioyr, that the calling code should free() the memory if no error occured and not free() if an error occurs?

Philipp Tölke <philipptoelke>
Tue 11 Oct 2011 09:52:46 AM UTC, comment #2:

Hmm, from reading the code, the patch seems to be wrong and the current code correct: q (as the pbuf passed to udp_sendto) does not get ref'ed by the send-path and thus should not be freed, either. In contrast: only if (copy_needed==0), it is ref'ed by us.

I haven't debugged this, though, only from reading the code.

Maybe you forgot to pbuf_free() the pbuf you sent after udp_sendto() returns?

Simon Goldschmidt <goldsimon>
Project AdministratorIn charge of this item.
Tue 11 Oct 2011 09:32:38 AM UTC, comment #1:

I mean the attached patch, of course.

Philipp Tölke <philipptoelke>
Tue 11 Oct 2011 09:31:47 AM UTC, original submission:

Hi,

following a rather harrowing bug:

udp_sendto -> udp_sendto_if -> ip6_output_if -> ethip6_output -> nd6_queue_packet

Please see attached bug.

Philipp Tölke <philipptoelke>

 

Attached Files
file #24131:  0001-Free-pbuf-when-copying-it.patch added by philipptoelke (650B - application/octet-stream - A patch fixing a memory-leak in nd6-queuing)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -unavailable- added by goldsimon (Posted a comment)
  • -unavailable- added by philipptoelke (Submitted the item)
  •  

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 4 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Tue 11 Oct 2011 10:52:44 AM UTCgoldsimonStatusNone=>Invalid
      Assigned toNone=>goldsimon
      Open/ClosedOpen=>Closed
    Tue 11 Oct 2011 09:31:47 AM UTCphilipptoelkeAttached File-=>Added 0001-Free-pbuf-when-copying-it.patch, #24131

    Back to the top


    Powered by Savane 3.1-cleanup1