Re: many scripts with output cause abort

[Brandon Enright <bmenrigh () ucsd edu>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Tue, 17 Mar 2009 01:41:04 +0000
Brandon Enright <bmenrigh () ucsd edu> wrote:

> On Mon, 16 Mar 2009 17:46:10 -0600
> Patrick Donnelly <batrick.donnelly () gmail com> wrote:
>
> > Hello list,
> >
> > I've run into a problem that causes an ABORT for all the branches
> > I'm doing performance testing on for NSE. The problem appears to be
> > unrelated to NSE except having huge numbers of scripts with output
> > seems to cause it. Sometime while the output table is being printed
> > the code will mess the heap up. I've attached example output and the
> > shell script that creates the scripts.
> >
> > This code should reproduce the issue:
> >
> > batrick:~/nmap/svn/nmap$ rm scripts/*
> > batrick:~/nmap/svn/nmap$ sdb_test2.sh scripts
> > batrick:~/nmap/svn/nmap$ ./nmap --script-updatedb
> >
> > Starting Nmap 4.85BETA4 ( http://nmap.org ) at 2009-03-16 02:16 MDT
> > NSE script database updated successfully.
> > Nmap done: 0 IP addresses (0 hosts up) scanned in 0.13 seconds
> > batrick:~/nmap/svn/nmap$ ./nmap --script all localhost
> > [...]
> >
> > I truncated the rest.
> >
> > Hopefully someone should be able to locate the issue as I am
> > currently unable to investigate this.
>
> This is the first time I've looked at this code so I still need to
> figure out what is broken but so far this is what I have.  Ultimately
> the problem is in NmapOutputTable::printableSize().
>
> If you run in Valgrind you get this (line numbers are off because I
> added some prints to stderr):
>
> Entering printtableTable, tableout[(nil)] is 0
> Resized tableout[0xebc9718] to size 481519 tableoutsz
> Setting p to 0xebc9718
> ==15134==
> ==15134== Invalid write of size 1
> ==15134==    at 0x4B2D17: NmapOutputTable::printableTable(int*)
> (NmapOutputTable.cc:296) ==15134==    by 0x47D7CD:
> printportoutput(Target*, PortList*) (output.cc:862) ==15134==    by
> 0x451919: nmap_main(int, char**) (nmap.cc:1837) ==15134==    by
> 0x448E99: main (main.cc:224) ==15134==  Address 0xec3f007 is 0 bytes
> after a block of size 481,519 alloc'd ==15134==    at 0x4C214D0:
> malloc (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
> ==15134==    by 0x4C21604: realloc
> (in /usr/lib64/valgrind/amd64-linux/vgpreload_memcheck.so)
> ==15134==    by 0x4D1DD2: safe_realloc (nbase_memalloc.c:137)
> ==15134==    by 0x4B2C4E: NmapOutputTable::printableTable(int*)
> (NmapOutputTable.cc:281) ==15134==    by 0x47D7CD:
> printportoutput(Target*, PortList*) (output.cc:862) ==15134==    by
> 0x451919: nmap_main(int, char**) (nmap.cc:1837) ==15134==    by
> 0x448E99: main (main.cc:224) ==15134==
>
> Which when you attach GDB gives us this:
>
> Attaching to program: /proc/26232/fd/65525, process 26232
> 0x00000000004b2d17 in NmapOutputTable::printableTable
> (this=0x75c9fb0, size=0x0) at NmapOutputTable.cc:296
> 296           memcpy(p, cell->str,  cell->strlength);
>
> The memcpy to put this cell in the table p is overrunning the
> allocated memory.
>
> Here are some pertinent prints:
>
> The size that maxsz (a call to NmapOutputTable::printableSize())
> thinks the table should be:
>
> (gdb) p maxsz
> $1 = 481518
>
> The location of the start of the table:
>
> (gdb) p (void *)tableout
> $4 = (void *) 0xebc9718
>
> Our current pointer into the table:
>
> (gdb) p (void *)p
> $5 = (void *) 0xec3effc
>
> The point at which our write faults is 0xec3f007 which when you do the
> math comes out very nicely:
>
> (gdb) p /x (481518 + 1 + 0xebc9718)
> $7 = 0xec3f007
>
> And for kicks, this is what we were writing at the time:
> (gdb) p cell->strlength
> $2 = 37
> (gdb) p cell->str
> $3 = 0x8993b30 "|_ test_sdb_611: some constant string"
>
>
> The issue is that maxsz is not computed properly in the
> NmapOutputTable::printableSize() routine.  I haven't figured out what
> printtableSize() is supposed to be doing because I'm not sure I
> understand what a cell is supposed to be.
>
> If printtableSize() is doing what I think it is then in most cases it
> would cause *way* to much memory to be allocated.  In this case
> obviously it wasn't enough.
>
> A better approach may be to dynamically reallocate memory in
> NmapOutputTable::printableTable() whenever a cell string + space
> padding is too much to fit.  If we started off at 1k and doubled the
> size sent to realloc() every time we ran out we'd waste quite a bit
> less memory in the general case and probably wouldn't have to worry
> about making printtableSize() correct.
>
> I'll keep looking at the code.
>
> Brandon


I guess I'll keep talking to myself ;-)

I have located the problem.  printableSize() was calculating the max
row length as the size of the max full row string length and not taking
into account that printableTable() has to add a "\n" to the end of
every row.

I confirmed that in the above case, the max rowlen was calculated at 37
but in most cases p was actually increased by 38 because of the
newline.  This bug will only show up when nearly every single row being
printed is exactly at the max row length.  Normally several bytes would
be saved on most rows that would more than make up for the extra \n on
the longest row.

Attached is the patch that fixes this problem.

In fixing this issue I now understand why I've been running into
another issue which I'll send a followup email to shortly.

Brandon

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.9 (GNU/Linux)

iEYEARECAAYFAkm/CmkACgkQqaGPzAsl94KOSwCgi18jLlkb9Rge+3uSgfvYdFzI
pDoAnisYHKSofTnzB/ca9JIbm9bRBpcI
=gC8b
-----END PGP SIGNATURE-----

Attachment: printtable.diff
Description:

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org