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 777077 - Use of memory after it is freed
Use of memory after it is freed
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-01-10 08:50 UTC by Leslie Zhai
Modified: 2017-01-12 09:28 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gregex: Fix a potential use-after-free bug (1.20 KB, patch)
2017-01-11 17:16 UTC, Philip Withnall
committed Details | Review
gregex: Fix an assignment-after-free error (808 bytes, patch)
2017-01-11 17:17 UTC, Philip Withnall
committed Details | Review

Description Leslie Zhai 2017-01-10 08:50:00 UTC
Hi GLib developers,

Bug reported by the clang static analyzer https://pbs.twimg.com/media/C1zAJUbVIAAgF52.jpg

Description: Use of memory after it is freed
File: /data/project/gnome/glib/gio/gresource.c
Line: 1317

And the same story as:

Description: Use of memory after it is freed
File: /data/project/gnome/glib/glib/gregex.c
Line: 2295


Description: Use of memory after it is freed
File: /data/project/gnome/glib/glib/gregex.c
Line: 1993
Comment 1 Philip Withnall 2017-01-10 09:41:37 UTC
What version of Clang and GLib are you using? With Clang 3.8 on GLib master (6dfc6fee7bd6911ce8facb22d78c63439f60571f), I can’t reproduce any of those three reports.

The first report (in the image) is a false positive, as two references are held to the `GResource` instance: one by the `GStaticResource` container (freed by the `g_object_unref()` call), and one by the `registered_resources` list (freed by the `g_resources_unregister_unlocked()` call).
Comment 2 Philip Withnall 2017-01-10 09:43:32 UTC
Is this with your modified version of the `MallocChecker`?
Comment 3 Leslie Zhai 2017-01-11 02:10:15 UTC
Hi Philip,

Thanks for your reply!

> Is this with your modified version of the `MallocChecker`?

Yes ;-) we talked about 'MallocChecker for Glib API' https://phabricator.freedesktop.org/T7648

And I implemented it https://reviews.llvm.org/D28348

> What version of Clang and GLib are you using?

Clang svn trunk r291635
GLib git master 6dfc6fee7bd6911ce8facb22d78c63439f60571f

Regards,
Leslie Zhai
Comment 4 Philip Withnall 2017-01-11 17:16:56 UTC
Created attachment 343317 [details] [review]
gregex: Fix a potential use-after-free bug

If the match_info out argument is NULL, info will be freed, but then its
matches member will be accessed.

Spotted by Leslie Zhai <xiangzhai83@gmail.com>.
Comment 5 Philip Withnall 2017-01-11 17:17:01 UTC
Created attachment 343318 [details] [review]
gregex: Fix an assignment-after-free error

The match_info is freed just above this line, so this would result in a
write to freed memory.

Spotted by Leslie Zhai <xiangzhai83@gmail.com>.
Comment 6 Colin Walters 2017-01-11 18:00:59 UTC
Review of attachment 343317 [details] [review]:

LGTM.
Comment 7 Colin Walters 2017-01-11 18:04:37 UTC
Review of attachment 343318 [details] [review]:

I wondered why that assignment is there, but a quick annotate chase shows it existing in the original https://git.gnome.org/browse/glib/commit/?id=0196d639 - so yes, we've always been at war with Eurasia and had use-after-frees.

LGTM.
Comment 8 Leslie Zhai 2017-01-12 03:49:34 UTC
Hi Philip,

Thanks for your patch!

I hope Clang UPSTREAM would accept my patch https://reviews.llvm.org/D28348
Then we can make GNOME/GLib better - BUGFREE!

Regards,
Leslie Zhai
Comment 9 Philip Withnall 2017-01-12 09:05:11 UTC
Attachment 343317 [details] pushed as 88e9772 - gregex: Fix a potential use-after-free bug
Attachment 343318 [details] pushed as 2c35acf - gregex: Fix an assignment-after-free error
Comment 10 Philip Withnall 2017-01-12 09:05:39 UTC
(In reply to Colin Walters from comment #7)
> so yes, we've always been at war with Eurasia and had use-after-frees.

The status quo is always comforting.

Thanks for the fast reviews.
Comment 11 Leslie Zhai 2017-01-12 09:28:47 UTC
Hi Philip,

> The first report (in the image) is a false positive

Then I need to fix MallocChecker or add some checkers in tartan ;-)

Regards,
Leslie Zhai