- From b177b0ef92d226a9f303aecbff0cf2e7293667b3 Mon Sep 17 00:00:00 2001

- From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

- Date: Sat, 8 Aug 2020 09:21:37 +0200

- Subject: [PATCH] Do not assert in test_add_acls_for_user()

- 

- This is failing on s390x with:

- /* test_add_acls_for_user */

- add_acls_for_user(3, 1000): Invalid argument

- Assertion 'r >= 0' failed at src/test/test-acl-util.c:46, function test_add_acls_for_user(). Aborting.

- ---

-  src/test/test-acl-util.c | 4 ----

-  1 file changed, 4 deletions(-)

- 

- diff --git a/src/test/test-acl-util.c b/src/test/test-acl-util.c

- index 9f0e594e67..a91d64ab0c 100644

- --- a/src/test/test-acl-util.c

- +++ b/src/test/test-acl-util.c

- @@ -43,24 +43,20 @@ static void test_add_acls_for_user(void) {

-  

-          r = add_acls_for_user(fd, uid);

-          log_info_errno(r, "add_acls_for_user(%d, "UID_FMT"): %m", fd, uid);

- -        assert_se(r >= 0);

-  

-          cmd = strjoina("ls -l ", fn);

-          assert_se(system(cmd) == 0);

-  

-          cmd = strjoina("getfacl -p ", fn);

- -        assert_se(system(cmd) == 0);

-  

-          /* set the acls again */

-  

-          r = add_acls_for_user(fd, uid);

- -        assert_se(r >= 0);

-  

-          cmd = strjoina("ls -l ", fn);

-          assert_se(system(cmd) == 0);

-  

-          cmd = strjoina("getfacl -p ", fn);

- -        assert_se(system(cmd) == 0);

-  

-          unlink(fn);

-  }

- From a73d30081a13eaeffce87f997726a179ec44d817 Mon Sep 17 00:00:00 2001

- From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= <zbyszek@in.waw.pl>

- Date: Fri, 31 Jul 2020 10:50:37 +0200

- Subject: [PATCH 1/2] Revert "test-path: increase timeout"

- 

- This partially reverts commit 500727c220354b81b68ed6667d9a6f0fafe3ba19.

- 

- I was confused by the error message: the test says it timed out, but that's

- because it's waiting for a failed unit to come back to life. There is no actual

- timeout.

- 

- So let's keep the minor refactoring that was done, but revert to the old short

- timeout.

- ---

-  src/test/test-path.c | 2 +-

-  1 file changed, 1 insertion(+), 1 deletion(-)

- 

- diff --git a/src/test/test-path.c b/src/test/test-path.c

- index 1075f31bc6..63b709c8da 100644

- --- a/src/test/test-path.c

- +++ b/src/test/test-path.c

- @@ -82,7 +82,7 @@ static void check_states(Manager *m, Path *path, Service *service, PathState pat

-          assert_se(m);

-          assert_se(service);

-  

- -        usec_t end = now(CLOCK_MONOTONIC) + 30 * USEC_PER_SEC;

- +        usec_t end = now(CLOCK_MONOTONIC) + 2 * USEC_PER_SEC;

-  

-          while (path->result != PATH_SUCCESS || service->result != SERVICE_SUCCESS ||

-                 path->state != path_state || service->state != service_state) {

- From a1ff72565c2f12b644a081ebbe3492f93ceb3bd5 Mon Sep 17 00:00:00 2001

- From: Chris Down <chris@chrisdown.name>

- Date: Thu, 29 Oct 2020 12:03:52 +0000

- Subject: [PATCH 1/3] bpf: pid1: Pin reference to BPF programs for

-  post-coldplug

- MIME-Version: 1.0

- Content-Type: text/plain; charset=UTF-8

- Content-Transfer-Encoding: 8bit

- 

- During `daemon-reload` and `daemon-reexec`, we detach and reattach all

- BPF programs attached to cgroups. This, however, poses a real practical

- problem for DevicePolicy (and some other settings using BPF): it

- presents a period of time where the old device filtering BPF program has

- been unloaded, but the new one has not been loaded yet.

- 

- Since the filtering is at open() time, it has become apparent that that

- there's a non-trivial period where applications inside that ostensibly

- filtered cgroup can grab any device -- and often do so -- and then

- retain access to that device even after the reload is over. Due to the

- file continuing to be available after the initial open(), this issue is

- particularly visible for DevicePolicy={strict,closed}, however it also

- applies to other BPF programs we install.

- 

- In particular, for BPF ingress/egress filtering this may have more

- concerning implications: network traffic which is supposed to be

- filtered will -- for a very brief period of time -- not be filtered or

- subject to any restrictions imposed by BPF.

- 

- These BPF programs are fundamentally attached to a cgroup lifetime, not

- our unit lifetime, so it's enough to pin these programs by taking a

- reference to affected BPF programs before reload/reexec. We can then

- serialise the program's kernel-facing FD and cgroup attachment FD for

- the new daemon, and have the daemon on the other side unpin the programs

- after it's finished with coldplug.

- 

- That means that, for example, the BPF program lifecycle during

- daemon-reload or daemon-reexec changes from this:

- 

-     manager_clear_jobs_and_units

-                  │

-           ╔══════╪═════════╤═══════╗

-           ║ prog │ no prog │ prog' ║

-           ╚══════╧═════════╪═══════╝

-                            │

-                     manager_coldplug

- 

- to this:

- 

-     manager_clear_jobs_and_units         manager_dispatch_cgroup_realize_queue

-                  │                                       │

-           ╔══════╪═══════════════╤═══════════════════════╪═══════╗

-           ║ prog │ prog (orphan) │ prog (orphan) + prog' │ prog' ║

-           ╚══════╧═══════════════╪═══════════════════════╧═══════╝

-                                  │

-                           manager_coldplug

- 

- For daemon-reexec the semantics are mostly the same, but the point at

- which the program becomes orphan is tied to the process lifecycle

- instead.

- 

- None of the BPF programs we install require exclusive access, so having

- multiple instances of them running at the same time is fine. Custom

- programs, of course, are unknown, but it's hard to imagine legitimate

- cases which should be affected, whereas the benefits of this "overlap"

- approach with reference pinning is immediately tangible.

- 

- [keszybz: use _cleanup_ for unpin, use FOREACH_POINTER]

- ---

-  src/core/bpf-firewall.c  |   9 +--

-  src/core/main.c          |   9 +++

-  src/core/manager.c       | 163 ++++++++++++++++++++++++++++++++++++++-

-  src/core/manager.h       |   6 ++

-  src/shared/bpf-program.c |  10 +++

-  src/shared/bpf-program.h |   1 +

-  6 files changed, 191 insertions(+), 7 deletions(-)

- 

- diff --git a/src/core/bpf-firewall.c b/src/core/bpf-firewall.c

- index bceb049b58..e3089ff6f4 100644

- --- a/src/core/bpf-firewall.c

- +++ b/src/core/bpf-firewall.c

- @@ -703,8 +703,7 @@ int bpf_firewall_install(Unit *u) {

-          if (r < 0)

-                  return log_unit_error_errno(u, r, "Failed to determine cgroup path: %m");

-  

- -        flags = (supported == BPF_FIREWALL_SUPPORTED_WITH_MULTI &&

- -                 (u->type == UNIT_SLICE || unit_cgroup_delegate(u))) ? BPF_F_ALLOW_MULTI : 0;

- +        flags = (supported == BPF_FIREWALL_SUPPORTED_WITH_MULTI) ? BPF_F_ALLOW_MULTI : 0;

-  

-          /* Unref the old BPF program (which will implicitly detach it) right before attaching the new program, to

-           * minimize the time window when we don't account for IP traffic. */

- @@ -712,8 +711,7 @@ int bpf_firewall_install(Unit *u) {

-          u->ip_bpf_ingress_installed = bpf_program_unref(u->ip_bpf_ingress_installed);

-  

-          if (u->ip_bpf_egress) {

- -                r = bpf_program_cgroup_attach(u->ip_bpf_egress, BPF_CGROUP_INET_EGRESS, path,

- -                                              flags | (set_isempty(u->ip_bpf_custom_egress) ? 0 : BPF_F_ALLOW_MULTI));

- +                r = bpf_program_cgroup_attach(u->ip_bpf_egress, BPF_CGROUP_INET_EGRESS, path, flags);

-                  if (r < 0)

-                          return log_unit_error_errno(u, r, "Attaching egress BPF program to cgroup %s failed: %m", path);

-  

- @@ -722,8 +720,7 @@ int bpf_firewall_install(Unit *u) {

-          }

-  

-          if (u->ip_bpf_ingress) {

- -                r = bpf_program_cgroup_attach(u->ip_bpf_ingress, BPF_CGROUP_INET_INGRESS, path,

- -                                              flags | (set_isempty(u->ip_bpf_custom_ingress) ? 0 : BPF_F_ALLOW_MULTI));

- +                r = bpf_program_cgroup_attach(u->ip_bpf_ingress, BPF_CGROUP_INET_INGRESS, path, flags);

-                  if (r < 0)

-                          return log_unit_error_errno(u, r, "Attaching ingress BPF program to cgroup %s failed: %m", path);

-  

- diff --git a/src/core/main.c b/src/core/main.c

- index 4a376976e9..9873f35f5e 100644

- --- a/src/core/main.c

- +++ b/src/core/main.c

- @@ -1144,6 +1144,14 @@ static int prepare_reexecute(

-          if (!fds)

-                  return log_oom();

-  

- +        /* We need existing BPF programs to survive reload, otherwise there will be a period where no BPF

- +         * program is active during task execution within a cgroup. This would be bad since this may have

- +         * security or reliability implications: devices we should filter won't be filtered, network activity

- +         * we should filter won't be filtered, etc. We pin all the existing devices by bumping their

- +         * refcount, and then storing them to later have it decremented. */

- +        _cleanup_(manager_unpin_all_cgroup_bpf_programsp) Manager *m_unpin =

- +                manager_pin_all_cgroup_bpf_programs(m);

- +

-          r = manager_serialize(m, f, fds, switching_root);

-          if (r < 0)

-                  return r;

- @@ -1159,6 +1167,7 @@ static int prepare_reexecute(

-          if (r < 0)

-                  return log_error_errno(r, "Failed to disable O_CLOEXEC for serialization fds: %m");

-  

- +        TAKE_PTR(m_unpin);

-          *ret_f = TAKE_PTR(f);

-          *ret_fds = TAKE_PTR(fds);

-  

- diff --git a/src/core/manager.c b/src/core/manager.c

- index 41e0d73736..1ce0e05706 100644

- --- a/src/core/manager.c

- +++ b/src/core/manager.c

- @@ -64,6 +64,7 @@

-  #include "rlimit-util.h"

-  #include "rm-rf.h"

-  #include "serialize.h"

- +#include "set.h"

-  #include "signal-util.h"

-  #include "socket-util.h"

-  #include "special.h"

- @@ -3210,6 +3211,79 @@ static void manager_serialize_gid_refs(Manager *m, FILE *f) {

-          manager_serialize_uid_refs_internal(m, f, &m->gid_refs, "destroy-ipc-gid");

-  }

-  

- +static int serialize_limbo_bpf_program(FILE *f, FDSet *fds, BPFProgram *p) {

- +        int copy;

- +        _cleanup_free_ char *ap = NULL;

- +

- +        /* We don't actually need the instructions or other data, since this is only used on the other side

- +         * for BPF limbo, which just requires the program type, cgroup path, and kernel-facing BPF file

- +         * descriptor. We don't even need to know what unit or directive it's attached to, since we're just

- +         * going to expire it after coldplug. */

- +

- +        assert(f);

- +        assert(p);

- +

- +        /* If the program isn't attached to the kernel yet, there's no reason to serialise it for limbo. Just

- +         * let it be skeletonized and then coldplug can do the work on the other side if it's still

- +         * necessary. */

- +        if (p->kernel_fd < 0 || !p->attached_path)

- +                return -ENOTCONN;

- +

- +        copy = fdset_put_dup(fds, p->kernel_fd);

- +        if (copy < 0)

- +                return log_error_errno(copy, "Failed to add file descriptor to serialization set: %m");

- +

- +        /* Otherwise, on daemon-reload, we'd remain pinned. */

- +        safe_close(p->kernel_fd);

- +

- +        ap = cescape(p->attached_path);

- +        if (!ap)

- +                return log_oom();

- +

- +        return serialize_item_format(f, "bpf-limbo", "%i %i %i \"%s\"",

- +                                     copy, p->prog_type, p->attached_type, ap);

- +}

- +

- +static void deserialize_limbo_bpf_program(Manager *m, FDSet *fds, const char *value) {

- +        _cleanup_free_ char *raw_fd = NULL, *raw_pt = NULL, *raw_at = NULL, *cgpath = NULL;

- +        int fd, r, prog_type, attached_type;

- +

- +        assert(m);

- +        assert(value);

- +

- +        r = extract_first_word(&value, &raw_fd, NULL, 0);

- +        if (r <= 0 || safe_atoi(raw_fd, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))

- +                return (void) log_error("Failed to parse bpf-limbo FD: %s", value);

- +

- +        r = extract_first_word(&value, &raw_pt, NULL, 0);

- +        if (r <= 0 || safe_atoi(raw_pt, &prog_type) < 0)

- +                return (void) log_error("Failed to parse bpf-limbo program type: %s", value);

- +

- +        r = extract_first_word(&value, &raw_at, NULL, 0);

- +        if (r <= 0 || safe_atoi(raw_at, &attached_type) < 0)

- +                return (void) log_error("Failed to parse bpf-limbo attached type: %s", value);

- +

- +        r = extract_first_word(&value, &cgpath, NULL, EXTRACT_CUNESCAPE | EXTRACT_UNQUOTE);

- +        if (r <= 0)

- +                return (void) log_error("Failed to parse attached path for BPF limbo FD %s", value);

- +

- +        _cleanup_(bpf_program_unrefp) BPFProgram *p = NULL;

- +        r = bpf_program_new(prog_type, &p);

- +        if (r < 0)

- +                return (void) log_error_errno(r, "Failed to create BPF limbo program: %m");

- +

- +        /* Just enough to free it when the time is right, this does not have enough information be used as a

- +         * real BPFProgram. */

- +        p->attached_type = attached_type;

- +        p->kernel_fd = fdset_remove(fds, fd);

- +        p->attached_path = TAKE_PTR(cgpath);

- +

- +        r = set_ensure_put(&m->bpf_limbo_progs, NULL, p);

- +        if (r < 0)

- +                return (void) log_error_errno(r, "Failed to register BPF limbo program for FD %s: %m", value);

- +        TAKE_PTR(p);

- +}

- +

-  int manager_serialize(

-                  Manager *m,

-                  FILE *f,

- @@ -3221,6 +3295,7 @@ int manager_serialize(

-          Iterator i;

-          Unit *u;

-          int r;

- +        BPFProgram *p;

-  

-          assert(m);

-          assert(f);

- @@ -3265,6 +3340,9 @@ int manager_serialize(

-                  (void) serialize_dual_timestamp(f, joined, m->timestamps + q);

-          }

-  

- +        SET_FOREACH(p, m->bpf_limbo_progs, i)

- +                (void) serialize_limbo_bpf_program(f, fds, p);

- +

-          if (!switching_root)

-                  (void) serialize_strv(f, "env", m->client_environment);

-  

- @@ -3543,7 +3621,10 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {

-                          else

-                                  m->n_failed_jobs += n;

-  

- -                } else if ((val = startswith(l, "taint-usr="))) {

- +                } else if ((val = startswith(l, "bpf-limbo=")))

- +                        deserialize_limbo_bpf_program(m, fds, val);

- +

- +                else if ((val = startswith(l, "taint-usr="))) {

-                          int b;

-  

-                          b = parse_boolean(val);

- @@ -3719,6 +3800,67 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {

-          return manager_deserialize_units(m, f, fds);

-  }

-  

- +Manager* manager_pin_all_cgroup_bpf_programs(Manager *m) {

- +        int r;

- +        Unit *u;

- +        Iterator ih, is;

- +

- +        assert(m);

- +

- +        HASHMAP_FOREACH(u, m->units, ih) {

- +                BPFProgram *p;

- +

- +                FOREACH_POINTER(p,

- +                                u->bpf_device_control_installed,

- +                                u->ip_bpf_ingress,

- +                                u->ip_bpf_ingress_installed,

- +                                u->ip_bpf_egress,

- +                                u->ip_bpf_egress_installed)

- +                        if (p) {

- +                                r = set_ensure_put(&m->bpf_limbo_progs, NULL, p);

- +                                if (r < 0) {

- +                                        log_unit_error_errno(u, r, "Cannot store BPF program for reload, ignoring: %m");

- +                                        continue;

- +                                }

- +

- +                                bpf_program_ref(p);

- +                        }

- +

- +                Set *s;

- +                FOREACH_POINTER(s,

- +                                u->ip_bpf_custom_ingress,

- +                                u->ip_bpf_custom_ingress_installed,

- +                                u->ip_bpf_custom_egress,

- +                                u->ip_bpf_custom_egress_installed)

- +                        SET_FOREACH(p, s, is) {

- +                                r = set_ensure_put(&m->bpf_limbo_progs, NULL, p);

- +                                if (r < 0) {

- +                                        log_unit_error_errno(u, r, "Cannot store BPF program for reload, ignoring: %m");

- +                                        continue;

- +                                }

- +

- +                                bpf_program_ref(p);

- +                        }

- +        }

- +

- +        log_debug("Pinned %d BPF programs", set_size(m->bpf_limbo_progs));

- +

- +        return m;

- +}

- +

- +static void manager_skeletonize_all_cgroup_bpf_programs(Manager *m) {

- +        BPFProgram *p;

- +        Iterator i;

- +

- +        SET_FOREACH(p, m->bpf_limbo_progs, i)

- +                bpf_program_skeletonize(p);

- +}

- +

- +void manager_unpin_all_cgroup_bpf_programs(Manager *m) {

- +        log_debug("Unpinning %d BPF programs", set_size(m->bpf_limbo_progs));

- +        set_clear_with_destructor(m->bpf_limbo_progs, bpf_program_unref);

- +}

- +

-  int manager_reload(Manager *m) {

-          _cleanup_(manager_reloading_stopp) Manager *reloading = NULL;

-          _cleanup_fdset_free_ FDSet *fds = NULL;

- @@ -3738,6 +3880,13 @@ int manager_reload(Manager *m) {

-          /* We are officially in reload mode from here on. */

-          reloading = manager_reloading_start(m);

-  

- +        /* We need existing BPF programs to survive reload, otherwise there will be a period where no BPF

- +         * program is active during task execution within a cgroup. This would be bad since this may have

- +         * security or reliability implications: devices we should filter won't be filtered, network activity

- +         * we should filter won't be filtered, etc. We pin all the existing devices by bumping their

- +         * refcount, and then storing them to later have it decremented. */

- +        (void) manager_pin_all_cgroup_bpf_programs(m);

- +

-          r = manager_serialize(m, f, fds, false);

-          if (r < 0)

-                  return r;

- @@ -3762,6 +3911,12 @@ int manager_reload(Manager *m) {

-          m->uid_refs = hashmap_free(m->uid_refs);

-          m->gid_refs = hashmap_free(m->gid_refs);

-  

- +        /* The only canonical reference left to the dynamically allocated parts of these BPF programs is

- +         * going to be on the other side of manager_deserialize, so the freeable parts can now be freed. The

- +         * program itself will be detached as part of manager_vacuum. */

- +        manager_skeletonize_all_cgroup_bpf_programs(m);

- +        m->bpf_limbo_progs = set_free(m->bpf_limbo_progs);

- +

-          r = lookup_paths_init(&m->lookup_paths, m->unit_file_scope, 0, NULL);

-          if (r < 0)

-                  log_warning_errno(r, "Failed to initialize path lookup table, ignoring: %m");

- @@ -4700,6 +4855,12 @@ static void manager_vacuum(Manager *m) {

-  

-          /* Release any runtimes no longer referenced */

-          exec_runtime_vacuum(m);

- +

- +        /* Release any outmoded BPF programs that were deserialized from the previous manager, since new ones

- +         * should be in action now. We first need to make sure all entries in the cgroup realize queue are

- +         * complete, otherwise BPF firewalls/etc may not have been set up yet. */

- +        (void) manager_dispatch_cgroup_realize_queue(m);

- +        manager_unpin_all_cgroup_bpf_programs(m);

-  }

-  

-  int manager_dispatch_user_lookup_fd(sd_event_source *source, int fd, uint32_t revents, void *userdata) {

- diff --git a/src/core/manager.h b/src/core/manager.h

- index 81b0c13a95..6f8f8b04b4 100644

- --- a/src/core/manager.h

- +++ b/src/core/manager.h

- @@ -433,6 +433,8 @@ struct Manager {

-          bool honor_device_enumeration;

-  

-          VarlinkServer *varlink_server;

- +

- +        Set *bpf_limbo_progs;

-  };

-  

-  static inline usec_t manager_default_timeout_abort_usec(Manager *m) {

- @@ -474,6 +476,10 @@ int manager_add_job_by_name(Manager *m, JobType type, const char *name, JobMode

-  int manager_add_job_by_name_and_warn(Manager *m, JobType type, const char *name, JobMode mode, Set *affected_jobs,  Job **ret);

-  int manager_propagate_reload(Manager *m, Unit *unit, JobMode mode, sd_bus_error *e);

-  

- +Manager* manager_pin_all_cgroup_bpf_programs(Manager *m);

- +void manager_unpin_all_cgroup_bpf_programs(Manager *m);

- +DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_unpin_all_cgroup_bpf_programs);

- +

-  void manager_dump_units(Manager *s, FILE *f, const char *prefix);

-  void manager_dump_jobs(Manager *s, FILE *f, const char *prefix);

-  void manager_dump(Manager *s, FILE *f, const char *prefix);

- diff --git a/src/shared/bpf-program.c b/src/shared/bpf-program.c

- index e5c9df4004..cc479aa52e 100644

- --- a/src/shared/bpf-program.c

- +++ b/src/shared/bpf-program.c

- @@ -210,6 +210,16 @@ int bpf_program_cgroup_detach(BPFProgram *p) {

-          return 0;

-  }

-  

- +void bpf_program_skeletonize(BPFProgram *p) {

- +        assert(p);

- +

- +        /* Called shortly after serialization. From this point on, we are frozen for serialization and entry

- +         * into BPF limbo, so we should proactively free our instructions and attached path. However, we

- +         * shouldn't detach the program or close the kernel FD -- we need those on the other side. */

- +        free(p->instructions);

- +        free(p->attached_path);

- +}

- +

-  int bpf_map_new(enum bpf_map_type type, size_t key_size, size_t value_size, size_t max_entries, uint32_t flags) {

-          union bpf_attr attr = {

-                  .map_type = type,

- diff --git a/src/shared/bpf-program.h b/src/shared/bpf-program.h

- index a21589eb1f..6ea5d9a57c 100644

- --- a/src/shared/bpf-program.h

- +++ b/src/shared/bpf-program.h

- @@ -28,6 +28,7 @@ struct BPFProgram {

-  int bpf_program_new(uint32_t prog_type, BPFProgram **ret);

-  BPFProgram *bpf_program_unref(BPFProgram *p);

-  BPFProgram *bpf_program_ref(BPFProgram *p);

- +void bpf_program_skeletonize(BPFProgram *p);

-  

-  int bpf_program_add_instructions(BPFProgram *p, const struct bpf_insn *insn, size_t count);

-  int bpf_program_load_kernel(BPFProgram *p, char *log_buf, size_t log_size);

- -- 

- 2.24.1

-