Bug 433756 - timidity++ -d0 crashes
timidity++ -d0 crashes
Status: CLOSED RAWHIDE
Product: Fedora
Classification: Fedora
Component: timidity++ (Show other bugs)
rawhide
All Linux
high Severity high
: ---
: ---
Assigned To: Jindrich Novy
Fedora Extras Quality Assurance
:
Depends On:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-21 05:16 EST by Andrew Bartlett
Modified: 2013-07-02 19:26 EDT (History)
2 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2008-02-21 07:13:23 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
Patch to fix the segfault (335 bytes, patch)
2008-02-21 05:16 EST, Andrew Bartlett
no flags Details | Diff
patch for the spec file too! (1.08 KB, patch)
2008-02-21 05:17 EST, Andrew Bartlett
no flags Details | Diff

  None (edit)
Description Andrew Bartlett 2008-02-21 05:16:50 EST
Description of problem:
timidity crashes due to free() of constant pointer

Version-Release number of selected component (if applicable):
TiMidity++-2.13.2

How reproducible:
Every time

Steps to Reproduce:
1. timidity -d0 foo.midi
2. 
3.
  
Actual results:
glibc detects double-free

Expected results:
midi playing

Additional info:
Perhaps an audit to find other cases might be worth while.

I've chosen to 'leak' a very small amount of memory in the rare case that -d is
specified twice.
Comment 1 Andrew Bartlett 2008-02-21 05:16:50 EST
Created attachment 295494 [details]
Patch to fix the segfault
Comment 2 Andrew Bartlett 2008-02-21 05:17:47 EST
Created attachment 295495 [details]
patch for the spec file too!
Comment 3 Jindrich Novy 2008-02-21 07:00:34 EST
The spec file patch was not cleanly applicable due to recent changes to
timidity++ so I modified it.

Maybe a better solution would be to originally strdup the constant string
otherwise the dynamic_lib_root value would be leaked.

I rewrote your patch to accomodate that.

Thanks!
Comment 4 Andrew Bartlett 2008-02-21 16:21:13 EST
Commenting only in the interests of computer science:

How would we have a leak?  We can certainly use more memory - if we always keep
two copies of the string - the static const default string, and the strdup()
default copy. 

The only 'leak' I can see is if a user specifies -d twice, and if they do that,
they get what they deserve :-)
Comment 5 Jindrich Novy 2008-02-22 04:35:36 EST
It's better to add code that is safe by principle than code that is safe only
after some assumptions if it is not hard or performance problematic. Consider
that upstream may change the code in a way that it uses dynamic_lib_root on some
other places than now, the problem you describe is still fixed, but leaks could
have been silently introduced.

Note You need to log in before you can comment on or make changes to this bug.