[pulseaudio-discuss] ABI breakage in introspect (85e7fbc196f4424f68e530c2e3a01d9b941f293e)

[Tanu Kaskinen]

On Thu, 2014-01-09 at 13:27 +0000, Colin Guthrie wrote:
> [ Solved, Tanu is on the ball once again! Ignore the pointless patch (it
> truely is pointless) ]
> 
> See below if you want more info.
> 
> 'Twas brillig, and Tanu Kaskinen at 09/01/14 12:50 did gyre and gimble:
> > On Thu, 2014-01-09 at 10:07 +0000, Colin Guthrie wrote:
> >> Hi,
> >>
> >> In order to get Bluetooth support (at least A2DP) I've been tracking the
> >> 5.0 release for Mageia (it's generally an unfortunate situation but such
> >> is life).
> >>
> >> Sadly there appears to be some ABI breakage introduced related to card
> >> profiles.
> >>
> >> This is purportedly fixed by the below commit but I think there are
> >> still issues.
> >>
> >> I've had crashes reported in both KDE's kcm_phonon (the bits which which
> >> I wrote) and pavucontrol directly, both related to updating card info.
> >> It seems invalid string information is passed in to string compare
> >> functions and they crash.
> >>
> >> https://bugs.mageia.org/show_bug.cgi?id=12155
> > 
> > Here's a snippet of the pavucontrol backtrace:
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > __strcmp_ssse3 () at ../sysdeps/i386/i686/multiarch/strcmp-ssse3.S:240
> > 240             movzbl  (%eax), %ecx
> > (gdb) bt
> > #0  __strcmp_ssse3 () at ../sysdeps/i386/i686/multiarch/strcmp-ssse3.S:240
> > #1  0x0806daa0 in operator() (this=0xbfffe80c, lhs=..., lhs=..., rhs=..., 
> >     rhs=...) at mainwindow.cc:43
> > #2  _M_get_insert_unique_pos (__k=..., this=0xbfffe80c)
> >     at /usr/include/c++/4.8.2/bits/stl_tree.h:1324
> > #3  std::_Rb_tree<pa_card_profile_info, pa_card_profile_info, std::_Identity<pa_card_profile_info>, profile_prio_compare, std::allocator<pa_card_profile_info> >::_M_insert_unique (this=0xbfffe80c, __v=...)
> >     at /usr/include/c++/4.8.2/bits/stl_tree.h:1377
> > #4  0x08069947 in insert (__x=..., this=0xbfffe80c)
> >     at /usr/include/c++/4.8.2/bits/stl_set.h:463
> > #5  MainWindow::updateCard (this=0x822e688, info=...) at mainwindow.cc:319
> > #6  0xb7422e33 in context_get_card_info_callback (pd=pd at entry=0x8133208, 
> >     command=command at entry=2, tag=tag at entry=5, t=t at entry=0x825f930, 
> >     userdata=userdata at entry=0x82c94f8) at pulse/introspect.c:971
> > 
> > One of the parameters of std::_Rb_tree is
> > std::allocator<pa_card_profile_info>, and that looks like the data
> > structure allocates pa_card_profile_info objects. That's not safe. It
> > looks like pavucontrol is buggy.
> 
> Just out of interest, why is that not safe? Is allocating a struct not
> safe in a set?
> 
> Various bits I've read seem to suggest this isn't a problem.

Right, I was concerned with the set allocating too small structs if
libpulse has added more fields to it, but actually allocating too small
structs does no harm because pavucontrol won't access the new fields
anyway. So, pavucontrol isn't buggy after all. The problem here was
simply that a field was *removed* from the struct, and that naturally
broke the ABI compatibility.

-- 
Tanu