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 724789 - g-s-d hardening for BadDevice errors
g-s-d hardening for BadDevice errors
Status: RESOLVED FIXED
Product: gnome-settings-daemon
Classification: Core
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gnome-settings-daemon-maint
gnome-settings-daemon-maint
Depends on:
Blocks:
 
 
Reported: 2014-02-20 10:07 UTC by Peter Hutterer
Modified: 2014-03-04 16:19 UTC
See Also:
GNOME target: ---
GNOME version: ---

Attachments
0001-mouse-wrap-device-button-mapping-into-gdk_error_trap.patch (1.38 KB, patch)
2014-02-20 10:07 UTC, Peter Hutterer 		committed Details | Review
0002-mouse-wrap-pointer-acceleration-changes-into-a-gdk_e.patch (1.25 KB, patch)
2014-02-20 10:08 UTC, Peter Hutterer 		accepted-commit_now Details | Review
0003-common-provide-a-helper-function-to-close-an-XDevice.patch (9.97 KB, patch)
2014-02-20 10:09 UTC, Peter Hutterer 		accepted-commit_now Details | Review
0002-mouse-wrap-pointer-acceleration-changes-into-a-gdk_e.patch (1.24 KB, patch)
2014-02-21 00:14 UTC, Peter Hutterer 		committed Details | Review
0003-common-provide-a-helper-function-to-close-an-XDevice.patch (9.82 KB, patch)
2014-02-21 00:15 UTC, Peter Hutterer 		committed Details | Review
GNOME3.10: 0001-mouse-wrap-device-button-mapping-into-gdk_error_trap.patch (1.38 KB, patch)
2014-02-21 00:32 UTC, Peter Hutterer 		committed Details | Review
GNOME3.10: 0002-mouse-wrap-pointer-acceleration-changes-into-a-gdk_e.patch (1.24 KB, patch)
2014-02-21 00:33 UTC, Peter Hutterer 		committed Details | Review
GNOME3.10: 0003-common-provide-a-helper-function-to-close-an-XDevice.patch (18.01 KB, patch)
2014-02-21 00:33 UTC, Peter Hutterer 		committed Details | Review

Description Peter Hutterer 2014-02-20 10:07:50 UTC 
Created attachment 269785 [details] [review]
0001-mouse-wrap-device-button-mapping-into-gdk_error_trap.patch

g-s-d struggles to handle devices that appear and immediately disappear (libinput test suite). The expectation must be that any XI call can fail with BadDevice. Most calls are handled correctly already, but XCloseDevice can fail too with a BadDevice error.
Comment 1 Peter Hutterer 2014-02-20 10:08:42 UTC 
Created attachment 269786 [details] [review]
0002-mouse-wrap-pointer-acceleration-changes-into-a-gdk_e.patch
Comment 2 Peter Hutterer 2014-02-20 10:09:20 UTC 
Created attachment 269787 [details] [review]
0003-common-provide-a-helper-function-to-close-an-XDevice.patch

I need to go through the other plugins to audit them for similar issues, but these fix all the crashes I've been seeing so far.
Comment 3 Bastien Nocera 2014-02-20 10:18:04 UTC 
Review of attachment 269785 [details] [review]:

Looks good.
Comment 4 Bastien Nocera 2014-02-20 10:19:38 UTC 
Review of attachment 269786 [details] [review]:

Looks good otherwise

::: plugins/mouse/gsd-mouse-manager.c
@@ +462,3 @@
 
+        if (gdk_error_trap_pop ())
+                g_warning ("Error in setting acceleration on on \"%s\"", gdk_device_get_name (device));

"on on"?
Comment 5 Bastien Nocera 2014-02-20 10:21:39 UTC 
Review of attachment 269787 [details] [review]:

Looks good otherwise.

::: plugins/common/gsd-input-helper.h
@@ +81,3 @@
 char *    xdevice_get_device_node  (int                     deviceid);
 int       xdevice_get_last_tool_id (int                     deviceid);
+void      device_close_safely      (XDevice                *xdevice);

xdevice_close() is a nicer name. I don't think we need the "safely"
Comment 6 Peter Hutterer 2014-02-21 00:14:34 UTC 
Created attachment 269858 [details] [review]
0002-mouse-wrap-pointer-acceleration-changes-into-a-gdk_e.patch

fixed error message
Comment 7 Peter Hutterer 2014-02-21 00:15:27 UTC 
Created attachment 269859 [details] [review]
0003-common-provide-a-helper-function-to-close-an-XDevice.patch

Function renamed to xdevice_close

fwiw, all three patches are for master
Comment 8 Peter Hutterer 2014-02-21 00:32:31 UTC 
Created attachment 269860 [details] [review]
GNOME3.10: 0001-mouse-wrap-device-button-mapping-into-gdk_error_trap.patch

Patch for 3.10.2 (Fedora 20), that's the one I tested :)
Comment 9 Peter Hutterer 2014-02-21 00:33:15 UTC 
Created attachment 269861 [details] [review]
GNOME3.10: 0002-mouse-wrap-pointer-acceleration-changes-into-a-gdk_e.patch

Patch for 3.10.2 (Fedora 20)
Comment 10 Peter Hutterer 2014-02-21 00:33:49 UTC 
Created attachment 269862 [details] [review]
GNOME3.10: 0003-common-provide-a-helper-function-to-close-an-XDevice.patch

Patch for 3.10.2 (Fedora 20)
Comment 11 Bastien Nocera 2014-02-21 08:25:03 UTC 
Review of attachment 269858 [details] [review]:

++
Comment 12 Bastien Nocera 2014-02-21 08:29:16 UTC 
Review of attachment 269859 [details] [review]:

Looks good.
Comment 13 Bastien Nocera 2014-02-21 08:29:45 UTC 
Review of attachment 269860 [details] [review]:

Looks good.
Comment 14 Bastien Nocera 2014-02-21 08:30:08 UTC 
Review of attachment 269861 [details] [review]:

++
Comment 15 Bastien Nocera 2014-02-21 08:30:44 UTC 
Review of attachment 269862 [details] [review]:

++
Comment 16 Rui Matos 2014-03-04 16:19:32 UTC 
Pushed all of these to the respective branches.