From 405645d78889b5effdcfbcc0d9ef6ba75a3ac40d Mon Sep 17 00:00:00 2001 From: Ben Hutchings Date: Thu, 5 May 2016 23:13:51 +0100 Subject: [PATCH] Add bpf security fixes --- debian/changelog | 3 + ...x-check_map_func_compatibility-logic.patch | 94 +++++++++++ ...fdput-in-replace_map_fd_with_map_ptr.patch | 41 +++++ .../bugfix/all/bpf-fix-refcnt-overflow.patch | 147 ++++++++++++++++++ debian/patches/series | 3 + 5 files changed, 288 insertions(+) create mode 100644 debian/patches/bugfix/all/bpf-fix-check_map_func_compatibility-logic.patch create mode 100644 debian/patches/bugfix/all/bpf-fix-double-fdput-in-replace_map_fd_with_map_ptr.patch create mode 100644 debian/patches/bugfix/all/bpf-fix-refcnt-overflow.patch diff --git a/debian/changelog b/debian/changelog index 67ca9af22..919e3eb6d 100644 --- a/debian/changelog +++ b/debian/changelog @@ -7,6 +7,9 @@ linux (4.5.2-2) UNRELEASED; urgency=medium * bug control: Update list of related firmware packages * Revert "sp5100_tco: fix the device check for SB800 and later chipsets" (Closes: #823146; probably fixes #822651) + * bpf: fix double-fdput in replace_map_fd_with_map_ptr() (CVE-2016-XXXX) + * bpf: fix refcnt overflow (CVE-2016-XXXX) + * bpf: fix check_map_func_compatibility logic (CVE-2016-XXXX) -- Uwe Kleine-König Sun, 01 May 2016 16:13:04 +0200 diff --git a/debian/patches/bugfix/all/bpf-fix-check_map_func_compatibility-logic.patch b/debian/patches/bugfix/all/bpf-fix-check_map_func_compatibility-logic.patch new file mode 100644 index 000000000..83a0254f5 --- /dev/null +++ b/debian/patches/bugfix/all/bpf-fix-check_map_func_compatibility-logic.patch @@ -0,0 +1,94 @@ +From: Alexei Starovoitov +Date: Wed, 27 Apr 2016 18:56:21 -0700 +Subject: [3/3] bpf: fix check_map_func_compatibility logic +Origin: https://git.kernel.org/linus/6aff67c85c9e5a4bc99e5211c1bac547936626ca + +The commit 35578d798400 ("bpf: Implement function bpf_perf_event_read() that get the selected hardware PMU conuter") +introduced clever way to check bpf_helper<->map_type compatibility. +Later on commit a43eec304259 ("bpf: introduce bpf_perf_event_output() helper") adjusted +the logic and inadvertently broke it. +Get rid of the clever bool compare and go back to two-way check +from map and from helper perspective. + +Fixes: a43eec304259 ("bpf: introduce bpf_perf_event_output() helper") +Reported-by: Jann Horn +Signed-off-by: Alexei Starovoitov +Signed-off-by: Daniel Borkmann +Signed-off-by: David S. Miller +[bwh: Backported to 4.5: + - Drop the STACK_TRACE case + - No verbose() logging] +--- +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -239,15 +239,6 @@ static const char * const reg_type_str[] + [CONST_IMM] = "imm", + }; + +-static const struct { +- int map_type; +- int func_id; +-} func_limit[] = { +- {BPF_MAP_TYPE_PROG_ARRAY, BPF_FUNC_tail_call}, +- {BPF_MAP_TYPE_PERF_EVENT_ARRAY, BPF_FUNC_perf_event_read}, +- {BPF_MAP_TYPE_PERF_EVENT_ARRAY, BPF_FUNC_perf_event_output}, +-}; +- + static void print_verifier_state(struct verifier_env *env) + { + enum bpf_reg_type t; +@@ -898,24 +889,42 @@ static int check_func_arg(struct verifie + + static int check_map_func_compatibility(struct bpf_map *map, int func_id) + { +- bool bool_map, bool_func; +- int i; +- + if (!map) + return 0; + +- for (i = 0; i < ARRAY_SIZE(func_limit); i++) { +- bool_map = (map->map_type == func_limit[i].map_type); +- bool_func = (func_id == func_limit[i].func_id); +- /* only when map & func pair match it can continue. +- * don't allow any other map type to be passed into +- * the special func; +- */ +- if (bool_func && bool_map != bool_func) +- return -EINVAL; ++ /* We need a two way check, first is from map perspective ... */ ++ switch (map->map_type) { ++ case BPF_MAP_TYPE_PROG_ARRAY: ++ if (func_id != BPF_FUNC_tail_call) ++ goto error; ++ break; ++ case BPF_MAP_TYPE_PERF_EVENT_ARRAY: ++ if (func_id != BPF_FUNC_perf_event_read && ++ func_id != BPF_FUNC_perf_event_output) ++ goto error; ++ break; ++ default: ++ break; ++ } ++ ++ /* ... and second from the function itself. */ ++ switch (func_id) { ++ case BPF_FUNC_tail_call: ++ if (map->map_type != BPF_MAP_TYPE_PROG_ARRAY) ++ goto error; ++ break; ++ case BPF_FUNC_perf_event_read: ++ case BPF_FUNC_perf_event_output: ++ if (map->map_type != BPF_MAP_TYPE_PERF_EVENT_ARRAY) ++ goto error; ++ break; ++ default: ++ break; + } + + return 0; ++error: ++ return -EINVAL; + } + + static int check_call(struct verifier_env *env, int func_id) diff --git a/debian/patches/bugfix/all/bpf-fix-double-fdput-in-replace_map_fd_with_map_ptr.patch b/debian/patches/bugfix/all/bpf-fix-double-fdput-in-replace_map_fd_with_map_ptr.patch new file mode 100644 index 000000000..4c43fcdb7 --- /dev/null +++ b/debian/patches/bugfix/all/bpf-fix-double-fdput-in-replace_map_fd_with_map_ptr.patch @@ -0,0 +1,41 @@ +From: Jann Horn +Date: Tue, 26 Apr 2016 22:26:26 +0200 +Subject: [1/3] bpf: fix double-fdput in replace_map_fd_with_map_ptr() +Origin: https://git.kernel.org/linus/8358b02bf67d3a5d8a825070e1aa73f25fb2e4c7 + +When bpf(BPF_PROG_LOAD, ...) was invoked with a BPF program whose bytecode +references a non-map file descriptor as a map file descriptor, the error +handling code called fdput() twice instead of once (in __bpf_map_get() and +in replace_map_fd_with_map_ptr()). If the file descriptor table of the +current task is shared, this causes f_count to be decremented too much, +allowing the struct file to be freed while it is still in use +(use-after-free). This can be exploited to gain root privileges by an +unprivileged user. + +This bug was introduced in +commit 0246e64d9a5f ("bpf: handle pseudo BPF_LD_IMM64 insn"), but is only +exploitable since +commit 1be7f75d1668 ("bpf: enable non-root eBPF programs") because +previously, CAP_SYS_ADMIN was required to reach the vulnerable code. + +(posted publicly according to request by maintainer) + +Signed-off-by: Jann Horn +Signed-off-by: Linus Torvalds +Acked-by: Alexei Starovoitov +Acked-by: Daniel Borkmann +Signed-off-by: David S. Miller +--- + kernel/bpf/verifier.c | 1 - + 1 file changed, 1 deletion(-) + +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -2003,7 +2003,6 @@ static int replace_map_fd_with_map_ptr(s + if (IS_ERR(map)) { + verbose("fd %d is not pointing to valid bpf_map\n", + insn->imm); +- fdput(f); + return PTR_ERR(map); + } + diff --git a/debian/patches/bugfix/all/bpf-fix-refcnt-overflow.patch b/debian/patches/bugfix/all/bpf-fix-refcnt-overflow.patch new file mode 100644 index 000000000..a5b3d77fc --- /dev/null +++ b/debian/patches/bugfix/all/bpf-fix-refcnt-overflow.patch @@ -0,0 +1,147 @@ +From: Alexei Starovoitov +Date: Wed, 27 Apr 2016 18:56:20 -0700 +Subject: [2/3] bpf: fix refcnt overflow +Origin: https://git.kernel.org/linus/92117d8443bc5afacc8d5ba82e541946310f106e + +On a system with >32Gbyte of phyiscal memory and infinite RLIMIT_MEMLOCK, +the malicious application may overflow 32-bit bpf program refcnt. +It's also possible to overflow map refcnt on 1Tb system. +Impose 32k hard limit which means that the same bpf program or +map cannot be shared by more than 32k processes. + +Fixes: 1be7f75d1668 ("bpf: enable non-root eBPF programs") +Reported-by: Jann Horn +Signed-off-by: Alexei Starovoitov +Acked-by: Daniel Borkmann +Signed-off-by: David S. Miller +--- + include/linux/bpf.h | 3 ++- + kernel/bpf/inode.c | 7 ++++--- + kernel/bpf/syscall.c | 24 ++++++++++++++++++++---- + kernel/bpf/verifier.c | 11 +++++++---- + 4 files changed, 33 insertions(+), 12 deletions(-) + +--- a/include/linux/bpf.h ++++ b/include/linux/bpf.h +@@ -165,12 +165,13 @@ void bpf_register_prog_type(struct bpf_p + void bpf_register_map_type(struct bpf_map_type_list *tl); + + struct bpf_prog *bpf_prog_get(u32 ufd); ++struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog); + void bpf_prog_put(struct bpf_prog *prog); + void bpf_prog_put_rcu(struct bpf_prog *prog); + + struct bpf_map *bpf_map_get_with_uref(u32 ufd); + struct bpf_map *__bpf_map_get(struct fd f); +-void bpf_map_inc(struct bpf_map *map, bool uref); ++struct bpf_map *bpf_map_inc(struct bpf_map *map, bool uref); + void bpf_map_put_with_uref(struct bpf_map *map); + void bpf_map_put(struct bpf_map *map); + +--- a/kernel/bpf/inode.c ++++ b/kernel/bpf/inode.c +@@ -31,10 +31,10 @@ static void *bpf_any_get(void *raw, enum + { + switch (type) { + case BPF_TYPE_PROG: +- atomic_inc(&((struct bpf_prog *)raw)->aux->refcnt); ++ raw = bpf_prog_inc(raw); + break; + case BPF_TYPE_MAP: +- bpf_map_inc(raw, true); ++ raw = bpf_map_inc(raw, true); + break; + default: + WARN_ON_ONCE(1); +@@ -297,7 +297,8 @@ static void *bpf_obj_do_get(const struct + goto out; + + raw = bpf_any_get(inode->i_private, *type); +- touch_atime(&path); ++ if (!IS_ERR(raw)) ++ touch_atime(&path); + + path_put(&path); + return raw; +--- a/kernel/bpf/syscall.c ++++ b/kernel/bpf/syscall.c +@@ -201,11 +201,18 @@ struct bpf_map *__bpf_map_get(struct fd + return f.file->private_data; + } + +-void bpf_map_inc(struct bpf_map *map, bool uref) ++/* prog's and map's refcnt limit */ ++#define BPF_MAX_REFCNT 32768 ++ ++struct bpf_map *bpf_map_inc(struct bpf_map *map, bool uref) + { +- atomic_inc(&map->refcnt); ++ if (atomic_inc_return(&map->refcnt) > BPF_MAX_REFCNT) { ++ atomic_dec(&map->refcnt); ++ return ERR_PTR(-EBUSY); ++ } + if (uref) + atomic_inc(&map->usercnt); ++ return map; + } + + struct bpf_map *bpf_map_get_with_uref(u32 ufd) +@@ -217,7 +224,7 @@ struct bpf_map *bpf_map_get_with_uref(u3 + if (IS_ERR(map)) + return map; + +- bpf_map_inc(map, true); ++ map = bpf_map_inc(map, true); + fdput(f); + + return map; +@@ -600,6 +607,15 @@ static struct bpf_prog *__bpf_prog_get(s + return f.file->private_data; + } + ++struct bpf_prog *bpf_prog_inc(struct bpf_prog *prog) ++{ ++ if (atomic_inc_return(&prog->aux->refcnt) > BPF_MAX_REFCNT) { ++ atomic_dec(&prog->aux->refcnt); ++ return ERR_PTR(-EBUSY); ++ } ++ return prog; ++} ++ + /* called by sockets/tracing/seccomp before attaching program to an event + * pairs with bpf_prog_put() + */ +@@ -612,7 +628,7 @@ struct bpf_prog *bpf_prog_get(u32 ufd) + if (IS_ERR(prog)) + return prog; + +- atomic_inc(&prog->aux->refcnt); ++ prog = bpf_prog_inc(prog); + fdput(f); + + return prog; +--- a/kernel/bpf/verifier.c ++++ b/kernel/bpf/verifier.c +@@ -2022,15 +2022,18 @@ static int replace_map_fd_with_map_ptr(s + return -E2BIG; + } + +- /* remember this map */ +- env->used_maps[env->used_map_cnt++] = map; +- + /* hold the map. If the program is rejected by verifier, + * the map will be released by release_maps() or it + * will be used by the valid program until it's unloaded + * and all maps are released in free_bpf_prog_info() + */ +- bpf_map_inc(map, false); ++ map = bpf_map_inc(map, false); ++ if (IS_ERR(map)) { ++ fdput(f); ++ return PTR_ERR(map); ++ } ++ env->used_maps[env->used_map_cnt++] = map; ++ + fdput(f); + next_insn: + insn++; diff --git a/debian/patches/series b/debian/patches/series index 91d8b4a53..6211c5bf8 100644 --- a/debian/patches/series +++ b/debian/patches/series @@ -155,3 +155,6 @@ bugfix/all/power-cpupower-fix-manpages-NAME.patch bugfix/all/tools-lib-traceevent-fix-use-of-uninitialized-variables.patch bugfix/all/scripts-fix-x.509-pem-support-in-sign-file.patch bugfix/x86/revert-sp5100_tco-fix-the-device-check-for-SB800-and.patch +bugfix/all/bpf-fix-double-fdput-in-replace_map_fd_with_map_ptr.patch +bugfix/all/bpf-fix-refcnt-overflow.patch +bugfix/all/bpf-fix-check_map_func_compatibility-logic.patch