Bug#821321: libvorbisfile: ov_pcm_seek wrongly returns OV_EOF or segfaults sometimes

[Martin Steghöfer]

Hi Frank,

Thank you for your bug report!

Judging from the code that reproduces the bug (two subsequent seeks to 
0) and from the related vorbis code you are mentioning, this sounds a 
lot like #782831, which was fixed in 1.3.4-3. Could you confirm or 
refute that theory by testing your code (I guess you have further 
examples apart from the one you posted) with 1.3.4-3?

Cheers,
Martin


El 17/04/16 a les 18:56, Frank Heckenbach ha escrit:
> Package: libvorbisfile3
> Version: 1.3.4-2
> Severity: important
>
> ov_pcm_seek wrongly returns OV_EOF or segfaults sometimes. I've
> observed it in some situations, below is a very simple one to
> reproduce. It's an important problem to me, because (unless fixed or
> you can tell me exactly when seeking will work), I'd have to treat
> all Ogg/Vorbis files as unseekable in my code, which would be a huge
> performance penalty (decoding sequentially and buffering all in
> memory).
>
> % cat test.c
> #include <stdio.h>
> #include <vorbis/vorbisfile.h>
>
> int main ()
> {
>    OggVorbis_File vf;
>    fprintf (stderr, "%i\n", ov_fopen ("foo.ogg", &vf));
>    fprintf (stderr, "%i\n", ov_pcm_seek (&vf, 0));
>    fprintf (stderr, "%i\n", ov_pcm_seek (&vf, 0));
>    return 0;
> }
> % head -c 100000 /dev/zero|oggenc -Q -r - -o foo.ogg&& gcc -g test.c -lvorbisfile && ./a.out
>
> On i386:
> 0
> 0
> -2
>
> On amd64:
> 0
> 0
> Segmentation fault
>
> I tried to debug it and found: The 2nd time ov_pcm_seek_is_called
> (from the 2nd ov_pcm_seek call), at line 1461
>
>        if(bisect!=vf->offset){
>          result=_seek_helper(vf,bisect);
>          if(result) goto seek_error;
>        }
>
> begin == 3997 and vf->offset == 3997, so the call to _seek_helper is
> skipped. However, ftell((FILE*)vf->datasource) == 4076, so it goes
> on with a wrong file position, so _get_next_page fails and og
> remains unintialized and ogg_page_serialno(&og) (line 1554) results
> in UB.
>
> I don't really understand the code: Telling from this line,
> vf->offset should reflect the actual position of the data source.
> But if so, I'd expect it to be adjusted after each successfull call
> of seek_func (that's correctly done) and read_func. read_func is
> only called from _get_data, but it doesn't adjust vf->offset.
> Instead it puts the data into an internal buffer AFAIUI, so the
> users of the data from the buffer are probably responsible for
> adjusting vf->offset, but apparently something goes wrong there.
>
> If I just set vf->offset to 4076 before line 1461 (2nd time), it
> continues correctly. That's of course, not a fix, just an indication
> that the wrong value of vf->offset is the real problem here.
>
> Maybe vf->offset just needs to be revalidated before line 1461, but
> it's used in many places, and I don't know how many of them might be
> affected too.
>
> -- System Information:
> Debian Release: 8.4
>    APT prefers stable-updates
>    APT policy: (500, 'stable-updates'), (500, 'proposed-updates'), (500, 'stable')
> Architecture: amd64 (x86_64)
>
> Kernel: Linux 3.16.0-4-amd64 (SMP w/8 CPU cores)
> Locale: LANG=de_DE, LC_CTYPE=de_DE (charmap=ISO-8859-1)
> Shell: /bin/sh linked to /bin/dash
> Init: systemd (via /run/systemd/system)
>
> Versions of packages libvorbisfile3:amd64 depends on:
> ii  libc6              2.19-18+deb8u4
> ii  libogg0            1.3.2-1
> ii  libvorbis0a        1.3.4-2
> ii  multiarch-support  2.19-18+deb8u4
>
> libvorbisfile3:amd64 recommends no packages.
>
> libvorbisfile3:amd64 suggests no packages.
>
> -- no debconf information
>