| Message ID | 20220215065732.3179408-1-houtao1@huawei.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 8cbf062a250ed52148badf6f3ffd03657dd4a3f0 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next,v4] bpf: reject kfunc calls that overflow insn->imm | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/fixes_present | success | Fixes tag not required for -next series |
| netdev/subject_prefix | success | Link |
| netdev/cover_letter | success | Single patches do not need cover letters |
| netdev/patch_count | success | Link |
| netdev/header_inline | success | No static functions without inline keyword in header files |
| netdev/build_32bit | success | Errors and warnings before: 20 this patch: 20 |
| netdev/cc_maintainers | success | CCed 10 of 10 maintainers |
| netdev/build_clang | success | Errors and warnings before: 18 this patch: 18 |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Signed-off-by tag matches author and committer |
| netdev/verify_fixes | success | No Fixes tag |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 25 this patch: 25 |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 25 lines checked |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
| bpf/vmtest-bpf-next | success | VM_Test |
On 2/14/22 10:57 PM, Hou Tao wrote: > Now kfunc call uses s32 to represent the offset between the address of > kfunc and __bpf_call_base, but it doesn't check whether or not s32 will > be overflowed. The overflow is possible when kfunc is in module and the > offset between module and kernel is greater than 2GB. Take arm64 as an > example, before commit b2eed9b58811 ("arm64/kernel: kaslr: reduce module > randomization range to 2 GB"), the offset between module symbol and > __bpf_call_base will in 4GB range due to KASLR and may overflow s32. > > So add an extra checking to reject these invalid kfunc calls. > > Signed-off-by: Hou Tao <houtao1@huawei.com> Acked-by: Yonghong Song <yhs@fb.com>
Hello: This patch was applied to bpf/bpf-next.git (master) by Alexei Starovoitov <ast@kernel.org>: On Tue, 15 Feb 2022 14:57:32 +0800 you wrote: > Now kfunc call uses s32 to represent the offset between the address of > kfunc and __bpf_call_base, but it doesn't check whether or not s32 will > be overflowed. The overflow is possible when kfunc is in module and the > offset between module and kernel is greater than 2GB. Take arm64 as an > example, before commit b2eed9b58811 ("arm64/kernel: kaslr: reduce module > randomization range to 2 GB"), the offset between module symbol and > __bpf_call_base will in 4GB range due to KASLR and may overflow s32. > > [...] Here is the summary with links: - [bpf-next,v4] bpf: reject kfunc calls that overflow insn->imm https://git.kernel.org/bpf/bpf-next/c/8cbf062a250e You are awesome, thank you!
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index bbef86cb4e72..d7473fee247c 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1842,6 +1842,7 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset) struct bpf_kfunc_desc *desc; const char *func_name; struct btf *desc_btf; + unsigned long call_imm; unsigned long addr; int err; @@ -1926,9 +1927,17 @@ static int add_kfunc_call(struct bpf_verifier_env *env, u32 func_id, s16 offset) return -EINVAL; } + call_imm = BPF_CALL_IMM(addr); + /* Check whether or not the relative offset overflows desc->imm */ + if ((unsigned long)(s32)call_imm != call_imm) { + verbose(env, "address of kernel function %s is out of range\n", + func_name); + return -EINVAL; + } + desc = &tab->descs[tab->nr_descs++]; desc->func_id = func_id; - desc->imm = BPF_CALL_IMM(addr); + desc->imm = call_imm; desc->offset = offset; err = btf_distill_func_proto(&env->log, desc_btf, func_proto, func_name,
Now kfunc call uses s32 to represent the offset between the address of kfunc and __bpf_call_base, but it doesn't check whether or not s32 will be overflowed. The overflow is possible when kfunc is in module and the offset between module and kernel is greater than 2GB. Take arm64 as an example, before commit b2eed9b58811 ("arm64/kernel: kaslr: reduce module randomization range to 2 GB"), the offset between module symbol and __bpf_call_base will in 4GB range due to KASLR and may overflow s32. So add an extra checking to reject these invalid kfunc calls. Signed-off-by: Hou Tao <houtao1@huawei.com> --- v4: * explain why the overflow check is needed. v3: https://lore.kernel.org/bpf/2339465e-1f87-595a-2954-eb92b6bfa9cc@huawei.com * call BPF_CALL_IMM() once (suggested by Yonghong) v2: https://lore.kernel.org/bpf/20220208123348.40360-1-houtao1@huawei.com * instead of checking the overflow in selftests, just reject these kfunc calls directly in verifier v1: https://lore.kernel.org/bpf/20220206043107.18549-1-houtao1@huawei.com --- kernel/bpf/verifier.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)