Re: [gdm-list] Help with GDM 2.20.2: gdm ignoring custom server config

[Brian Cameron <Brian Cameron Sun COM>]

Martin:

Looking at the code a bit more closely, I noticed the code was
allocating xserver_group once outside the loop, assigning it
to srv_entry->group, then it gets freed in the gdm_config_entry_free().
Then next time around the loop, it is already freed!

I think the attached patch (which applies on top of the previous patch)
should fix the problem.  Now the code makes sure to call stdup each
time it sets srv_entry->group.  Can you verify this avoids the crash?
If so, then I'll commit this upstream.

Thanks,

Brian


> > > However, the patch gdm dies with a seg fault in parsing the config in
> > > gdm-daemon-config.c at line 1211 in load_xservers_group().  I compiled
> > > with debug to see if there was something obvious to me.  Does this
> > > help?  I don't think this is on my end (e.g. in the patching).  The
> > > loop looks like:
> > I don't see any crashing problems here, but sometimes crashing problems
> > can be OS-speicifc.
> >
> > You don't specify which line in the loop is line 1211.  In my copy of
> > the file it is the gdm_config_entry_free line, but I suspect it is a
> > different line in your file (since that would be an odd line for it
> > to crash on just by looking at the code).
>
> Yes it really is in gdm_config_entry_free().  I'm attaching three
> files:
> 	script.out	a "script" capture of the gdb session
> 	/etc/gdm.conf
> 	/usr/share/gdm/defaults.conf
>
> The latter two from the results of:
>
> gdmflexiserver --command=GET_CONFIG_FILE
> gdmflexiserver --command=GET_CUSTOM_CONFIG_FILE
>
> from the unpatched version.
>
> > Also, could you try running gdm in the debugger and see if it gives any
> > further info about what is causing the crash?  Since the crash is
> > happening early it is probably a situation where you can actually run
> > GDM in a debugger and get some useful information.  This isn't always
> > the case, since GDM forks into slave daemons - so it is harder to use
> > the debugger to find issues after such forks happen.
> >
> > So, what I'd recommend doing is:
> >
> > 1) Run gdm-binary (not the gdm script) in the debugger like this:
> >    gdb /usr/sbin/gdm-binary
> >    run --nodaemon
> >
> >    The --nodaemon argument tells gdm-binary not to fork immediately
> >    which is what you want when debugging.
> >
> > 2) Then it should crash and tell you that the SEGV happened and drop
> >    you into a prompt.  Try printing out various values like run
> >    "print srv_entry" and see if the group variable is NULL or something
> >    causing a crash when we try to access srv_entry->group.
> >
> > > 			for (j = 0; j < G_N_ELEMENTS 
> > > 			(gdm_daemon_server_config_entries); j++) {
> > > 				GdmConfigEntry *srv_entry;
> > > 				if (gdm_daemon_server_config_entries[j].key 
> > > 				== NULL) {
> > > 					continue;
> > > 				}
> > > 				srv_entry = gdm_config_entry_copy 
> > > 				(&gdm_daemon_server_config_entries[j]);
> > > 				g_free (srv_entry->group);
> > > 				srv_entry->group = xserver_group;
> > > 				gdm_config_process_entry (config, srv_entry, 
> > > 				NULL);
> > > 				gdm_config_entry_free (srv_entry);
> > > 			}
> > >
> > >
> > > BTW, the [servers] section in my gdm.conf is default, that is to say,
> > > empty.
> > It would be helpful if you could run these commands:
> >
> > gdmflexiserver --command=GET_CONFIG_FILE
> > gdmflexiserver --command=GET_CUSTOM_CONFIG_FILE
> >
> > And send me these two files as attachments.  Perhaps the crash is
> > triggered by how your [servers] and [server-foo] sections are set up.
> > If you can send me your files, I can see if I can trigger the crash
> > on my system using your style of configuration.
> >
> > Brian
> >
> > ------------------------------------------------------------------------
> >
> > _______________________________________________
> > gdm-list mailing list
> > gdm-list gnome org
> > http://mail.gnome.org/mailman/listinfo/gdm-list

Index: gdm-daemon-config.c
===================================================================
--- gdm-daemon-config.c	(revision 5599)
+++ gdm-daemon-config.c	(working copy)
@@ -1184,21 +1184,18 @@
 {
 	GPtrArray  *server_groups;
 	char      **vname_array;
-	char       *xserver_value;
+	char       *xserver_group;
 	int         i, j;
 
 	server_groups = gdm_config_get_server_groups (config);
 
 	for (i=0; i < server_groups->len; i++) {
-		xserver_value = g_ptr_array_index (server_groups, i);
-		gdm_debug ("Processing server group <%s>", xserver_value);
+		xserver_group = g_ptr_array_index (server_groups, i);
+		gdm_debug ("Processing server group <%s>", xserver_group);
 
-		if (g_str_has_prefix (xserver_value, "server-")) {
-			char * xserver_group;
+		if (g_str_has_prefix (xserver_group, "server-")) {
 			char * xserver_name;
 
-			xserver_group = g_strdup (xserver_value);
-			
 			for (j = 0; j < G_N_ELEMENTS (gdm_daemon_server_config_entries); j++) {
 				GdmConfigEntry *srv_entry;
 				if (gdm_daemon_server_config_entries[j].key == NULL) {
@@ -1206,7 +1203,7 @@
 				}
 				srv_entry = gdm_config_entry_copy (&gdm_daemon_server_config_entries[j]);
 				g_free (srv_entry->group);
-				srv_entry->group = xserver_group;
+				srv_entry->group = g_strdup (xserver_group);
 				gdm_config_process_entry (config, srv_entry, NULL);
 				gdm_config_entry_free (srv_entry);
 			}