SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

bpf: make sure the kernel do not submit an invalid program if no pattern matched

Browse Source 
It turns out that the kernel verifier would reject a program we would build
if there was a whitelist, but no entries in the whitelist matched.
The program would approximately like this:
   0: (61) r2 = *(u32 *)(r1 +0)
   1: (54) w2 &= 65535
   2: (61) r3 = *(u32 *)(r1 +0)
   3: (74) w3 >>= 16
   4: (61) r4 = *(u32 *)(r1 +4)
   5: (61) r5 = *(u32 *)(r1 +8)
  48: (b7) r0 = 0
  49: (05) goto pc+1
  50: (b7) r0 = 1
  51: (95) exit
and insn 50 is unreachable, which is illegal. We would then either keep a
previous version of the program or allow everything. Make sure we build a
valid program that simply rejects everything.
This commit is contained in:
Zbigniew Jędrzejewski-Szmek
2019-11-11 15:14:09 +01:00
parent 0048657828
commit 45669ae264
3 changed files with 68 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/core/bpf-devices.c
+6 -1
View File
@@ -395,7 +395,7 @@ int bpf_devices_whitelist_major(BPFProgram *prog, const char *path, const char *
return whitelist_device_pattern(prog, path, type, &maj, NULL, acc);
_cleanup_fclose_ FILE *f = NULL;
bool good = false;
bool good = false, any = false;
f = fopen("/proc/devices", "re");
if (!f)
@@ -448,9 +448,14 @@ int bpf_devices_whitelist_major(BPFProgram *prog, const char *path, const char *
if (fnmatch(name, w, 0) != 0)
continue;
any = true;
(void) whitelist_device_pattern(prog, path, type, &maj, NULL, acc);
}
if (!any)
return log_debug_errno(SYNTHETIC_ERRNO(ENOENT),
"Device whitelist pattern \"%s\" did not match anything.", name);
return 0;
}
src/core/cgroup.c
+28 -11
View File
@@ -960,13 +960,16 @@ static int cgroup_apply_devices(Unit *u) {
const char *path;
CGroupContext *c;
CGroupDeviceAllow *a;
CGroupDevicePolicy policy;
int r;
assert_se(c = unit_get_cgroup_context(u));
assert_se(path = u->cgroup_path);
policy = c->device_policy;
if (cg_all_unified() > 0) {
r = bpf_devices_cgroup_init(&prog, c->device_policy, c->device_allow);
r = bpf_devices_cgroup_init(&prog, policy, c->device_allow);
if (r < 0)
return log_unit_warning_errno(u, r, "Failed to initialize device control bpf program: %m");
@@ -974,7 +977,7 @@ static int cgroup_apply_devices(Unit *u) {
/* Changing the devices list of a populated cgroup might result in EINVAL, hence ignore
* EINVAL here. */
if (c->device_allow || c->device_policy != CGROUP_DEVICE_POLICY_AUTO)
if (c->device_allow || policy != CGROUP_DEVICE_POLICY_AUTO)
r = cg_set_attribute("devices", path, "devices.deny", "a");
else
r = cg_set_attribute("devices", path, "devices.allow", "a");
@@ -983,10 +986,12 @@ static int cgroup_apply_devices(Unit *u) {
"Failed to reset devices.allow/devices.deny: %m");
}
if (c->device_policy == CGROUP_DEVICE_POLICY_CLOSED ||
(c->device_policy == CGROUP_DEVICE_POLICY_AUTO && c->device_allow))
bool whitelist_static = policy == CGROUP_DEVICE_POLICY_CLOSED ||
(policy == CGROUP_DEVICE_POLICY_AUTO && c->device_allow);
if (whitelist_static)
(void) bpf_devices_whitelist_static(prog, path);
bool any = whitelist_static;
LIST_FOREACH(device_allow, a, c->device_allow) {
char acc[4], *val;
unsigned k = 0;
@@ -997,23 +1002,35 @@ static int cgroup_apply_devices(Unit *u) {
acc[k++] = 'w';
if (a->m)
acc[k++] = 'm';
if (k == 0)
continue;
acc[k++] = 0;
if (path_startswith(a->path, "/dev/"))
(void) bpf_devices_whitelist_device(prog, path, a->path, acc);
r = bpf_devices_whitelist_device(prog, path, a->path, acc);
else if ((val = startswith(a->path, "block-")))
(void) bpf_devices_whitelist_major(prog, path, val, 'b', acc);
r = bpf_devices_whitelist_major(prog, path, val, 'b', acc);
else if ((val = startswith(a->path, "char-")))
(void) bpf_devices_whitelist_major(prog, path, val, 'c', acc);
else
r = bpf_devices_whitelist_major(prog, path, val, 'c', acc);
else {
log_unit_debug(u, "Ignoring device '%s' while writing cgroup attribute.", a->path);
continue;
}
if (r >= 0)
any = true;
}
r = bpf_devices_apply_policy(prog, c->device_policy, c->device_allow, path, &u->bpf_device_control_installed);
if (prog && !any) {
log_unit_warning_errno(u, SYNTHETIC_ERRNO(ENODEV), "No devices matched by device filter.");
/* The kernel verifier would reject a program we would build with the normal intro and outro
but no whitelisting rules (outro would contain an unreachable instruction for successful
return). */
policy = CGROUP_DEVICE_POLICY_STRICT;
}
r = bpf_devices_apply_policy(prog, policy, any, path, &u->bpf_device_control_installed);
if (r < 0) {
static bool warned = false;
src/test/test-bpf-devices.c
+34
View File
@@ -219,6 +219,37 @@ static void test_policy_whitelist_major_star(char type, const char *cgroup_path,
assert_se(wrong == 0);
}
static void test_policy_empty(bool add_mismatched, const char *cgroup_path, BPFProgram **installed_prog) {
_cleanup_(bpf_program_unrefp) BPFProgram *prog = NULL;
unsigned wrong = 0;
int r;
log_info("/* %s(add_mismatched=%s) */", __func__, yes_no(add_mismatched));
r = bpf_devices_cgroup_init(&prog, CGROUP_DEVICE_POLICY_STRICT, add_mismatched);
assert_se(r >= 0);
if (add_mismatched) {
r = bpf_devices_whitelist_major(prog, cgroup_path, "foobarxxx", 'c', "rw");
assert_se(r < 0);
}
r = bpf_devices_apply_policy(prog, CGROUP_DEVICE_POLICY_STRICT, false, cgroup_path, installed_prog);
assert_se(r >= 0);
{
_cleanup_close_ int fd;
const char *s = "/dev/null";
fd = open(s, O_CLOEXEC|O_RDWR|O_NOCTTY);
log_debug("open(%s, \"r\") = %d/%s", s, fd, fd < 0 ? errno_to_name(errno) : "-");
wrong += fd >= 0;
}
assert_se(wrong == 0);
}
int main(int argc, char *argv[]) {
_cleanup_free_ char *cgroup = NULL, *parent = NULL;
_cleanup_(rmdir_and_freep) char *controller_path = NULL;
@@ -258,6 +289,9 @@ int main(int argc, char *argv[]) {
test_policy_whitelist_major_star('c', cgroup, &prog);
test_policy_whitelist_major_star('b', cgroup, &prog);
test_policy_empty(false, cgroup, &prog);
test_policy_empty(true, cgroup, &prog);
assert_se(parent = dirname_malloc(cgroup));
assert_se(cg_mask_supported(&supported) >= 0);