GNOME Bugzilla – Bug 607505
double free crash in get_session_command_for_file()
Last modified: 2010-01-20 21:54:03 UTC
The bug has been opene don https://bugs.launchpad.net/ubuntu/+source/gdm/+bug/505051 "This causes the login process to hang indefinitely. Cancel does nothing... Here's what gets logged in /var/log/gdm/:0-slave.log: *** glibc detected *** /usr/lib/gdm/gdm-simple-slave: double free or corruption (fasttop): 0x09a61c88 *** ======= Backtrace: ========= /lib/tls/i686/cmov/libc.so.6[0x440ff1] /lib/tls/i686/cmov/libc.so.6[0x4426f2] /lib/tls/i686/cmov/libc.so.6(cfree+0x6d)[0x44579d] /lib/libglib-2.0.so.0(g_free+0x36)[0xbd8196] /usr/lib/gdm/gdm-simple-slave[0x8056ac5] get_session_command_for_file /usr/lib/gdm/gdm-simple-slave[0x8056d23] get_fallback_session_name /usr/lib/gdm/gdm-simple-slave[0x805851d] gdm_session_direct_defaults_changed /usr/lib/gdm/gdm-simple-slave[0x8058709] gdm_session_direct_setup_for_user /usr/lib/libgobject-2.0.so.0(g_cclosure_marshal_VOID__STRING+0x88)[0xa251c8] /usr/lib/libgobject-2.0.so.0(g_closure_invoke+0x1b2)[0xa18072] /usr/lib/libgobject-2.0.so.0[0xa2d7a8] /usr/lib/libgobject-2.0.so.0(g_signal_emit_valist+0x7bd)[0xa2eb2d] /usr/lib/libgobject-2.0.so.0(g_signal_emit+0x26)[0xa2efb6] /usr/lib/gdm/gdm-simple-slave[0x804f44a] greeter_server_message_handler /lib/libdbus-1.so.3[0x3b9f13] /lib/libdbus-1.so.3(dbus_connection_dispatch+0x3cc)[0x3accec] /usr/lib/libdbus-glib-1.so.2[0x60075d] /lib/libglib-2.0.so.0(g_main_context_dispatch+0x1f8)[0xbcfe88] /lib/libglib-2.0.so.0[0xbd3730] /lib/libglib-2.0.so.0(g_main_loop_run+0x1bf)[0xbd3b9f] /usr/lib/gdm/gdm-simple-slave[0x804dbc6] /lib/tls/i686/cmov/libc.so.6(__libc_start_main+0xe6)[0x3ecb56] /usr/lib/gdm/gdm-simple-slave[0x804d7e1] I have added the decoded symbol names from the ddeb. From this you can see that get_session_command_for_file likely causes the double free. Looking at that code I noticed this: static gboolean get_session_command_for_file (const char *file, char **command) { GKeyFile *key_file; GError *error; char *full_path; char *exec; gboolean ret; gboolean res; exec = NULL; ... exec = g_key_file_get_string (key_file, G_KEY_FILE_DESKTOP_GROUP, G_KEY_FILE_DESKTOP_KEY_TRY_EXEC, NULL); if (exec != NULL) { res = is_prog_in_path (exec); g_free (exec); if (! res) { g_debug ("GdmSessionDirect: Command not found: %s", G_KEY_FILE_DESKTOP_KEY_TRY_EXEC); goto out; } } ... out: g_free (exec); return ret; } So exec gets g_free'd twice if there is a TryExec key and is_prog_in_path fails."
Created attachment 151797 [details] [review] don't free the exec variable twice
Review of attachment 151797 [details] [review]: ::: gdm-2.29.5/daemon/gdm-session-direct.c @@ +573,2 @@ g_free (exec); +out: I don't think this is right, there's a number of other exit points where the free should happen. In fact, looking at the code, it seems like the exec variable is getting abused for checking tryexec. That's probably okay, but I think we need to nullify it when we're done abusing it.
I pushed 6b28faeac82d3310c3c142187069d10b513cbf97 Does that work okay for you?
> I don't think this is right, there's a number of other exit points where the free should happen. The variable is used only twice in this function so the suggested change was doing the job, yours is another way and working fine too, thanks!