Bug#840754: Memory leak in libmagic and failure in magic_load

[Arnaud Quette]

2016-12-02 0:04 GMT+01:00 Christoph Biedl :
> Arnaud Quette wrote...
>
>> $ gcc test.c -lmagic
>> valgrind ./a.out
> (...)
>> ==30967== HEAP SUMMARY:
>> ==30967== in use at exit: 48 bytes in 3 blocks
>> ==30967== total heap usage: 28 allocs, 25 frees, 2,179 bytes allocated
> (...)
>
> Hello,
>
> according to my tests this was fixed in file/libmagic 5.29-1 (as in
> sid and stretch):
>
> ==416629== Memcheck, a memory error detector
> ==416629== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
> ==416629== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright 
> info
> ==416629== Command: ./a.out
> ==416629==
> ==416629==
> ==416629== HEAP SUMMARY:
> ==416629== in use at exit: 0 bytes in 0 blocks
> ==416629==   total heap usage: 16 allocs, 16 frees, 1,123 bytes allocated
> ==416629==
> ==416629== All heap blocks were freed -- no leaks are possible
> ==416629==
> ==416629== For counts of detected and suppressed errors, rerun with: -v
> ==416629== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
>
> Please confirm, I'll prepare a fix for jessie then (wheezy is
> appearently not affected).

Hi Christoph,

I confirm that libmagic 5.29-1 (tested on stretch) fixes the issue.

thanks and cheers,
Arno
-- 
Eaton Data Center Automation Solutions - Opensource Leader - http://42ity.org
NUT (Network UPS Tools) Project Leader - http://www.networkupstools.org
Debian Developer - http://www.debian.org
Free Software Developer - http://arnaud.quette.fr

Bug#840754: Memory leak in libmagic and failure in magic_load

[Christoph Biedl]

Arnaud Quette wrote...

> $ gcc test.c -lmagic
> valgrind ./a.out
(...)
> ==30967== HEAP SUMMARY:
> ==30967== in use at exit: 48 bytes in 3 blocks
> ==30967== total heap usage: 28 allocs, 25 frees, 2,179 bytes allocated
(...)

Hello,

according to my tests this was fixed in file/libmagic 5.29-1 (as in
sid and stretch):

==416629== Memcheck, a memory error detector
==416629== Copyright (C) 2002-2015, and GNU GPL'd, by Julian Seward et al.
==416629== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright 
info
==416629== Command: ./a.out
==416629==
==416629==
==416629== HEAP SUMMARY:
==416629== in use at exit: 0 bytes in 0 blocks
==416629==   total heap usage: 16 allocs, 16 frees, 1,123 bytes allocated
==416629==
==416629== All heap blocks were freed -- no leaks are possible
==416629==
==416629== For counts of detected and suppressed errors, rerun with: -v
==416629== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Please confirm, I'll prepare a fix for jessie then (wheezy is
appearently not affected).

Christoph



signature.asc
Description: Digital signature

Bug#840754: Memory leak in libmagic and failure in magic_load

[Christoph Biedl]

Arnaud Quette wrote...

> looking closer at the patch I pointed, it's very probably not suitable.

Now I see a patch. I'll take a close look tomorrow

> IMHO, it's Debian related, though I've not tested on Redhat nor others.

As far as I understand it the leak is caused by the fact Debian uses
a different compile-time option for MAGIC: Two items, separated by ':';
and the code that splits this has the leak.

If this is true, it's technically upstream although it does not
manifest there. But is should happen in Redhat, too.

> again, the simple solution, while waiting for fixing the code, may be
> to export MAGIC=/usr/share/misc/magic

Yes, but that's just a workaround. I want to identify the underlying
issue, and fix it.

Christoph


signature.asc
Description: Digital signature

Bug#840754: Memory leak in libmagic and failure in magic_load

[Arnaud Quette]

Hi Christoph,

2016-10-17 20:21 GMT+02:00 Christoph Biedl :
> tags 840754 confirmed moreinfo
> thanks
>
> Arnaud Quette wrote...
>
>> ==30967== definitely lost: 48 bytes in 1 blocks
>
> Confirmed, can reproduce this from jessie on, not in wheezy though.
>
>> - Redhat has a similar ticket which they solved:
>> https://bugzilla.redhat.com/show_bug.cgi?id=919466
>
> Unfortunately I fail to see the actual fix for this (and I'm also
> somewhat surprised why it never made upstream, but that's a different
> story).
>
> Since you added the patch tag, do you have some details available?

looking closer at the patch I pointed, it's very probably not suitable.
IMHO, it's Debian related, though I've not tested on Redhat nor others.
As per my original report, the issue is with the return value of
get_default_magic(), which is "/etc/magic" instead of the documented
default "/usr/share/misc/magic" (using "MAGIC=/usr/share/misc/magic
valgrind ./a.out" shows no leak).
Not sure how other distros ships the magic.mgc
Another point is that removing (or renaming) /etc/magic also fixes the issue.

I was not able to hunt the bug yet, but here is some details.
- magic.c->magic_getpath return "/etc/magic:/usr/share/misc/magic"
- the processing of /etc/magic goes fine, but the loops then leak when
processing /usr/share/misc/magic
- the leak happen in apprentice.c->apprentice_1() when calling
add_mlist(). Still not sure why, I don't quite get the deeper logic of
this code.

I'll try to dig more and find a suitable patch, but kids vacations are
on the way in the short run...

again, the simple solution, while waiting for fixing the code, may be
to export MAGIC=/usr/share/misc/magic

Do you want me to remove the patch tag, or do you consider the
"export" solution as being a patch?

cheers,
Arno
-- 
Eaton Data Center Automation Solutions - Opensource Leader
NUT (Network UPS Tools) Project Leader - http://www.networkupstools.org
Debian Developer - http://www.debian.org
Free Software Developer - http://arnaud.quette.fr

Bug#840754: Memory leak in libmagic and failure in magic_load

[Christoph Biedl]

tags 840754 confirmed moreinfo
thanks

Arnaud Quette wrote...

> ==30967== definitely lost: 48 bytes in 1 blocks

Confirmed, can reproduce this from jessie on, not in wheezy though.

> - Redhat has a similar ticket which they solved:
> https://bugzilla.redhat.com/show_bug.cgi?id=919466

Unfortunately I fail to see the actual fix for this (and I'm also
somewhat surprised why it never made upstream, but that's a different
story).

Since you added the patch tag, do you have some details available?

Christoph


signature.asc
Description: Digital signature

Bug#840754: Memory leak in libmagic and failure in magic_load

[Arnaud Quette]

Package: libmagic1
Version: 1:5.22+15-2+deb8u2
Severity: normal

libmagic suffers from a memory leak, apparently due to magic_load()
not satisfying its contract:0

As per man magic_load:
--
The magic_load() function must be used to load the the colon separated
list of database files passed in as filename, or NULL for the default
database file before any magic queries can performed.

The default database file is named by the MAGIC environment variable.
If that variable is not set, the default database file name is
/usr/share/misc/magic.  magic_load() adds “.mgc” to the database
filename as appropriate.
--

Steps to reproduce:

$ cat test.c
#include 
#include 

int main ()
{ magic_t _magic = magic_open (MAGIC_MIME); int r = magic_load
(_magic, NULL); magic_close (_magic); }

$ gcc test.c -lmagic
valgrind ./a.out
==30967== Memcheck, a memory error detector
==30967== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==30967== Using Valgrind-3.10.0 and LibVEX; rerun with -h for copyright info
==30967== Command: ./a.out
==30967==
==30967==
==30967== HEAP SUMMARY:
==30967== in use at exit: 48 bytes in 3 blocks
==30967== total heap usage: 28 allocs, 25 frees, 2,179 bytes allocated
==30967==
==30967== LEAK SUMMARY:
==30967== definitely lost: 48 bytes in 1 blocks
==30967== indirectly lost: 0 bytes in 2 blocks
==30967== possibly lost: 0 bytes in 0 blocks
==30967== still reachable: 0 bytes in 0 blocks
==30967== suppressed: 0 bytes in 0 blocks
==30967== Rerun with --leak-check=full to see details of leaked memory
==30967==
==30967== For counts of detected and suppressed errors, rerun with: -v
==30967== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

To fix the above, either:
- use "MAGIC=/usr/share/misc/magic valgrind ./a.out"
- or modify the above test code "magic_load (_magic, "/usr/share/misc/magic")

It turns out the private function get_default_magic() returns
"/etc/magic" instead of the document default "/usr/share/misc/magic",
which in turns generates some lost blocks.

Solution:

Either fix upstream or export the MAGIC environment variable (probably
better / easier considering the code!).

Side notes:
- Reported with details by Michal Vyskocil from the Eaton Opensource Team
- Redhat has a similar ticket which they solved:
https://bugzilla.redhat.com/show_bug.cgi?id=919466

cheers,
Arno
-- 
Eaton Data Center Automation Solutions - Opensource Leader
NUT (Network UPS Tools) Project Leader - http://www.networkupstools.org
Debian Developer - http://www.debian.org
Free Software Developer - http://arnaud.quette.fr