move genhomedircon call out of transaction

Attached is a patch that moves the genhomedircon patch out of the libsemanage transaction. This is necessary since genhomedircon now uses libsemanage, and enters a transaction itself.

Chad

----------------------
Chad Sellers
Tresys Technology, LLC
[email protected]
(410)290-1411 x117
http://www.tresys.com

Index: libsemanage/src/semanage_store.c
===================================================================
RCS file: /cvsroot/selinux/nsa/selinux-usr/libsemanage/src/semanage_store.c,v retrieving revision 1.21

diff -u -r1.21 semanage_store.c
@@ -950,11 +950,6 @@
 		goto cleanup;
 	}
 
-	if ((r = semanage_exec_prog(sh, sh->conf->genhomedircon, sh->conf->store_path, "")) != 0) {
-		ERR(sh, "genhomedircon returned error code %d.", r);
-		goto cleanup;
-	}
-
 	retval = 0;
 cleanup:
 	free(storepath);

@@ -1070,6 +1065,12 @@
 		goto cleanup;
 	}
 
+	if ((retval = semanage_exec_prog(sh, sh->conf->genhomedircon, sh->conf->store_path, "")) != 0) {
+		ERR(sh, "genhomedircon returned error code %d.", retval);
+		goto cleanup;
+	}
+
+
 cleanup:
 	return retval;
 

I don't think genhomedircon enters a transaction... nor should it - it's a read-only operation with respect to the objects that we manage (currently). I could be wrong, but I can't find where it enters a transaction in the patch.

Also, even if it did enter transaction, exiting and re-entering "immediatly" would create a race condition.

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Note: calling multiple semanage query functions outside a transaction is a bad idea, because a policy is object is created and destroyed per call (while it is cached when in-transaction). However, here only the list() function is called (once?), so that seems fine. The active lock guards against modification during list().

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Yep it does. It calls semanage_user_list(), which calls dbase_list(), which calls enter_ro(), which gets the lock.

Could you expand on this race condition? genhomedircon is only reading from libsemanage to generate file_contexts.homedir. What race condition are you worried about?

----------------------
Chad Sellers
Tresys Technology, LLC
[email protected]
(410)290-1411 x117
http://www.tresys.com

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

I see the problem now - you confused me, because that's the active lock, not the transaction lock. It is not entering a transaction - it's guarding against a concurrent commit, but you *are* doing a concurrent commit, and calling genhomedircon as part of it.

Well... any time you're releasing a lock, and you want to re-acquire it before changes are made - that's a potential race condition. I guess in this case you don't care if changes are made in the meantime, so there is no race condition.

On the other hand, genhomedir called outside libsemanage has a race in itself, because changes can be introduced between the time you call user_list, and seuser_list. It seems you're moving the place it's called to the end of install_sandbox (hard to tell without -p flag), so if it's called within libsemanage, that won't be a problem (active lock released, transaction lock still held, concurrent commit not possible).

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Really, genhomedircon should be using a transaction, which is both safer (avoids race), and faster (requires only 1 policydb_read, as opposed to 2). However, I don't know how you'd call it within the original transaction...

--
This message was distributed to subscribers of the selinux mailing list. If you no longer wish to subscribe, send mail to [email protected] with the words "unsubscribe selinux" without quotes as the message.

Thanks, merged as of libsemanage 1.3.54.

Stephen Smalley
National Security Agency


--
This message was distributed to subscribers of the selinux mailing list.
If you no longer wish to subscribe, send mail to [email protected] with
the words "unsubscribe selinux" without quotes as the message.