cgroup: check systemd unit creation

While playing with Fedora 31 host with old/broken selinux packages,
I found out that systemd fails to create a transient unit. Here is
an except from journalctl:

> audit[1]: AVC avc:  denied  { setsched } for  pid=1 comm="systemd" scontext=system_u:system_r:init_t:s0 tcontext=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 tclass=process permissive=0
> systemd[1]: crun-555.scope: Failed to add PIDs to scope's control group: Permission denied
> systemd[1]: crun-555.scope: Failed with result 'resources'.
> systemd[1]: Failed to start libcrun container.

and yet crun did not show any error and proceeded to start the
container, which lead to a number of issues.

1. Since the cgroup was not created by systemd, but the error
was not detected, the container process was not put into its own
cgroup (but left in the same cgroup as the shell from which `crun`
was called).

2. Since crun gets the cgroup name from /proc/$PID/cgroup
(where $PID is container process PID), it proceeded to set the
limits for that (wrong) cgroup:

> # cat /sys/fs/cgroup/system.slice/sshd.service/memory.max
> 536870912

3. `crun delete` apparently removes the `system.slice/sshd.service`
cgroup :(

The primary cause is the missing check that the transient unit has
been created. This is what this patch adds (similar to how it's done
in cgroup-run code).

After this patch:

> # ../crun --systemd-cgroup run -d 555
> 2020-04-16T14:47:34.000354150Z: Systemd unit crun-555.scope failed: failed. See "journalctl -xe" for details.
> 2020-04-16T14:47:34.000355064Z: Systemd unit crun-555.scope failed: failed. See "journalctl -xe" for details.

For more background on how the issue was found, steps to repro etc
please see a similar (but much less brutal -- it just fails to start
the container) issue in runc:

 - opencontainers/runc#2313

[v2: also check status in destroy_systemd_cgroup_scope()]

Fixes: eaccb4b
Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>

src/libcrun/cgroup.c

@@ -720,7 +720,9 @@ int systemd_finalize (struct libcrun_cgroup_args *args, const char *suffix, libc
 struct systemd_job_removed_s
 {
   const char *path;
-  int *terminated;
+  int terminated;
+  char *unit;
+  char *result;
 };
 
 static int
@@ -736,7 +738,70 @@ systemd_job_removed (sd_bus_message *m, void *userdata, sd_bus_error *error arg_
     return -1;
 
   if (strcmp (p->path, path) == 0)
-    *p->terminated = 1;
+    {
+      p->terminated = 1;
+      p->unit = xstrdup(unit);
+      p->result = xstrdup(result);
+    }
+  return 0;
+}
+
+static int
+systemd_check_job_status_setup (sd_bus *bus,
+                                struct systemd_job_removed_s *d,
+                                libcrun_error_t *err)
+{
+  int ret;
+
+  ret = sd_bus_match_signal (bus,
+                             NULL,
+                             "org.freedesktop.systemd1",
+                             "/org/freedesktop/systemd1",
+                             "org.freedesktop.systemd1.Manager",
+                             "JobRemoved",
+                             systemd_job_removed, d);
+  if (UNLIKELY (ret < 0))
+    return crun_make_error (err, -ret, "sd-bus match signal");
+
+  return 0;
+}
+
+static int
+systemd_check_job_status (sd_bus *bus,
+                          struct systemd_job_removed_s *d,
+                          const char *path,
+                          libcrun_error_t *err)
+{
+  int sd_err;
+
+  d->path = path;
+  while (!d->terminated)
+    {
+      sd_err = sd_bus_process (bus, NULL);
+      if (UNLIKELY (sd_err < 0))
+        return crun_make_error (err, -sd_err, "sd-bus process");
+
+      if (sd_err == 0)
+        {
+          sd_err = sd_bus_wait (bus, (uint64_t) -1);
+          if (UNLIKELY (sd_err < 0))
+            return crun_make_error (err, -sd_err, "sd-bus wait");
+
+          continue;
+        }
+    }
+
+  if (d->unit && d->result)
+    {
+      if (strcmp(d->result, "done") != 0)
+        {
+          return crun_make_error (err, 0, "Systemd unit %s creation error: %s. See \"journalctl -xe\" for details.", d->unit, d->result);
+        }
+
+      free(d->unit);
+      free(d->result);
+    }
+
   return 0;
 }
 
@@ -890,8 +955,7 @@ int enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *reso
   int sd_err, ret = 0;
   sd_bus_error error = SD_BUS_ERROR_NULL;
   const char *object;
-  int terminated = 0;
-  struct systemd_job_removed_s userdata;
+  struct systemd_job_removed_s status;
   int i;
   const char *boolean_opts[10];
 
@@ -922,19 +986,9 @@ int enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *reso
         }
     }
 
-  sd_err = sd_bus_add_match (bus,
-                          NULL,
-                          "type='signal',"
-                          "sender='org.freedesktop.systemd1',"
-                          "interface='org.freedesktop.systemd1.Manager',"
-                          "member='JobRemoved',"
-                          "path='/org/freedesktop/systemd1'",
-                          systemd_job_removed, &userdata);
-  if (UNLIKELY (sd_err < 0))
-    {
-      ret = crun_make_error (err, -sd_err, "sd-bus add match");
+  ret = systemd_check_job_status_setup(bus, &status, err);
+  if (UNLIKELY (ret < 0))
       goto exit;
-    }
 
   sd_err = sd_bus_message_new_method_call (bus, &m, "org.freedesktop.systemd1",
                                            "/org/freedesktop/systemd1",
@@ -1050,28 +1104,7 @@ int enter_systemd_cgroup_scope (runtime_spec_schema_config_linux_resources *reso
       goto exit;
     }
 
-  userdata.path = object;
-  userdata.terminated = &terminated;
-  while (!terminated)
-    {
-      sd_err = sd_bus_process (bus, NULL);
-      if (UNLIKELY (sd_err < 0))
-        {
-          ret = crun_make_error (err, -sd_err, "sd-bus process");
-          break;
-        }
-
-      if (sd_err == 0)
-        {
-          sd_err = sd_bus_wait (bus, (uint64_t) -1);
-          if (UNLIKELY (sd_err < 0))
-            {
-              ret = crun_make_error (err, -sd_err, "sd-bus wait");
-              break;
-            }
-          continue;
-        }
-    }
+  ret = systemd_check_job_status (bus, &status, object, err);
 
 exit:
   if (bus)
@@ -1094,27 +1127,17 @@ int destroy_systemd_cgroup_scope (const char *scope, libcrun_error_t *err)
   sd_bus_error error = SD_BUS_ERROR_NULL;
   const char *object;
   int terminated = 0;
-  struct systemd_job_removed_s userdata;
+  struct systemd_job_removed_s status;
 
   if (sd_bus_default (&bus) < 0)
     {
       ret = crun_make_error (err, 0, "cannot open sd-bus");
       goto exit;
     }
 
-  ret = sd_bus_add_match (bus,
-                          NULL,
-                          "type='signal',"
-                          "sender='org.freedesktop.systemd1',"
-                          "interface='org.freedesktop.systemd1.Manager',"
-                          "member='JobRemoved',"
-                          "path='/org/freedesktop/systemd1'",
-                          systemd_job_removed, &userdata);
+  ret = systemd_check_job_status_setup(bus, &status, err);
   if (UNLIKELY (ret < 0))
-    {
-      ret = crun_make_error (err, -ret, "sd-bus message read");
-      goto exit;
-    }
+    goto exit;
 
   ret = sd_bus_message_new_method_call (bus, &m,
                                         "org.freedesktop.systemd1",
@@ -1147,28 +1170,8 @@ int destroy_systemd_cgroup_scope (const char *scope, libcrun_error_t *err)
       goto exit;
     }
 
-  userdata.path = object;
-  userdata.terminated = &terminated;
-  while (!terminated)
-    {
-      ret = sd_bus_process (bus, NULL);
-      if (UNLIKELY (ret < 0))
-        {
-          ret = crun_make_error (err, -ret, "sd-bus process");
-          break;
-        }
+  ret = systemd_check_job_status (bus, &status, object, err);
 
-      if (ret == 0)
-        {
-          ret = sd_bus_wait (bus, (uint64_t) -1);
-          if (UNLIKELY (ret < 0))
-            {
-              ret = crun_make_error (err, -ret, "sd-bus wait");
-              break;
-            }
-          continue;
-        }
-    }
 exit:
   if (bus)
     sd_bus_unref (bus);