lwip-users
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

RE: [lwip-users] questions on lwip (two TCP/IP sockets)


From: Kroesche, Joe
Subject: RE: [lwip-users] questions on lwip (two TCP/IP sockets)
Date: Thu, 12 Aug 2010 18:13:52 -0500

I updated the post on the TI support forum.  Please let me know if you see any 
problem with my answer.  Thank you all for pointing out the problem. 

>Unless of course TI's EDK has a modified pbuf_free() which really does 
>behave the way described, completely changing its meaning and affecting 
>portability of code to/from any other LWIP port.

No, we have not done that.  We do our best to avoid modifying any third party 
code, certainly anything that would affect an API.  Generally the code is the 
same as if pulled from the maintainer's repository.  We even leave in the 
warnings, if there are any.

Regards,
Joe Kroesche

-----Original Message-----
From: address@hidden [mailto:address@hidden On Behalf Of David Empson
Sent: Thursday, August 12, 2010 5:36 PM
To: Mailing list for lwIP users
Subject: Re: [lwip-users] questions on lwip (two TCP/IP sockets)

Simon Goldschmidt <address@hidden> wrote:
>  Kieran Mansley wrote:
>> On 12 Aug 2010, at 20:16, Jeff Barber wrote:
>>
>>> I'd like to see someone more authoritative (like Simon or Kieran?)
>>> weigh in on this but I'm pretty sure that the advice you cited is just
>>> plain wrong.
>> Yes, it's very wrong.  It may once have been true in the dim past,
> Hmm, "dim past"... that post seems to be only less than 2 months old, 
> however, I don't remember having read it or I would have told the author 
> that it's wrong...

It wasn't posted to an lwip mailing list, but to a ti.com forum, so you can 
be forgiven for not having seen it.

I suspect the actual bug in the forum posting was the missing tcp_recved() 
call. The looped pbuf_free() code is dangerous for all the reasons cited 
here.

>>   but even so it references memory after it was freed
> And that's the point at which I'm really confused: Either our 
> documentation is really really bad or the author of that post should first 
> start programming with something easier than lwIP... accessing freed 
> memory seems like a beginner's mistake to me (sorry if this sounds rude!), 
> and (unfortunately) lwIP is a bit more complicated to handle than a simple 
> "hello world" example :-(

The worrying part is that a TI employee in responding to the question 
claimed a simple call to pbuf_free() would leak memory if there was a chain 
of pbufs, recommended freeing each pbuf in the chain separately, and 
confirmed the resulting code. That suggestion is now shown in the forum as a 
verified answer. How many other people have or will be misled?

The implementation is of course wrong in many ways, but there can be no 
"right" way of calling pbuf_free() in a loop for each pbuf in the chain: 
assuming a reference count of 1, even if p->next had been copied elsewhere 
prior to calling pbuf_free(p), it would no longer be valid afterwards, 
because the chained pbuf (if any) was already freed. The next call in the 
loop would be trying to free an already freed pbuf (and referencing a 
possibly invalid p->next pointer for all subsequent pbufs). If the reference 
count was higher, then this code would free part of a pbuf chain still being 
used elsewhere, leaving dangling pointers into free memory.

This is the sort of bug which might hide and bite you only in unusual 
circumstances, because most of the time the receive callback will be dealing 
with a single pbuf.

Unless of course TI's EDK has a modified pbuf_free() which really does 
behave the way described, completely changing its meaning and affecting 
portability of code to/from any other LWIP port.

(Shudder)

Someone who actually uses that EDK should confirm the details and post a 
followup on that forum pointing out the error.


_______________________________________________
lwip-users mailing list
address@hidden
http://lists.nongnu.org/mailman/listinfo/lwip-users



reply via email to

[Prev in Thread] Current Thread [Next in Thread]