After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 742569 - gnome-rr-config: Make sure to copy over vendor/product/serial
gnome-rr-config: Make sure to copy over vendor/product/serial
Status: RESOLVED FIXED
Product: gnome-desktop
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: Desktop Maintainers
Desktop Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-01-08 05:25 UTC by Jasper St. Pierre (not reading bugmail)
Modified: 2015-01-08 23:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gnome-rr-config: Make sure to copy over vendor/product/serial (1.87 KB, patch)
2015-01-08 05:25 UTC, Jasper St. Pierre (not reading bugmail)
committed Details | Review

Description Jasper St. Pierre (not reading bugmail) 2015-01-08 05:25:23 UTC
See patch. This caused crashes for us in the Display panel of
gnome-control-center on ARM.
Comment 1 Jasper St. Pierre (not reading bugmail) 2015-01-08 05:25:25 UTC
Created attachment 294073 [details] [review]
gnome-rr-config: Make sure to copy over vendor/product/serial

When GnomeRROutputInfo is normally created, we strdup the
vendor/product/serial strings that we read from EDID. When it's
finalized, we free them as well.

When we copy the output info, we actually copy the raw struct, and then
selectively strdup members that we care about. When the copy is freed,
we then free the pointer that we dup'd from. When the original output
info is freed, this leads to a double-free. Due to coincidences, on x86,
this doesn't lead to a crash, but on ARM, the heap is completely
corrupted.

To prevent this, when we copy the output infos, strdup them from the
original output as well. We really should clean this API up so that it's
not so awfully RandR-y in style, since it's obvious that the APIs we
have aren't great and really aren't what we need.
Comment 2 Bastien Nocera 2015-01-08 07:48:09 UTC
Review of attachment 294073 [details] [review]:

Looks good. Could you simplify that code in a second patch, to remove the "if (old->priv->vendor)" constructs? g_strdup() can handle NULL just fine.
Comment 3 Jasper St. Pierre (not reading bugmail) 2015-01-08 23:04:09 UTC
Attachment 294073 [details] pushed as 7fdf921 - gnome-rr-config: Make sure to copy over vendor/product/serial