core/bpf: tighten handling of return values, improve messages

The code was written unidiomatically, using r as a boolean value, and
confusing errno and r in some places. AFAICS, there wasn't any actual
problem: even in the one place where errno was used instead of r, it would
almost certainly be initialized.

It seems that some libbpf functions set errno, while others return the
error, possibly encoded. Since there are almost no docs, the only way to
know is to read the code of the function. To make matters worse, there is
a global libbpf_mode which can be set to change the convention. With
LIBBPF_STRICT_DIRECT_ERRS in libbpf_mode, some functions set errno while others
return a negative error, and the only way to know is to read the code, except
that the split is now different. We currently don't set
LIBBPF_STRICT_DIRECT_ERRS, but even the possibility makes everything harder
to grok.

This is all very error-prone. Let's at least add some asserts to make sure that
the returned values are as expected.

diff --git a/src/core/bpf-lsm.c b/src/core/bpf-lsm.c
index 8689efb14124e74eb632663159649c8958e87b1f..79d17b0751e38c7e70d35895dc1a2c9b9293bcc2 100644 (file)
--- a/src/core/bpf-lsm.c
+++ b/src/core/bpf-lsm.c
@@ -64,10 +64,10 @@ static int prepare_restrict_fs_bpf(struct restrict_fs_bpf **ret_obj) {
 
         /* TODO Maybe choose a number based on runtime information? */
         r = sym_bpf_map__resize(obj->maps.cgroup_hash, CGROUP_HASH_SIZE_MAX);
-        if (r != 0)
-                return log_error_errno(r,
-                                "Failed to resize BPF map '%s': %m",
-                                sym_bpf_map__name(obj->maps.cgroup_hash));
+        assert(r <= 0);
+        if (r < 0)
+                return log_error_errno(r, "Failed to resize BPF map '%s': %m",
+                                       sym_bpf_map__name(obj->maps.cgroup_hash));
 
         /* Dummy map to satisfy the verifier */
         inner_map_fd = sym_bpf_create_map(BPF_MAP_TYPE_HASH, sizeof(uint32_t), sizeof(uint32_t), 128, 0);
@@ -75,11 +75,13 @@ static int prepare_restrict_fs_bpf(struct restrict_fs_bpf **ret_obj) {
                 return log_error_errno(errno, "Failed to create BPF map: %m");
 
         r = sym_bpf_map__set_inner_map_fd(obj->maps.cgroup_hash, inner_map_fd);
+        assert(r <= 0);
         if (r < 0)
                 return log_error_errno(r, "Failed to set inner map fd: %m");
 
         r = restrict_fs_bpf__load(obj);
-        if (r)
+        assert(r <= 0);
+        if (r < 0)
                 return log_error_errno(r, "Failed to load BPF object");
 
         *ret_obj = TAKE_PTR(obj);
@@ -99,9 +101,8 @@ static int mac_bpf_use(void) {
 
         r = read_one_line_file("/sys/kernel/security/lsm", &lsm_list);
         if (r < 0) {
-               if (errno != ENOENT)
-                       log_debug_errno(r, "Failed to read /sys/kernel/security/lsm, ignoring: %m");
-
+               if (r != -ENOENT)
+                       log_notice_errno(r, "Failed to read /sys/kernel/security/lsm, assuming bpf is unavailable: %m");
                return 0;
         }
 
@@ -110,21 +111,17 @@ static int mac_bpf_use(void) {
 
                 r = extract_first_word(&p, &word, ",", 0);
                 if (r == 0)
-                        break;
+                        return 0;
                 if (r == -ENOMEM)
                         return log_oom();
                 if (r < 0) {
-                        log_debug_errno(r, "Failed to parse /sys/kernel/security/lsm, ignoring: %m");
+                        log_notice_errno(r, "Failed to parse /sys/kernel/security/lsm, assuming bpf is unavailable: %m");
                         return 0;
                 }
 
-                if (streq(word, "bpf")) {
-                        cached_use = 1;
-                        break;
-                }
+                if (streq(word, "bpf"))
+                        return cached_use = 1;
         }
-
-        return cached_use;
 }
 
 int lsm_bpf_supported(void) {