segfault opening pat

[rofl0r]

https://0x0.st/zHEJ.PAT

Program received signal SIGSEGV, Segmentation fault.
0x00007ffff7b4d8aa in convert_8sp (
    data=0x6d5a8f "\365\365\022\021\t\372\366\t\006\001\375\374\370", <incomplete sequence \356>, gus_sample=0x6a4ee0) at /home/rofl/wildmidi/src/gus_pat.c:98
98                  *write_data++ = (*read_data++) << 8;
(gdb) bt
#0  0x00007ffff7b4d8aa in convert_8sp (
    data=0x6d5a8f "\365\365\022\021\t\372\366\t\006\001\375\374\370", <incomplete sequence \356>, gus_sample=0x6a4ee0) at /home/rofl/wildmidi/src/gus_pat.c:98
#1  0x00007ffff7b50423 in _WM_load_gus_pat (
    filename=0x66e9e0 "/share/midi/propatches/HELICPTR.PAT", fix_release=0)
    at /home/rofl/wildmidi/src/gus_pat.c:878
#2  0x00007ffff7b5e25e in _WM_load_sample (sample_patch=0x66e940)
    at /home/rofl/wildmidi/src/sample.c:126
#3  0x00007ffff7b56863 in _WM_load_patch (mdi=0x7ffff7f82020, patchid=1917)
    at /home/rofl/wildmidi/src/patches.c:84
#4  0x00007ffff7b53928 in midi_setup_patch (mdi=0x7ffff7f82020, 
    channel=11 '\v', patch=125 '}')
    at /home/rofl/wildmidi/src/internal_midi.c:1628
#5  0x00007ffff7b5562a in _WM_SetupMidiEvent (mdi=0x7ffff7f82020, 
    event_data=0x69a33e "\203`\273[n\203`\233\060d\217", input_length=28, 
    running_event=0 '\000') at /home/rofl/wildmidi/src/internal_midi.c:2067
#6  0x00007ffff7b59ff7 in _WM_ParseNewMidi (midi_data=0x69a551 "", midi_size=0)
    at /home/rofl/wildmidi/src/f_midi.c:261
#7  0x00007ffff7b4a8d3 in WildMidi_Open (
    midifile=0x7fffffffed97 "/home/rofl/test.mid")
    at /home/rofl/wildmidi/src/wildmidi_lib.c:1692
#8  0x0000000000403553 in main (argc=4, argv=0x7fffffffeb08)
    at /home/rofl/wildmidi/src/wildmidi.c:1816
(gdb) p read_data
$1 = (uint8_t *) 0x6eed70 ""
(gdb) p *read_data
$2 = 0 '\000'
(gdb) p write_data
$3 = (int16_t *) 0x710002
(gdb) p *write_data
Cannot access memory at address 0x710002
(gdb) p *(write_data-1)
Cannot access memory at address 0x710000
(gdb) p *(write_data-2)
$4 = 0

[psi29a]

Woooo... thanks!

[sezero]

Hm, timidity (libtimidity) had no problems loading that pat.

[rofl0r]

the code in that function looks really bogus, i suspect it was never tested. 2 different ways to calculate patch length, and later on there are 2 loops doing stuff until datapos != endpos, without resetting datapos after the first loop.

[sezero]

If I apply the following patch the function succeeds:

diff --git a/src/gus_pat.c b/src/gus_pat.c
index 5fc3dbe..0b9a62b 100644
--- a/src/gus_pat.c
+++ b/src/gus_pat.c
@@ -94,7 +94,7 @@ static int convert_8sp(uint8_t *data, struct _sample *gus_sample) {
     gus_sample->data = (int16_t *) calloc((new_length + 2), sizeof(int16_t));
     if (__builtin_expect((gus_sample->data != NULL), 1)) {
         write_data = gus_sample->data;
-        do {
+        if (read_data < read_end) do {
             *write_data++ = (*read_data++) << 8;
         } while (read_data != read_end);
 
@@ -114,7 +114,7 @@ static int convert_8sp(uint8_t *data, struct _sample *gus_sample) {
         *write_data = (*read_data++ << 8);
         *write_data_b++ = *write_data;
         read_end = data + gus_sample->data_length;
-        if (__builtin_expect((read_data != read_end), 1)) {
+        if (__builtin_expect((read_data < read_end), 1)) {
             do {
                 *write_data_b++ = (*read_data++) << 8;
             } while (read_data != read_end);

The issue is gus_sample->loop_start is 0, and we set read_end as
data + gus_sample->loop_start, therefore the first loop tries
going forever, hence the segfault.

The if condition for the 3rd loop was wrong too: After correcting
the 1st loop, we reach the 3rd loop with loop_data at loop_end+1,
hence the equality check being wrong.

Other 8 bit 'ping pong' functions would need patching, too.

Comments?

[sezero]

Here is my simple test case, to directly test _WM_load_gus_pat():

#include <stdio.h>
extern unsigned char *_WM_load_gus_pat(const char *, int);
int main (void) {
    unsigned char *p = _WM_load_gus_pat("HELICPTR.PAT", 0);
    if (p) puts("SUCCESS");
    else puts("FAILED");
    return 0;
}

Obviously, you need to link to a static debug build of the library.

[sezero]

The gus patch file in question HELICPTR.PAT is ProPatches Lite:
https://retronn.de/imports/gus_config_guide.html
ftp://retronn.de/driver/Gravis/UltraSound/
Related files at there are ProPatchesLight150.rar, pp3_1-19.zip, and
pplt_160.zip.
The HELICPTR.PAT file is from ProPatchesLight150.rar or pplt_160.zip
Attaching here for convenience and so that it doesn't get lost.
HELICPTR.PAT.zip

[sezero]

If I apply the following patch the function succeeds:
[...]
Comments?

I only got an emoji reaction, huh.

Note that my patch only prevents the segfault, it doesn't have to sound correct at runtime
(and I would need either testers, e.g. @rofl0r, or a midi that actually uses that guspatch at
runtime.)

[rofl0r]

i can't really help there, i stumbled across this issue entirely accidentally. iirc i wanted to test how gravis ultrasound sounds, downloaded some patches and some midi library advertising to support them, and the first thing i tested resulted in the segfault. so i have actually never successfully played this stuff, and have no idea whether it will sound "right" or not.
from my point of view, if your patch fixes the crash, that's already a huge improvement so it should be merged.

[rofl0r]

took me about an hour to figure out what i did 1.5 years ago but here goes
install propatches (sabotage butch build recipe):

[mirrors]
http://web.archive.org/web/20170419131940/http://www.thierryb.net/site/IMG/zip/ProPatchesLite1.61.zip
http://www.thierryb.net/site/IMG/zip/ProPatchesLite1.61.zip

[vars]
filesize=6309158
sha512=5cf3e5bd1d0c6d3b05d69f8178e6b5e87b6fbf3fd0f87d6b25b0f4f07f49d74f4f132f0767202d186f907690ab7cd7368d5aca7a99f52c7351f4a7209542ceb0
tardir=.
pkgver=1

[build]
cat << 'EOF' > printsec.awk
#!/bin/awk -f
# use: $0 <file> <section>
# prints section of file
BEGIN {
  if (ARGC!=3) {
    print "error: require filename and sectionname" > "/dev/stderr"
    exit 1
  }
  print_sec(ARGV[1], ARGV[2])
  exit 0
}
function print_sec(file, sec,            r, insect) {
  insect=0
  while ((r = getline < file) == 1) {
    if (insect && /^\[.*\]$/) break
    if ($0 == "[" sec "]") {
      insect=1
      continue
    }
    if(insect) {
      if(/^\s*$/) continue
      print $0
    }
  }
  if (r == -1) {
    print "unable to read from " file > "/dev/stderr"
    exit 1
  }
  close(file)
}
EOF
chmod +x printsec.awk
unzip ProPatchesLite1.60/pplt_160.zip
unzip Update\ 160to161/pplt161u.zip
for i in ORCDRUM 808DRUM 909DRUM PWRDRUM BRSHDRUM JAZZDRUM ; do
mv pplt_160/$i/* pplt_160/
done
dos2unix pplt161u/ULTRASND.INI
ini() {
bankname="$1"
gusbank="$2"
startno="$3"
endno="$4"
echo "$bankname"
./printsec.awk pplt161u/ULTRASND.INI "$gusbank" > tmp
for n in `seq $startno $endno` ; do
fn=$(cat tmp | grep "^$n=" | cut -d '=' -f 2 | tr abcdefghijklmnopqrstuvwxyz ABCDEFGHIJKLMNOPQRSTUVWXYZ)
printf "      %03d   %s.PAT\n" $n "$fn"
done
}
printit() {
ini "bank 0" "Melodic Bank 0" 0 127
ini "drumset 0" "Drum Patches" 27 87
ini "drumset 1" "Drum Bank 9" 27 87
ini "drumset 2" "Drum Bank 17" 27 87
ini "drumset 3" "Drum Bank 25" 27 87
ini "drumset 4" "Drum Bank 26" 27 87
ini "drumset 5" "Drum Bank 33" 27 87
ini "drumset 6" "Drum Bank 41" 27 87
ini "drumset 7" "Drum Bank 49" 27 87
ini "drumset 8" "Drum Bank 0" 27 87
}
printit > propatches.cfg
dest="$butch_install_dir""$butch_prefix"
install -Dm 644 propatches.cfg "$dest"/etc/propatches/propatches.cfg
for i in pplt_160/*.PAT ; do
install -Dm 644 "$i" "$dest"/share/midi/propatches/"$(basename "$i")"
done

create /etc/wildmidi/wildmifi.cfg

dir /share/midi/propatches
source /etc/propatches/propatches.cfg

save this file https://0x0.st/idwY.mid as test.mid

run ./wildmidi -d default test.mid

your patch from #219 fixes the segfault, music plays and i can't hear anything odd.

EDIT at around 2:38 in the file, something that sounds like a helicopter can be heard.

[sezero]

Thanks!

Saving the test midi here, so that it doesn't get lost.
test.mid.zip

[sezero]

Fixed by 56f0b5c

[sezero]

Fix applied to wildmidi-0.3 branch too:
ba2dca8

[rofl0r]

sorry for the potentially stupid and offtopic question, but how can i configure wildmidi.cfg to use the sf2 file here ? https://github.com/Mindwerks/opl3-soundfont

[sezero]

sorry for the potentially stupid and offtopic question, but how can i configure wildmidi.cfg to use the sf2 file here ? https://github.com/Mindwerks/opl3-soundfont

You can not. No soundfont support yet.

[psi29a]

You can use unsf which converts sf to pat