Mir

libmirprotobuf's ABI can be broken when modifying protobuf message definitions

Bug #1465883 reported by Alberto Aguirre
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Mir
Fix Released
High
Alberto Aguirre
0.13
Fix Released
High
Alberto Aguirre
mir (Ubuntu)
Fix Released
High
Unassigned

Bug Description

libmirprotobuf's ABI can be broken when modifying protobuf message definitions.

This is normally not an issue because updates keep replacing libmirclient.so.8 unless we bump client ABI.

Existing clients linked to libmirclient8 (like glmark2) will use the old message layouts, however when linked to newer versions of libmirprotobuf.so.0 at runtime those messages can be bigger or potentially different. Allocating these messages on the stack in libmirclient means this causes stack corruption and crashes.

Ideally, we should bump libmirprotobuf ABI but that's not currently possible since:
- protobuf design does not allow symbols to be registered twice
- libmirprotobuf does not version its symbols
- consequently a libmirprotobuf.so.0 and libmirprotobuf.so.1 cannot be loaded in the same process at the same time
- Loading two libmirprotobuf library versions at the same time would be common since the mesa client platform module is linked against libmirclient - just probing the module would cause a new version of libmirprotobuf to load

The mir team most commonly just adds additional fields to a protobuf message. To improve compatibility in this case, we can avoid allocating protobuf messages on the stack and instead use the factory methods provided by the generated message classes which are defined in libmirprotobuf itself.

We still need to tread carefully when modifying protobuf message definitions however, to avoid changing class layouts in a non-compatible way (like removing fields or adding fields in the midle of existing ones)

Related branches

Changed in mir:
milestone: none → 0.14.0
summary: - avoid allocating mir protobuf message objects on the stack
+ MIRPROTOBUF_ABI is being broken by message design changes, but
+ MIRPROTOBUF_ABI is never bumped resulting in client crashes.
summary: - MIRPROTOBUF_ABI is being broken by message design changes, but
- MIRPROTOBUF_ABI is never bumped resulting in client crashes.
+ libmirprotobuf's ABI has been broken by message design changes, but
+ MIRPROTOBUF_ABI was never bumped, resulting in client crashes.
description: updated
Changed in mir:
importance: Medium → High
description: updated
Revision history for this message
Daniel van Vugt (vanvugt) wrote : Re: libmirprotobuf's ABI has been broken by message design changes, but MIRPROTOBUF_ABI was never bumped, resulting in client crashes.

I think bug 1341490 might cover this actually.

description: updated
summary: - libmirprotobuf's ABI has been broken by message design changes, but
- MIRPROTOBUF_ABI was never bumped, resulting in client crashes.
+ libmirprotobuf's ABI has been broken by message design changes,
+ resulting in client crashes and/or leaks
summary: - libmirprotobuf's ABI has been broken by message design changes,
- resulting in client crashes and/or leaks
+ avoid allocating mir protobuf message objects on the stack
summary: - avoid allocating mir protobuf message objects on the stack
+ Avoid allocating mir protobuf message objects on the stack
description: updated
description: updated
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Re: Avoid allocating mir protobuf message objects on the stack

Fix committed into lp:mir at revision None, scheduled for release in mir, milestone 0.14.0

Changed in mir:
status: In Progress → Fix Committed
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Remember bugs should try to describe the problem and ideally not describe a solution in their titles.

summary: - Avoid allocating mir protobuf message objects on the stack
+ libmirprotobuf's ABI can be broken when modifying protobuf message
+ definitions
Revision history for this message
Daniel van Vugt (vanvugt) wrote :

mir (0.13.3+15.10.20150617-0ubuntu1) wily; urgency=medium

  [ Alberto Aguirre ]
  * New upstream release 0.13.3 (https://launchpad.net/mir/+milestone/0.13.3)
    - Bug fixes:
      . mir-client-platform-mesa-dev package dep is incorrect (LP: #1465642)
      . Avoid allocating mir protobuf message objects on stack (LP: #1465883)

 -- CI Train Bot <email address hidden> Wed, 17 Jun 2015 20:02:15 +0000

Changed in mir (Ubuntu):
status: New → Fix Released
importance: Undecided → High
Changed in mir:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.