Opened 3 years ago

Closed 2 years ago

#2311 closed defect (fixed)

strdup allocates without free-ing

Reported by: nobody Owned by: beastd
Priority: normal Component: core
Version: HEAD Severity: minor
Keywords: Cc:
Blocked By: Blocking:
Reproduced by developer: yes Analyzed by developer: yes

Description

=================================================================
==26396==ERROR: LeakSanitizer?: detected memory leaks

Direct leak of 22 byte(s) in 1 object(s) allocated from:

#0 0x5588c9417e90 in interceptor_strdup (/tmp/mplayer-export-2016-12-19/mplayer+0x14ee90)
#1 0x5588c9525719 in copy_str /tmp/mplayer-export-2016-12-19/m_option.c:419:27

Direct leak of 15 byte(s) in 1 object(s) allocated from:

#0 0x5588c9417e90 in interceptor_strdup (/tmp/mplayer-export-2016-12-19/mplayer+0x14ee90)
#1 0x5588c95bec9a in get_term_charset /tmp/mplayer-export-2016-12-19/osdep/getch2.c:317:15
#2 0x7f63943ab2b0 in
libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer?: 37 byte(s) leaked in 2 allocation(s).

Change History (6)

comment:1 Changed 3 years ago by beastd

  • Status changed from new to open

Thank you for the report.

I guess you compiled mplayer with "-fsanitize=address" and just ran mplayer without any options.

Would have been good if you had provided info on what you did and pasted complete output, so we would know how to reproduce and which version of mplayer you were using. At least the path /tmp/mplayer-export-2016-12-19 is a hint.

I have no idea about the first leak yet (m_option.c), AFAICS it seems not to add up.

The second leak (getch2.c) is rather strange. Could even be something more serious like a memory corruption, but I don't know yet. AFAIK the get_term_charset function is only called in mp_msg.c inside mp_msg_init and the result is saved in a global variable, but when mp_msg_uninit is invoked that variable is already NULL and I do not see where that happens.

comment:2 Changed 3 years ago by nobody

To summarize, for a while I have been using mplayer in a terminal as my music player and I was wondering why all the tags were restricted to a total of 40 characters max no matter how large the terminal was so I went spelunking into the code (for get_metadata()) and noticed the strdup-s which I vaguely remember the manpage telling me about it needing to be free-d, and considering how large mplayer is with ffmpeg too, I just added '-O1 -g -fsanitize=address -fno-omit-frame-pointer' to the line when compiling mplayer.c (because get_metadata() calls get_demuxer_info() which calls strdup() which doesn't seem to be indicated in this bug report (maybe it's not a bug, I'm also not good at programming)).

comment:3 follow-up: Changed 3 years ago by nobody

This is the result of adding -O1 -g -fsanitize=address -fno-omit-frame-pointer to just the mplayer objects being compiled and not ffmpeg.

==26480==ERROR: LeakSanitizer?: detected memory leaks

Direct leak of 22 byte(s) in 1 object(s) allocated from:

#0 0x55f8808be060 in interceptor_strdup (/tmp/mplayer-export-2016-12-19/mplayer+0x3ab060)
#1 0x55f880a56401 in copy_str /tmp/mplayer-export-2016-12-19/m_option.c:419:27
#2 0x55f880a533db in m_config_add_option /tmp/mplayer-export-2016-12-19/m_config.c:355:7
#3 0x55f880a5203d in m_config_register_options /tmp/mplayer-export-2016-12-19/m_config.c:380:5
#4 0x55f880980741 in main /tmp/mplayer-export-2016-12-19/mplayer.c:2796:5
#5 0x7fbb58c952b0 in
libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

Direct leak of 15 byte(s) in 1 object(s) allocated from:

#0 0x55f8808be060 in interceptor_strdup (/tmp/mplayer-export-2016-12-19/mplayer+0x3ab060)
#1 0x55f880be5835 in get_term_charset /tmp/mplayer-export-2016-12-19/osdep/getch2.c:317:15
#2 0x55f880a5e488 in mp_msg_init /tmp/mplayer-export-2016-12-19/mp_msg.c:90:24
#3 0x55f8809806f1 in main /tmp/mplayer-export-2016-12-19/mplayer.c:2791:5
#4 0x7fbb58c952b0 in
libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

SUMMARY: AddressSanitizer?: 37 byte(s) leaked in 2 allocation(s).
Exit 1

comment:4 in reply to: ↑ 3 Changed 3 years ago by beastd

  • Analyzed by developer set
  • Reproduced by developer set

Replying to nobody:

This is the result of adding -O1 -g -fsanitize=address -fno-omit-frame-pointer to just the mplayer objects being compiled and not ffmpeg.

==26480==ERROR: LeakSanitizer?: detected memory leaks

Direct leak of 22 byte(s) in 1 object(s) allocated from:

#0 0x55f8808be060 in interceptor_strdup (/tmp/mplayer-export-2016-12-19/mplayer+0x3ab060)
#1 0x55f880a56401 in copy_str /tmp/mplayer-export-2016-12-19/m_option.c:419:27
#2 0x55f880a533db in m_config_add_option /tmp/mplayer-export-2016-12-19/m_config.c:355:7
#3 0x55f880a5203d in m_config_register_options /tmp/mplayer-export-2016-12-19/m_config.c:380:5
#4 0x55f880980741 in main /tmp/mplayer-export-2016-12-19/mplayer.c:2796:5
#5 0x7fbb58c952b0 in
libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

This one is because of the option codecpath and...

Direct leak of 15 byte(s) in 1 object(s) allocated from:

#0 0x55f8808be060 in interceptor_strdup (/tmp/mplayer-export-2016-12-19/mplayer+0x3ab060)
#1 0x55f880be5835 in get_term_charset /tmp/mplayer-export-2016-12-19/osdep/getch2.c:317:15
#2 0x55f880a5e488 in mp_msg_init /tmp/mplayer-export-2016-12-19/mp_msg.c:90:24
#3 0x55f8809806f1 in main /tmp/mplayer-export-2016-12-19/mplayer.c:2791:5
#4 0x7fbb58c952b0 in
libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x202b0)

...this because of the option msgcharset.

SUMMARY: AddressSanitizer?: 37 byte(s) leaked in 2 allocation(s).
Exit 1

MPlayer's config system (m_config.c, m_option.c, etc.) allocates its own buffers on the heap for e.g. string-type options. But the initial values of the variables in which these two options are stored are already allocated on the heap before the options get registered. Thus by overwriting the original pointers with pointers to its own buffers the config system clearly creates (typically small) memory leaks because the original buffers are no longer reachable.

comment:5 Changed 3 years ago by beastd

The msgcharset leak was fixed by Reimar in SVN commit 37908 .

I proposed a patch for fixing the codecpath leak on the development mailing list:

http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/2017-January/073542.html

comment:6 Changed 2 years ago by beastd

  • Resolution set to fixed
  • Status changed from open to closed

The codecpath leak should be fixed with SVN commit 37927 .

Note: See TracTickets for help on using tickets.