Bug 115399 - Data race in typelib_typedescription_register
Summary: Data race in typelib_typedescription_register
Status: UNCONFIRMED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: sdk (show other bugs)
Version:
(earliest affected)
4.1.4.2 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: needsDevAdvice
Depends on:
Blocks:
 
Reported: 2018-02-02 11:54 UTC by straub
Modified: 2018-03-27 11:35 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description straub 2018-02-02 11:54:14 UTC
cppu/source/typelibtypelib.cxx, typelib_typedescription_register contains a data race that might cause crashes.

In short: typelib_typedescription_register replaces members of already stored type descriptions without synchronizing with other threads that might currently use those members.

Crash scenario is as follows:
* thread A is processing an IPC call involving an out parameter with struct type (the type description for this struct's bComplete member is 0 up to now)
** thread A is copying the C++ out parameter to the BinaryAny objects for transferring it via IPC, and calls _copyConstructStruct (copy.hxx)
** _copyConstructStruct copies member per member, using the type description's pMemberOffsets and ppTypeRefs member attributes

* at the same time thread B also needs this struct type for its work, and tries to complete the struct type description (complete(...) in typelib.cxx).
** after completing the type description, the completed description is registered via typelib_typedescription_register
** typelib_typedescription_register notices that the new description is complete, but the previously stored was not -> it calls typelib_typedescription_destructExtendedMembers(...) on the old, stored typedescription
** typelib_typedescription_destructExtendedMembers(...) deletes the memory used for the pMemberOffsets and ppTypeRefs members (while they are in use by thread A)
** typelib_typedescription_register replaces the members in the stored struct with those from the new struct

Depending on the timing, thread A accesses already freed memory (no crash), or sees already different data at the pMemberOffset/ppTypeRefs pointers. The latter case happened for me, pMemberOffsets[0] was 0 and pMemberOffsets[1] was 0, ie. have already been reused for something else.
My struct contained a long and a type member, and the code crashed since it wanted to copy the type member at offset 0, which contained a 1 -> access to memory 0x1 instead of just copying the 1 as long value.


The issue seems to be present in libreoffice 6.0.0.3 (at least from looking at the typelib code), I found the issue in libreoffice 4.1.4.2
Comment 1 Buovjaga 2018-02-24 15:43:39 UTC
Would be cool, if you submitted a patch to solve it: https://wiki.documentfoundation.org/Development/gerrit
Comment 2 Xisco Faulí 2018-03-27 09:12:38 UTC
(In reply to Buovjaga from comment #1)
> Would be cool, if you submitted a patch to solve it:
> https://wiki.documentfoundation.org/Development/gerrit

Dear Reporter,
Would you like to submit a patch using the link above ?
Comment 3 straub 2018-03-27 11:17:42 UTC
A workaround could be to not delete the extended data members, but this would create a memory leak. I have no idea how to fix the issue.
Comment 4 Xisco Faulí 2018-03-27 11:35:19 UTC
@Stephan, would you mind taking a look at the description provided ?