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 757661 - Use of uninitialised pointer could lead to segfault in output_get_backlight_limits_xrandr
Use of uninitialised pointer could lead to segfault in output_get_backlight_l...
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: general
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
: 758050 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2015-11-06 03:34 UTC by Ikey Doherty
Modified: 2017-05-31 23:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
backends/x11: Ensure reply is initialised to NULL (939 bytes, patch)
2015-11-06 03:35 UTC, Ikey Doherty
committed Details | Review

Description Ikey Doherty 2015-11-06 03:34:38 UTC
output_get_backlight_limits_xrandr in src/backends/x11/meta-monitor-manager-xrandr.c has a g_autofree xcb_randr_query_output_property_reply_t that isn't initialised to NULL.

As much as it's good practice to initialise pointers to NULL anyway, this is especially true of __attribute__(cleanup())) helpers.

Exists in 3.18 and git master
Comment 1 Ikey Doherty 2015-11-06 03:35:40 UTC
Created attachment 314952 [details] [review]
backends/x11: Ensure reply is initialised to NULL

Added patch to fix this on 3.18.x and git master
Comment 2 Rui Matos 2015-11-11 14:49:50 UTC
Review of attachment 314952 [details] [review]:

We don't return without the variable being assigned but sure, this doesn't hurt
Comment 3 Ikey Doherty 2015-11-11 14:57:42 UTC
It was mostly to kill the annoying compiler warning :) I noticed some distros just passing --enable-compile-warnings=minimum - which felt a bit dirty to do in my own distro :)
Comment 4 Owen Taylor 2015-11-11 15:03:44 UTC
If your compiler is warning about this, I think it's buggy. What compiler, what options?
Comment 5 Rui Matos 2015-11-20 15:56:13 UTC
*** Bug 758050 has been marked as a duplicate of this bug. ***
Comment 6 Ikey Doherty 2015-11-20 15:57:44 UTC
GCC 5.2.0 and Clang 3.6.2

So they're both buggy? :)
Comment 7 Owen Taylor 2015-11-20 16:46:17 UTC
My only guess is that the compilers are defensively assuming that XInternAtom might throw a C++ exception across the code trigger cleanups. It would be interesting whether -fexceptions -fno-exceptions makes a difference for GCC.
Comment 8 Ikey Doherty 2015-11-20 17:33:20 UTC
Current CFLAGS:
-mtune=generic -march=x86-64 -g2 -O2 -pipe -fPIC -Wformat -Wformat-security -fomit-frame-pointer -fstack-protector-strong --param ssp-buffer-size=4 -fexceptions -D_FORTIFY_SOURCE=2 -feliminate-unused-debug-types -Wno-error

And the ldflags:

-Wl,--copy-dt-needed-entries -Wl,-O1 -Wl,-z,relro -Wl,-z,now

I'll try -fno-exceptions later and see if that helps, however the point stands that autofree-style variables should still be initialised to NULL. Good practice n all that.
Comment 9 Ikey Doherty 2016-07-27 23:21:11 UTC
Still happening with Clang 3.8.1, GCC 6.1.0, with Mutter 3.20.3

For reference, the patch still applies, and it's good practice to just ensure that you null-assign pointers.
Comment 10 Jeremy Bicha 2017-05-31 23:18:14 UTC
Ikey does not have git commit rights yet, so I'm pushing for him.

I also am pushing this to the gnome-3-24 and gnome-3-22 branches
Attachment 314952 [details] pushed as 8699aca - backends/x11: Ensure reply is initialised to NULL