Re: [systemd-devel] [PATCH v3 2/2] selinux: fix unnecessary generic SELinux check due to unit objects in UNIT_NOT_FOUND

[systemd github import bot]

Patchset imported to github.
To create a pull request, one of the main developers has to initiate one via:


--
Generated by https://github.com/haraldh/mail2git
___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel

[systemd-devel] [PATCH v3 2/2] selinux: fix unnecessary generic SELinux check due to unit objects in UNIT_NOT_FOUND

[HATAYAMA Daisuke]

systemd creates a unit object of A.service when it is referenced in
various contexts such as that systemd parses a unit file and finds a
dependency, like After=A.service, in some unit file or via systemd is
requested a D-Bus operation such as systemctl status A.service, and
then registers it in manager->units hash table.

If the referenced unit file actually doesn't exist, load state of the
unit object becomes UNIT_NOT_FOUND and is left in manager->units until
it is cleaned up at the next manager loop.

The problem here is that mac_selinux_unit_access_check_strv()
overlooks the situation that some unit object of A.service in
UNIT_NOT_FOUND could still be left in the manager->units hash table
but a new unit file A.service has just been created. Then, the unit
object in UNIT_NOT_FOUND state hides presence of the A.service. As a
result, we don't lead to an SELinux unit access check with a SELinux
context label of the A.service.

To fix this, if manager_load_unit() returns a unit object in
UNIT_NOT_FOUND from the manager->units hash table, clean up the unit
object and retry loading unit files.
---
 src/core/manager.c|4 ++--
 src/core/manager.h|2 ++
 src/core/selinux-access.c |   14 ++
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/core/manager.c b/src/core/manager.c
index a1c5433..d05d6fb 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -771,7 +771,7 @@ static int manager_connect_bus(Manager *m, bool 
reexecuting) {
 return bus_init(m, try_bus_connect);
 }
 
-static unsigned manager_dispatch_cleanup_queue(Manager *m) {
+unsigned manager_dispatch_cleanup_queue(Manager *m) {
 Unit *u;
 unsigned n = 0;
 
@@ -847,7 +847,7 @@ good:
 u->gc_marker = gc_marker + GC_OFFSET_GOOD;
 }
 
-static unsigned manager_dispatch_gc_queue(Manager *m) {
+unsigned manager_dispatch_gc_queue(Manager *m) {
 Unit *u;
 unsigned n = 0;
 unsigned gc_marker;
diff --git a/src/core/manager.h b/src/core/manager.h
index 4ef869d..bea7a17 100644
--- a/src/core/manager.h
+++ b/src/core/manager.h
@@ -322,6 +322,8 @@ void manager_dump_jobs(Manager *s, FILE *f, const char 
*prefix);
 
 void manager_clear_jobs(Manager *m);
 
+unsigned manager_dispatch_cleanup_queue(Manager *m);
+unsigned manager_dispatch_gc_queue(Manager *m);
 unsigned manager_dispatch_load_queue(Manager *m);
 
 int manager_environment_add(Manager *m, char **minus, char **plus);
diff --git a/src/core/selinux-access.c b/src/core/selinux-access.c
index f52bc6d..5921898 100644
--- a/src/core/selinux-access.c
+++ b/src/core/selinux-access.c
@@ -292,10 +292,24 @@ int mac_selinux_unit_access_check_strv(char **units,
 int r;
 
 STRV_FOREACH(i, units) {
+retry:
 r = manager_load_unit(m, *i, NULL, error, &u);
 if (r < 0)
 return r;
 if (u) {
+/* Clean up an unit object in UNIT_NOT_FOUND
+ * load state if it's still left in m->units,
+ * and then retry to load a given unit file.
+ * Give up UNIT_ERROR case to aovid such as
+ * infinite loop due to ENOMEM.
+ */
+if (r == 1 && u->load_state == UNIT_NOT_FOUND) {
+unit_add_to_gc_queue(u);
+manager_dispatch_gc_queue(m);
+unit_add_to_cleanup_queue(u);
+manager_dispatch_cleanup_queue(m);
+goto retry;
+}
 r = mac_selinux_unit_access_check(u, message, 
permission, error);
 if (r < 0)
 return r;

___
systemd-devel mailing list
systemd-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/systemd-devel