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 758823 - NULL dereference in g_local_file_monitor_start since 2.46
NULL dereference in g_local_file_monitor_start since 2.46
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.46.x
Other All
: Normal critical
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2015-11-30 06:18 UTC by Yaakov Selkowitz
Modified: 2016-02-18 02:24 UTC
See Also:
GNOME target: 3.20
GNOME version: ---

Attachments
file monitors: reorder some code to avoid segfault (1.42 KB, patch)
2015-11-30 15:15 UTC, Allison Karlitskaya (desrt) 		accepted-commit_now Details | Review

Description Yaakov Selkowitz 2015-11-30 06:18:32 UTC 
The following code, introduced in 2.46, contains a NULL dereference:

https://git.gnome.org/browse/glib/tree/gio/glocalfilemonitor.c#n737

In line 749, local_monitor->source is asserted to be NULL, presumably to avoid duplication.  If the conditional on line 751 is met, then a reference to local_monitor->source->dirname is attempted on line 761, while local_monitor->source isn't set to a real value until line 775.  Since this sequence guarantees local_monitor->source is still NULL in between, the dereference to dirname always fails.

This causes an immediate crash on Cygwin when starting nautilus, and would presumably affect at least all non-Linux *NIX platforms (which only have the FAM backend, not inotify).
Comment 1 Allison Karlitskaya (desrt) 2015-11-30 15:15:00 UTC 
Created attachment 316523 [details] [review]
file monitors: reorder some code to avoid segfault

We must initialise '->source' before we use fields inside of it.
Comment 2 Yaakov Selkowitz 2015-11-30 20:24:06 UTC 
(In reply to Allison Ryan Lortie (desrt) from comment #1)
> Created attachment 316523 [details] [review] [review]
> file monitors: reorder some code to avoid segfault
> 
> We must initialise '->source' before we use fields inside of it.

True, but do local_monitor->was_mounted and/or local_monitor->mount_monitor need to be set prior to g_file_monitor_source_new()?
Comment 3 Matthias Clasen 2015-12-01 20:19:06 UTC 
Review of attachment 316523 [details] [review]:

looks obviously correct to me.
Comment 4 Matthias Clasen 2016-02-18 02:24:03 UTC 
This was committed a while ago