Buffer overrun(?) in `ocaml_opus_decode_float`

[nagisa]

when running a liquidsoap based radio with an %opus encoder configured as such:

opus = %ogg(%opus(samplerate=48000, frame_size=60.))
output.harbor(opus, port=2048, mount="opus", radio)

I’m seeing a SIGSEGV occur rarely. This happens maybe once every 24 hours on average. There doesn’t seem to be any specific song or part of it that triggers it, at least as far as I can intuit.

The following is the stack trace I’ve been able to obtain from a coredump:

#0  0x0000000000c1f6a1 in ocaml_opus_encode_float ()
#1  0x0000000000a507a8 in camlOpus__fun_897 ()
#2  0x00000000006dc956 in camlOpus_encoder__data_encoder_455 ()
#3  0x00000000006e07d1 in camlOgg_muxer__encode_949 ()
#4  0x0000000000b78000 in camlStdlib__List__iter_507 ()
#5  0x00000000006df16b in camlOgg_encoder__encode_964 ()
#6  0x0000000000785a3d in camlOutput__output_chunks_1581 ()
#7  0x0000000000783a16 in camlOutput__fun_2241 ()
#8  0x00000000008a0ab2 in camlClock__fun_2268 ()
#9  0x0000000000b780d4 in camlStdlib__List__fold_left_521 ()
#10 0x000000000089e71a in camlClock__fun_2219 ()
#11 0x00000000008a07b2 in camlClock__loop_1348 ()
#12 0x000000000089ead8 in camlClock__fun_2196 ()
#13 0x00000000008d84f3 in camlTutils__process_832 ()
#14 0x0000000000b66eb5 in camlThread__fun_850 ()
#15 0x0000000000c722d5 in caml_start_program ()
#16 0x0000000000c683d0 in caml_callback_exn ()
#17 0x0000000000c494d8 in caml_thread_start ()
#18 0x00007f9155270e86 in start_thread () from libc.so.6
#19 0x00007f91552f7c60 in clone3 () from libc.so.6

The faulting instruction is the cvtsd2ss in what I believe is some sort of loop:

   0x0000000000c1f658 <+344>:	test   %r12d,%r12d
   0x0000000000c1f65b <+347>:	jle    0xc1f6c0 <ocaml_opus_encode_float+448>
   0x0000000000c1f65d <+349>:	mov    0x48(%rsp),%r11
   0x0000000000c1f662 <+354>:	mov    0x10(%rsp),%r8d
   0x0000000000c1f667 <+359>:	mov    %r15d,%r9d
   0x0000000000c1f66a <+362>:	xor    %r10d,%r10d
   0x0000000000c1f66d <+365>:	sub    0x2c(%rsp),%r9d
   0x0000000000c1f672 <+370>:	nopw   0x0(%rax,%rax,1)
   0x0000000000c1f678 <+376>:	test   %ebx,%ebx
   0x0000000000c1f67a <+378>:	jle    0xc1f6b1 <ocaml_opus_encode_float+433>
   0x0000000000c1f67c <+380>:	movslq %r9d,%rsi
   0x0000000000c1f67f <+383>:	mov    %r10d,%eax
   0x0000000000c1f682 <+386>:	mov    %r11,%rdx
   0x0000000000c1f685 <+389>:	shl    $0x3,%rsi
   0x0000000000c1f689 <+393>:	nopl   0x0(%rax)
   0x0000000000c1f690 <+400>:	mov    (%rdx),%rdi
   0x0000000000c1f693 <+403>:	movslq %eax,%rcx
   0x0000000000c1f696 <+406>:	pxor   %xmm0,%xmm0
   0x0000000000c1f69a <+410>:	add    $0x1,%eax
   0x0000000000c1f69d <+413>:	add    $0x8,%rdx
=> 0x0000000000c1f6a1 <+417>:	cvtsd2ss (%rdi,%rsi,1),%xmm0
   0x0000000000c1f6a6 <+422>:	movss  %xmm0,0x0(%rbp,%rcx,4)
   0x0000000000c1f6ac <+428>:	cmp    %eax,%r8d
   0x0000000000c1f6af <+431>:	jne    0xc1f690 <ocaml_opus_encode_float+400>
   0x0000000000c1f6b1 <+433>:	add    $0x1,%r9d
   0x0000000000c1f6b5 <+437>:	add    %r13d,%r10d
   0x0000000000c1f6b8 <+440>:	add    %r13d,%r8d
   0x0000000000c1f6bb <+443>:	cmp    %r15d,%r9d
   0x0000000000c1f6be <+446>:	jne    0xc1f678 <ocaml_opus_encode_float+376>

This is the only appearance of the cvtsd2ss in the function, so I suspect it corresponds to this code here:

ocaml-opus/src/opus_stubs.c

Lines 763 to 767 in 3e1a566

	for (i = 0; i < loops; i++) {
	for (j = 0; j < frame_size; j++)
	for (c = 0; c < chans; c++)
	pcm[chans * j + c] =
	Double_field(Field(buf, c), off + j + i * frame_size);

The following are the GPRs of interest:

rax            0x1302              4866
rbx            0x2                 2
rcx            0x1301              4865
rdx            0x7f915112fe00      140262107184640
rsi            0x4c00              19456
rdi            0x7f913a7f8400      140261728420864
rbp            0x7f912402ce80      0x7f912402ce80
rsp            0x7f914951b520      0x7f914951b520
r8             0x1302              4866
r9             0x980               2432
r10            0x1300              4864
r11            0x7f915112fdf0      140262107184624
r12            0xb40               2880
r13            0x2                 2
r14            0x7f91240b8740      140261351720768
r15            0xb40               2880

---

Trying to reason about the assembly, it would seem to me that the issue is with how the size of the input buffer is computed. Maybe the problem is with the fact that the input vector is _len - _off bytes long, rather than _len bytes long? The int loops = len / frame_size; would seem suspect in that case.

[toots]

Thanks for this and the detailed analysis. Couple of remarks before I do a deeper dive here:

• Do you think it is possible that some of the data could be weird? We've had issues with lame segfaulting on NaN float values. Wondering if this could be the case here as well
• Have you tried to add debugging symbols to the stacktrace, this would be quite useful I believe. If you're installing via our own package we provide the corresponding package with debugging symbols. They should also be provided in our production docker image.

[nagisa]

Have you tried to add debugging symbols to the stacktrace, this would be quite useful I believe. If you're installing via our own package we provide the corresponding package with debugging symbols. They should also be provided in our production docker image.

I built the package and deps on my own via opam. I could try debuginfo and some rr I guess, but I can’t promise being able to get to investigating this again anytime soon.

Do you think it is possible that some of the data could be weird? We've had issues with lame segfaulting on NaN float values. Wondering if this could be the case here as well

Plausible. I’m actually surprised I'm triggering the float pcm path and not always going off to s16le pcm, given that my library is exclusively flacs...

[toots]

Yeah that sounds like it. I understand that time is limited. If I push a tentative fix (cleanup NaN from input), would you have a second to test it? Looks like you can reproduce fairly reliably.

[nagisa]

Yeah, I can test anything as long as there is a commit in any git repository I can reference.

[toots]

Just getting back to this! You can try the latest commit on the main branch: 6329a0b

[nagisa]

Hm, I made an attempt to re-reproduce the problem without the patch, to make sure I’m validating the right thing. Alas, to no avail :(

Either something changed with liquidsoap, how it is built, or I just don’t remember what exactly led to the failure. I lost the full reproducer liq and the nix derivation that built the originally faulty liquidsoap. I reproduced the config I think I had from my memory, but that’s quite unlikely to be quite the same after 2 months.

So for now this is no longer a problem. I’ll close this and reopen/retest if I ever come across a crash again.

[toots]

Well I'm glad to know that the problem is solved, thanks for checking in!