Bug 93343

Version:           kdemultimedia-3.3.1, /usr/lib/libakode.so.1.0.0 (using KDE KDE 3.3.1)
Installed from:    Unlisted Binary Package
Compiler:          unknown N/A
OS:                Linux

There seems to be a problem in libakode when reading mmapped files.  I
suspect the problem occurs if the file system does not support mmap.

Here's the general description of the problem and scenarios. Below you
will find a debugger trace for artsd (the test application in this
example). Towards the end there is a code excerpt where I believe the
problem is.  Sorry, no diff.

Here's the setup:
OS: Linux 2.4.26
KDE 3.3.1, compiled by "The KDE-RedHat Project" http://kde-redhat.sf.net.
arts version: 1.3.1
kdemultimedia version: 3.3.1
akode version: /usr/lib/libakode.so.1.0.0


These are 2 scenarios where things go bad:

Scenario 1.
Result: system freeze
Description:

The music files, more specifically in MP3 format, are located in a NTFS
partition.  Assume the artsd daemon was launched by the kde scripts at the
start ot the session.  If one tries to play one of such files using juk,
artsplay, noatun, etc., the whole machine freezes.  For example, it does
not accept any additional incoming ssh connections.  However, it responds
to pings.

Scenario 2.
Result: application (artsd) crashes with SIGSEGV

Description:

Now the arts daemon is launched within a debugger.  The music files, more
specifically in MP3 format, are located in a NTFS partition.  If one tries
to play one of such files using juk, artsplay, noatun, etc., artsd crashes
with SIGSEGV.  Trace provided below.

Here's a scenario where things go well.

Scenario 3.
Result: files are played succesfully.
Play the file from a different partition, e.g., iso9660 or ext2.
The file is played with no problems.

Here's the stack trace produced by gdb for artsd:

-------------------------------------------------
[New Thread 32769 (LWP 924)]
[New Thread 16386 (LWP 925)]

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 16386 (LWP 925)]
0x407ed54a in memcpy () from /lib/i686/libc.so.6
(gdb) where
#0  0x407ed54a in memcpy () from /lib/i686/libc.so.6
#1  0x40a5c95d in aKode::MMapFile::read(char*, long) () from /usr/lib/libakode.so.1
#2  0x40a63acc in aKode::MPEGDecoder::skipID3v2() ()
   from /usr/lib/libakode_mpeg_decoder.so
#3  0x40a63f52 in aKode::MPEGDecoder::prepare() () from /usr/lib/libakode_mpeg_decoder.so
#4  0x40a642d9 in aKode::MPEGDecoder::readFrame(aKode::AudioFrame*) ()
   from /usr/lib/libakode_mpeg_decoder.so
#5  0x40a5db9f in aKode::StreamToFrameDecoder::audioConfiguration() ()
   from /usr/lib/libakode.so.1
#6  0x40632a21 in pthread_start_thread () from /lib/i686/libpthread.so.0
#7  0x40632b25 in pthread_start_thread_event () from /lib/i686/libpthread.so.0
(gdb)

---------------------------------------------

Artsd crashes in memcpy inside aKode::MMapFile::read(char*, long).

Now, taking a look at kdemultimedia/akode/lib/mmapfile.cpp, it seems that
the problem lies in the openRO method, please look at the comments added
inside the code excerpts:

bool MMapFile::openRO() {
    if(handle) return true;
    struct stat stat;

    fd = ::open(filename, O_RDONLY);
    if (fstat(fd, &stat) < 0) return false;
    len = stat.st_size;
    pos = 0;

    handle = mmap(0, len, PROT_READ, MAP_SHARED, fd, 0);
    if (handle == MAP_FAILED) {  /* <------ HERE !!! */

       /* MAP_FAILED is -1 and handle is never set to NULL,
        * which MMapFile::read checks for later
        */

        ::close(fd);
        return false;
    }

    return true;
}


long MMapFile::read(char* ptr, long num) {

    /* Notice that handle was not set NULL if the mmap failed in
     * openRO, thus the following check will succeed even if handle
     * is not valid.
     */

    if(!handle) return -1;

    if (pos+num > len) num = len-pos;

    /* and of course, this address (handle+pos) won't be valid
     * if mmap failed
     */

    memcpy(ptr, (char*)handle+pos, num);
    pos += num;

    return num;
}

--------------------------------

Assuming this is the real problem, the first fix is to make sure 'handle' is
asigned NULL in openRO when mmap fails.  This would prevent 'MMapFile::read'
from trying to read from an ilegal memory address.

However, the problem is probably deeper than that and requires additional
fixes.  When mmap fails, openRO returns false.  It seems that the
caller of openRO is not checking the return code and proceeded to perform
a read regardless of the fact that the operation failed.  This means that
other modifications are needed where openRO is called.

Finally, I'm not sure about whether this is implemented, but if not, I'd
like to add it to the wish list.  If mmap fails or it is not supported for
some reason, there should be a safe fallback mechanism to old plain file
read(), even if it is slower.  And, please don't tell me that the solution
is to put all my music files in a file system that support mmap.

Finally, I want to include my rant about the bug tracking system ... it's
a pain all the steps one has to go through to submit a bug description,
and the response time (of the bug tracking system) is very low.