[GRASS-dev] G7.0.svn: g.remove segfault

Fri Jun 26 12:54:31 PDT 2015

Hi,

2015-06-26 21:03 GMT+02:00 Vaclav Petras <wenzeslaus at gmail.com>:
>
>
> On Fri, Jun 26, 2015 at 2:37 PM, Sören Gebbert
> <soerengebbert at googlemail.com> wrote:
>>
>> Hi,
>> may i suggest a patch to solve this issue?
>
>
> Sure!

Ok, i will commit the patch.

>
> I'm not sure if I can judge the path. However, sprintf is used a lot in
> GRASS, so I'm not sure if we can just replace it with other function without
> understanding what is the issue (at least I don't understand).

All sprintf calls in GRASS should be replaced by G_snprintf() because
sprintf is by design unsafe and the result of many buffer overflows
and eventually exploits. sprintf does not check the size of the target
buffer but G_snprintf does (if used correctly).

> As for the magic number there, there is G_PATH_MAX or something, perhaps
> that would be more appropriate.

Yes, it would.

> If the issue is length of the path, then I understand why it was happening
> for the tests - they use long Mapset names and possibly also map long map
> names.

The length is one issue, but sprintf causes the segfault by writing
more than 50 chars if G_mapset() returns long mapset names as in case
of the temporary generated test mapsets.

Best regards
Soeren

>
>>
>>
>> http://pastebin.com/D8yautBh
>>
>> Best regards
>> Soeren
>>
>> 2015-06-26 20:27 GMT+02:00 Sören Gebbert <soerengebbert at googlemail.com>:
>> > Hi,
>> > here a traceback of a g.remove segfault that happens while testing
>> > temporal modules:
>> > http://pastebin.com/LDVKgEed
>> >
>> > Seems to be in M_do_remove calling some sort of sprintf?? Maybe line
>> > 102, since the colr2 buffer is only 50 chars in length ... however,
>> > sprintf() function shouldn't be used at all.
>> >
>> > Best regards
>> > Soeren
>> >
>> > 2015-06-26 16:11 GMT+02:00 Vaclav Petras <wenzeslaus at gmail.com>:
>> >>
>> >>
>> >> On Fri, Jun 26, 2015 at 6:09 AM, Maris Nartiss <maris.gis at gmail.com>
>> >> wrote:
>> >>>
>> >>> I can not repeat the issue with current trunk on ~AMD64 Gentoo.
>> >>> Please provide locale information; run g.remove under valgrind to see
>> >>> the source of memory corruption.
>> >>
>> >>
>> >> That's the hard part. I cannot run valgrind easily. I'm not getting the
>> >> error when I execute it manually (at least not often enough). It is
>> >> happening only with some part of tests for me. As I've written before,
>> >> I
>> >> originally haven't reported it at all because I'm getting with one copy
>> >> of
>> >> GRASS but not with the other on the same machine without being able to
>> >> find
>> >> any difference in between them. (Fortunately the copy which works is
>> >> the one
>> >> running the automated tests.) Only after Soeren and now Markus reported
>> >> the
>> >> same, I started to lean towards opinion that it is not a local issue.
>> >>
>> >> As for valgrind, that's unfortunately a feature request for gunittest.
>> >> Which
>> >> is even more advanced then I expected because valgrind is not need for
>> >> the
>> >> tested module but just for a helper one, anyway, the testing framework
>> >> should allow that.
>> >>
>> >> I'm not sure if Soeren can repeat it in more direct way.
>> >>
>> >>>
>> >>>
>> >>> Māris.
>> >>>
>> >>>
>> >>> 2015-06-25 3:47 GMT+03:00 Vaclav Petras <wenzeslaus at gmail.com>:
>> >>> >
>> >>> >
>> >>> > On Wed, Jun 24, 2015 at 9:29 AM, Vaclav Petras
>> >>> > <wenzeslaus at gmail.com>
>> >>> > wrote:
>> >>> >>
>> >>> >>
>> >>> >>
>> >>> >> On Wed, Jun 24, 2015 at 3:12 AM, Markus Neteler <neteler at osgeo.org>
>> >>> >> wrote:
>> >>> >>>
>> >>> >>> On Wed, Jun 24, 2015 at 9:06 AM, Markus Neteler
>> >>> >>> <neteler at osgeo.org>
>> >>> >>> wrote:
>> >>> >>> > Hi,
>> >>> >>> >
>> >>> >>> > to my surprise I managed to generate a segfault using g.remove:
>> >>> >>>
>> >>> >>> please disregard. Apparently a system update triggered this (no
>> >>> >>> idea
>> >>> >>> how).
>> >>> >>> "make distclean" helped.
>> >>> >>
>> >>> >>
>> >>> >> Well, you are not the only one with the problem:
>> >>> >>
>> >>> >> https://lists.osgeo.org/pipermail/grass-dev/2015-May/075083.html
>> >>> >>
>> >>> >> I will have to try distclean, although I think I tried it several
>> >>> >> times
>> >>> >> before.
>> >>> >>
>> >>> >
>> >>> > I'm getting the same issues again. Now I actually see a buffer
>> >>> > overflow:
>> >>> >
>> >>> > *** buffer overflow detected ***: g.remove terminated
>> >>> > ======= Backtrace: =========
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(+0x7338f)[0x7fc697b2b38f]
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(__fortify_fail+0x5c)[0x7fc697bc2c9c]
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(+0x109b60)[0x7fc697bc1b60]
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(+0x109069)[0x7fc697bc1069]
>> >>> >
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(_IO_default_xsputn+0xbc)[0x7fc697b3370c]
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(_IO_vfprintf+0x26b0)[0x7fc697b043a0]
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(__vsprintf_chk+0x84)[0x7fc697bc10f4]
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(__sprintf_chk+0x7d)[0x7fc697bc104d]
>> >>> >
>> >>> >
>> >>> > /home/vpetras/dev/grass/trunk/dist.x86_64-unknown-linux-gnu/lib/libgrass_manage.7.1.svn.so(M_do_remove+0
>> >>> > x2e8)[0x7fc6982e9148]
>> >>> > g.remove(main+0x68d)[0x401c5d]
>> >>> >
>> >>> > /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf5)[0x7fc697ad9ec5]
>> >>> > g.remove[0x401e3e]
>> >>> > ======= Memory map: ========
>> >>> > 00400000-00403000 r-xp 00000000 08:11 16127189
>> >>> > /home/vpetras/dev/grass/trunk/d
>> >>> > ist.x86_64-unknown-linux-gnu/bin/g.remove
>> >>> > 00602000-00603000 r--p 00002000 08:11 16127189
>> >>> > /home/vpetras/dev/grass/trunk/d
>> >>> > ist.x86_64-unknown-linux-gnu/bin/g.remove
>> >>> > 00603000-00604000 rw-p 00003000 08:11 16127189
>> >>> > /home/vpetras/dev/grass/trunk/d
>> >>> > ist.x86_64-unknown-linux-gnu/bin/g.remove
>> >>> > ...
>> >>> >
>> >>> > It fails with a lot of tests but with many other it works. I'm not
>> >>> > sure
>> >>> > if
>> >>> > all errors are stack overflow but I could catch all output the a
>> >>> > fine
>> >>> > next
>> >>> > time.
>> >>> >
>> >>> > Vaclav
>> >>> >
>> >>> >
>> >>> > _______________________________________________
>> >>> > grass-dev mailing list
>> >>> > grass-dev at lists.osgeo.org
>> >>> > http://lists.osgeo.org/mailman/listinfo/grass-dev
>> >>
>> >>
>> >>
>> >> _______________________________________________
>> >> grass-dev mailing list
>> >> grass-dev at lists.osgeo.org
>> >> http://lists.osgeo.org/mailman/listinfo/grass-dev
>
>