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 785320 - heap-user-after-free when show-all-sources is enabled
heap-user-after-free when show-all-sources is enabled
Product: gnome-desktop
Classification: Core
Component: libgnome-desktop
git master
Other Linux
: Normal normal
: ---
Assigned To: Desktop Maintainers
Desktop Maintainers
Depends on:
Reported: 2017-07-23 19:07 UTC by Jason Crain
Modified: 2018-06-01 15:07 UTC
See Also:
GNOME target: ---
GNOME version: ---

Fix-heap-use-after-free-with-duplicate-xkb-layouts.patch (2.88 KB, patch)
2017-07-23 19:30 UTC, Jason Crain
none Details | Review
Fix-heap-use-after-free-with-duplicate-xkb-layouts.patch (1.36 KB, patch)
2017-07-25 04:07 UTC, Jason Crain
committed Details | Review

Description Jason Crain 2017-07-23 19:07:00 UTC
Forwarding from


I enabled all input sources in the tweak tool. Now I can't add input
sources. g-c-c crashes with the following backtrace below.

Inspecting with gdb suggests that the GList returned by 
gnome_xkb_info_get_layouts_for_language is invalid, as the data pointed
by one of the entries is not readable:

(gdb) p (char*)list->data
$2 = 0x938 <error: Cannot access memory at address 0x938>

  • #0 __strcmp_sse2_unaligned
    at ../sysdeps/x86_64/multiarch/strcmp-sse2-unaligned.S line 31
  • #1 add_rows_to_table
    at cc-input-chooser.c line 730
  • #2 get_locale_infos
    at cc-input-chooser.c line 972
  • #3 cc_input_chooser_new
    at cc-input-chooser.c line 1088
  • #4 show_input_chooser
    at cc-region-panel.c line 1146
  • #5 _g_closure_invoke_va
    at ././gobject/gclosure.c line 867
  • #6 g_signal_emit_valist
    at ././gobject/gsignal.c line 3300
  • #7 g_signal_emit
    at ././gobject/gsignal.c line 3447
  • #8 0x00007fab6c0f3e7d in
  • #9 0x00007fab6c0f3ee5 in
  • #13 <emit signal ??? on instance 0xb965eb0dc0 [GtkButton]>
    at ././gobject/gsignal.c line 3447
  • #14 0x00007fab6c0f22e0 in
  • #15 ffi_call_unix64
  • #16 ffi_call
  • #17 g_cclosure_marshal_generic_va
    at ././gobject/gclosure.c line 1604
  • #18 _g_closure_invoke_va


I've confirmed this with version 3.22.2-1 of libgnome-desktop-3-12 on Debian and with gnome-desktop's git master.  Though master doesn't crash, running with ASAN still reports a heap-use-after-free error:

==5423==ERROR: AddressSanitizer: heap-use-after-free on address 0x60200032d570 at pc 0x7efbf6e85bb0 bp 0x7ffbffff94b0 sp 0x7ffbffff8c60                                                                                            
READ of size 1 at 0x60200032d570 thread T0                                                                                                                                                                                         
    #0 0x7efbf6e85baf  (/usr/lib/x86_64-linux-gnu/                                                                                                                                                            
    #1 0x7efbef858d58 in g_str_equal /home/jason/devel/checkout/glib/glib/ghash.c:1848                                                                                                                                             
    #2 0x7efbef857139 in g_hash_table_lookup_node /home/jason/devel/checkout/glib/glib/ghash.c:396                                                                                                                                 
    #3 0x7efbef8583e5 in g_hash_table_contains /home/jason/devel/checkout/glib/glib/ghash.c:1327                                                                                                                                   
    #4 0x7efbf3942eef in add_layout_to_table /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:301                                                                                                        
    #5 0x7efbf3942fba in add_layout_to_locale_tables /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:331                                                                                                
    #6 0x7efbf3943313 in parse_end_element /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:411                                                                                                          
    #7 0x7efbef870ade in emit_end_element /home/jason/devel/checkout/glib/glib/gmarkup.c:1076                                                                                                                                      
    #8 0x7efbef871dca in g_markup_parse_context_parse /home/jason/devel/checkout/glib/glib/gmarkup.c:1618                                                                                                                          
    #9 0x7efbf394376e in parse_rules_file /home/jason/devel/checkout/gnome-desktop/libgnome-desktop/gnome-xkb-info.c:535                                                                                                           

The problem is in the parse_end_element function of gnome-xkb-info.c when it encounters a duplicate layout.  It will add the first item to a hash table.  Later when processing the duplicate it will free the memory from the first item while it still exists in the hash table.
Comment 1 Jason Crain 2017-07-23 19:30:34 UTC
Created attachment 356241 [details] [review]

Attached patch fixes this crash.  It checks if the layout is already in the table before adding it.
Comment 2 Rui Matos 2017-07-24 16:36:05 UTC
Review of attachment 356241 [details] [review]:

::: libgnome-desktop/gnome-xkb-info.c
@@ +384,3 @@
+      if (g_hash_table_contains (priv->layouts_table, priv->current_parser_layout->id)) {

This is already there, see lines 380-383 just above

@@ +411,3 @@
+      if (g_hash_table_contains (priv->layouts_table, priv->current_parser_variant->id)) {
+	  free_layout (priv->current_parser_variant);

This is the one that's missing, I didn't consider this possibility when I wrote

Can you please change this patch to just add a similar early return here as the one I pointed to above?
Comment 3 Jason Crain 2017-07-25 04:07:01 UTC
Created attachment 356333 [details] [review]

(In reply to Rui Matos from comment #2)
> This is already there, see lines 380-383 just above

Sorry I hadn't noticed this.  I've copied and adapted this for the variant section.
Comment 4 Rui Matos 2017-07-25 14:19:49 UTC
Review of attachment 356333 [details] [review]:

looks good, thanks
Comment 5 Rui Matos 2017-07-25 14:21:27 UTC
cf1b213..6c6cc27  gnome-3-24 -> gnome-3-24
   a3c7ec8..dda6759  master -> master