Bug 1358

Summary:	PacketMetadata::~PacketMetadata() writes in freed memory
Product:	ns-3	Reporter:	Nicola Baldo <nicola>
Component:	network	Assignee:	ns-bugs <ns-bugs>
Status:	RESOLVED FIXED
Severity:	normal	CC:	mathieu.lacage
Priority:	P5
Version:	ns-3-dev
Hardware:	All
OS:	All
Attachments:	valgrind error log proposed fix revised patch

Description Nicola Baldo 2012-02-05 13:15:50 UTC

Created attachment 1327 [details]
valgrind error log

I am experiencing corrupted memory errors in a program when executed with around 90000 nodes or more. The symptoms (which vary a bit depending on the exact number of nodes) are sementation faults or failed assertions in malloc.c, which point to a memory corruption problem. With smaller number of nodes, the problem is not evident.

I've traced the problem to a write operation in a block of already freed memory performed by PacketMetadata::~PacketMetadata().  Please have a look at the attached valgrind log.

I've been able to reproduce this bug only with the LENA code, in particular the attached log was produced with changeset 7542c79c9d19 which is available at http://code.nsnam.org/nbaldo/ns-3-lena-dev/
Note that this code is pretty much up-to-date with respect to ns-3-dev (the last merged changeset is 90904c14135f of January 29, 2012)

Here is the command line used to reproduce the error:
./waf --run lena-rem-sector-antenna --command="%s --ns3::RadioEnvironmentMapHelper::XRes=300 --ns3::RadioEnvironmentMapHelper::YRes=300"

I have a very poor understanding of the inner working of the fancy memory management solution of PacketMetadata, hence any suggestion to get to a fix would be very much appreciated.

Comment 1 Nicola Baldo 2012-02-07 13:08:46 UTC

Created attachment 1328 [details]
proposed fix

I found the problem. The reference count in PacketMetadata::Data is done with an uint16_t, which allows for a maxium 65535 references. The counter wraps around in my simulation scenario due to the fact that the same packet is transmitted to more than 90000 nodes.

I am attaching a proposed fix.

Comment 2 Tommaso Pecorella 2012-02-08 15:27:04 UTC

Good for me.

+1

Comment 3 Nicola Baldo 2012-02-24 08:21:10 UTC

Mathieu, you're the maintainer of the network module, can you give me your +1 as well?

Comment 4 Mathieu Lacage 2012-02-24 09:26:23 UTC

if you change the size of this field, you need to change the size of the m_data array to be 8 so that the total size of struct Data is 16 bytes. If you do this, you need to be careful in PacketMetadata::Allocate for example: s/10/8/

Comment 5 Nicola Baldo 2012-02-24 13:45:06 UTC

Created attachment 1339 [details]
revised patch

here is a revised patch.

Comment 6 Mathieu Lacage 2012-02-24 14:23:41 UTC

ack. please, apply

(In reply to comment #5)
> Created attachment 1339 [details]
> revised patch
> 
> here is a revised patch.

Comment 7 Nicola Baldo 2012-02-28 06:59:59 UTC

changeset:   7748:1fbd1de4b24d
tag:         tip
user:        Nicola Baldo <nbaldo@cttc.es>
date:        Tue Feb 28 12:05:16 2012 +0100
summary:     fixed bug 1358