PolicyKit: protect session object during pam auth

[Kay Sievers]

Fix segfault cause of use of session object by pam process handler
while the session is already finalized.

Thanks,
Kay
-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /cvs/hal/PolicyKit/ChangeLog,v
retrieving revision 1.10
diff -u -p -r1.10 ChangeLog
--- ChangeLog	15 Mar 2006 16:11:33 -0000	1.10
+++ ChangeLog	15 Mar 2006 18:25:27 -0000
@@ -1,3 +1,16 @@
+2006-03-15  Kay Sievers  <kay.sievers at vrfy.org>
+
+	Protect session object(refcount) during pam work, to not go
+	away until until the auth process has finished.
+
+	* polkitd/polkit-manager.c: (bus_name_owner_changed),
+	(session_finalized):
+
+	* polkitd/polkit-session.c: (polkit_session_init), (data_from_pam),
+	(polkit_session_initiate_auth), (polkit_session_close),
+	(polkit_session_new), (polkit_session_initiator_disconnected):
+
+
 2006-03-15  David Zeuthen  <davidz at redhat.com>
 
 	* polkitd/polkit-manager.h: Include sys/types.h; fixed fd.o
Index: polkitd/polkit-manager.c
===================================================================
RCS file: /cvs/hal/PolicyKit/polkitd/polkit-manager.c,v
retrieving revision 1.1
diff -u -p -r1.1 polkit-manager.c
--- polkitd/polkit-manager.c	14 Mar 2006 06:14:33 -0000	1.1
+++ polkitd/polkit-manager.c	15 Mar 2006 18:25:27 -0000
@@ -169,13 +169,10 @@ bus_name_owner_changed (DBusGProxy  *bus
 		session = POLKIT_SESSION (g_hash_table_lookup (manager->priv->connection_name_to_session_object,
 							       old_service_name));
 		if (session != NULL) {
-			/* possibly revoke temporary privileges granted */
-			polkit_session_initiator_disconnected (session);
-
-			/* end the session */
-			g_object_unref (session);
-
 			g_hash_table_remove (manager->priv->connection_name_to_session_object, old_service_name);
+
+			/* possibly revoke temporary privileges granted, and cleanup session */
+			polkit_session_initiator_disconnected (session);
 		}
 	}
 
@@ -201,7 +198,7 @@ session_finalized (gpointer  data,
 		   GObject  *where_the_object_was)
 {
 	PolicyKitManager *manager = POLKIT_MANAGER (data);
-	
+
 	g_hash_table_foreach_remove (manager->priv->connection_name_to_session_object, 
 				     session_remover,
 				     where_the_object_was);
Index: polkitd/polkit-session.c
===================================================================
RCS file: /cvs/hal/PolicyKit/polkitd/polkit-session.c,v
retrieving revision 1.1
diff -u -p -r1.1 polkit-session.c
--- polkitd/polkit-session.c	14 Mar 2006 06:14:33 -0000	1.1
+++ polkitd/polkit-session.c	15 Mar 2006 18:25:28 -0000
@@ -69,6 +69,7 @@ struct PolicyKitSessionPrivate
 	gboolean have_granted_temp_privileges;
 
 	int auth_state;
+	gboolean is_closed;
 	gboolean is_authenticated;
 	char *auth_denied_reason;
 	GSList *auth_questions;
@@ -96,6 +97,7 @@ polkit_session_init (PolicyKitSession *s
 {
 	session->priv = g_new0 (PolicyKitSessionPrivate, 1);
 	session->priv->session_number = 42;
+	session->priv->is_closed = FALSE;
 	session->priv->is_authenticated = FALSE;
 	session->priv->auth_state = AUTH_STATE_NOT_STARTED;
 }
@@ -676,10 +678,8 @@ data_from_pam (GIOChannel *source,
 			/* left intentionally blank */
 			break;
 		}
-
 	}
 
-
 	if (condition & G_IO_HUP) {
 		/*g_debug ("in %s - hangup", __FUNCTION__);*/
 		if (session->priv->child_pid != 0) {
@@ -688,6 +688,10 @@ data_from_pam (GIOChannel *source,
 			session->priv->child_pid = 0;
 			waitpid (session->priv->child_pid, &status, 0);
 		}
+
+		/* we held a ref for the session as long as we watched */
+		g_object_unref (session);
+
 		/* remove the source */
 		return FALSE;
 	}
@@ -748,12 +752,12 @@ polkit_session_initiate_auth (PolicyKitS
 		g_warning ("pipe() failed: %s", strerror (errno));
 		goto fail;
 	}
-	
+
 	switch (pid = fork()) {
 	case -1:
 		g_warning ("fork() failed: %s", strerror (errno));
 		goto fail;
-		
+
 	case 0:
 		/* child; close unused ends */
 		close (fds[0]);
@@ -761,7 +765,7 @@ polkit_session_initiate_auth (PolicyKitS
 
 		do_pam_auth (fds[1], fdsb[0], session->priv);
 		break;
-		
+
 	default:
 		session->priv->auth_state = AUTH_STATE_IN_PROGRESS;
 
@@ -773,11 +777,13 @@ polkit_session_initiate_auth (PolicyKitS
 		session->priv->pam_channel_write = g_io_channel_unix_new (fdsb[1]);
 		session->priv->pam_channel = g_io_channel_unix_new (fds[0]);
 
-		g_io_add_watch (session->priv->pam_channel, 
+		/* we hold a ref for the session as long as we watch */
+		g_object_ref (session);
+
+		g_io_add_watch (session->priv->pam_channel,
 				G_IO_IN | G_IO_ERR | G_IO_HUP,
 				data_from_pam,
 				session);
-
 		break;
 	}
 
@@ -881,7 +887,6 @@ polkit_session_close (PolicyKitSession  
 		return FALSE;
 
 	if (!do_not_revoke_privilege && session->priv->have_granted_temp_privileges) {
-
 		if (!polkit_manager_remove_temporary_privilege (session->priv->manager,
 								session->priv->grant_to_uid,
 								session->priv->grant_privilege,
@@ -895,7 +900,10 @@ polkit_session_close (PolicyKitSession  
 		}
 	}
 
-	g_object_unref (session);
+	if (!session->priv->is_closed) {
+		g_object_unref (session);
+		session->priv->is_closed = TRUE;
+	}
 
 	dbus_g_method_return (context);
 	return TRUE;
@@ -959,6 +967,7 @@ polkit_session_new (DBusGConnection    *
 	static int session_number_base = 0;
 
 	session = POLKIT_SESSION (g_object_new (POLKIT_TYPE_SESSION, NULL));
+
 	session->priv->connection = dbus_g_connection_ref (connection);
 	session->priv->session_number = session_number_base++;
 	session->priv->manager = manager;
@@ -1000,4 +1009,10 @@ polkit_session_initiator_disconnected (P
 				   session->priv->grant_pid_restriction);
 		}
 	}
+
+	/* cleanup session if orphaned */
+	if (!session->priv->is_closed) {
+		g_object_unref (session);
+		session->priv->is_closed = TRUE;
+	}
 }