Message ID | 20200731135145.15003-1-daniel@iogearbox.net |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | pull-request: bpf 2020-07-31 | expand |
On Fri, Jul 31, 2020 at 03:51:45PM +0200, Daniel Borkmann wrote: > Hi David, > > The following pull-request contains BPF updates for your *net* tree. > > We've added 5 non-merge commits during the last 21 day(s) which contain > a total of 5 files changed, 126 insertions(+), 18 deletions(-). > > The main changes are: > > 1) Fix a map element leak in HASH_OF_MAPS map type, from Andrii Nakryiko. > > 2) Fix a NULL pointer dereference in __btf_resolve_helper_id() when no > btf_vmlinux is available, from Peilin Ye. > > 3) Init pos variable in __bpfilter_process_sockopt(), from Christoph Hellwig. > > 4) Fix a cgroup sockopt verifier test by specifying expected attach type, > from Jean-Philippe Brucker. > > Please consider pulling these changes from: > > git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git > > Thanks a lot! > > Note that when net gets merged into net-next later on, there is a small > merge conflict in kernel/bpf/btf.c between commit 5b801dfb7feb ("bpf: Fix > NULL pointer dereference in __btf_resolve_helper_id()") from the bpf tree > and commit 138b9a0511c7 ("bpf: Remove btf_id helpers resolving") from the > net-next tree. > > Resolve as follows: remove the old hunk with the __btf_resolve_helper_id() > function. Change the btf_resolve_helper_id() so it actually tests for a > NULL btf_vmlinux and bails out: > > int btf_resolve_helper_id(struct bpf_verifier_log *log, > const struct bpf_func_proto *fn, int arg) > { > int id; > > if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID || !btf_vmlinux) > return -EINVAL; > id = fn->btf_id[arg]; > if (!id || id > btf_vmlinux->nr_types) > return -EINVAL; > return id; > } > > Let me know if you run into any others issues (CC'ing Jiri Olsa so he's in > the loop with regards to merge conflict resolution). we'll loose the bpf_log message, but I'm fine with that ;-) looks good thanks, jirka
On 7/31/20 5:24 PM, Jiri Olsa wrote: > On Fri, Jul 31, 2020 at 03:51:45PM +0200, Daniel Borkmann wrote: >> Hi David, >> >> The following pull-request contains BPF updates for your *net* tree. >> >> We've added 5 non-merge commits during the last 21 day(s) which contain >> a total of 5 files changed, 126 insertions(+), 18 deletions(-). >> >> The main changes are: >> >> 1) Fix a map element leak in HASH_OF_MAPS map type, from Andrii Nakryiko. >> >> 2) Fix a NULL pointer dereference in __btf_resolve_helper_id() when no >> btf_vmlinux is available, from Peilin Ye. >> >> 3) Init pos variable in __bpfilter_process_sockopt(), from Christoph Hellwig. >> >> 4) Fix a cgroup sockopt verifier test by specifying expected attach type, >> from Jean-Philippe Brucker. >> >> Please consider pulling these changes from: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git >> >> Thanks a lot! >> >> Note that when net gets merged into net-next later on, there is a small >> merge conflict in kernel/bpf/btf.c between commit 5b801dfb7feb ("bpf: Fix >> NULL pointer dereference in __btf_resolve_helper_id()") from the bpf tree >> and commit 138b9a0511c7 ("bpf: Remove btf_id helpers resolving") from the >> net-next tree. >> >> Resolve as follows: remove the old hunk with the __btf_resolve_helper_id() >> function. Change the btf_resolve_helper_id() so it actually tests for a >> NULL btf_vmlinux and bails out: >> >> int btf_resolve_helper_id(struct bpf_verifier_log *log, >> const struct bpf_func_proto *fn, int arg) >> { >> int id; >> >> if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID || !btf_vmlinux) >> return -EINVAL; >> id = fn->btf_id[arg]; >> if (!id || id > btf_vmlinux->nr_types) >> return -EINVAL; >> return id; >> } >> >> Let me know if you run into any others issues (CC'ing Jiri Olsa so he's in >> the loop with regards to merge conflict resolution). > > we'll loose the bpf_log message, but I'm fine with that ;-) looks good Checking again on the fix, even though it was only triggered by syzkaller so far, I think it's also possible if users don't have BTF debug data set in the Kconfig but use a helper that expects it, so agree, lets re-add the log in this case: int btf_resolve_helper_id(struct bpf_verifier_log *log, const struct bpf_func_proto *fn, int arg) { int id; if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID) return -EINVAL; if (!btf_vmlinux) { bpf_log(log, "btf_vmlinux doesn't exist\n"); return -EINVAL; } id = fn->btf_id[arg]; if (!id || id > btf_vmlinux->nr_types) return -EINVAL; return id; } Thanks, Daniel
On Fri, Jul 31, 2020 at 06:12:48PM +0200, Daniel Borkmann wrote: SNIP > > > return -EINVAL; > > > return id; > > > } > > > > > > Let me know if you run into any others issues (CC'ing Jiri Olsa so he's in > > > the loop with regards to merge conflict resolution). > > > > we'll loose the bpf_log message, but I'm fine with that ;-) looks good > > Checking again on the fix, even though it was only triggered by syzkaller > so far, I think it's also possible if users don't have BTF debug data set > in the Kconfig but use a helper that expects it, so agree, lets re-add the > log in this case: > > int btf_resolve_helper_id(struct bpf_verifier_log *log, > const struct bpf_func_proto *fn, int arg) > { > int id; > > if (fn->arg_type[arg] != ARG_PTR_TO_BTF_ID) > return -EINVAL; > if (!btf_vmlinux) { > bpf_log(log, "btf_vmlinux doesn't exist\n"); > return -EINVAL; > } > id = fn->btf_id[arg]; > if (!id || id > btf_vmlinux->nr_types) > return -EINVAL; > return id; > } ok, looks good jirka
From: Daniel Borkmann <daniel@iogearbox.net> Date: Fri, 31 Jul 2020 15:51:45 +0200 > The following pull-request contains BPF updates for your *net* tree. > > We've added 5 non-merge commits during the last 21 day(s) which contain > a total of 5 files changed, 126 insertions(+), 18 deletions(-). > > The main changes are: > > 1) Fix a map element leak in HASH_OF_MAPS map type, from Andrii Nakryiko. > > 2) Fix a NULL pointer dereference in __btf_resolve_helper_id() when no > btf_vmlinux is available, from Peilin Ye. > > 3) Init pos variable in __bpfilter_process_sockopt(), from Christoph Hellwig. > > 4) Fix a cgroup sockopt verifier test by specifying expected attach type, > from Jean-Philippe Brucker. > > Please consider pulling these changes from: > > git://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf.git Pulled. > Note that when net gets merged into net-next later on, there is a small > merge conflict in kernel/bpf/btf.c between commit 5b801dfb7feb ("bpf: Fix > NULL pointer dereference in __btf_resolve_helper_id()") from the bpf tree > and commit 138b9a0511c7 ("bpf: Remove btf_id helpers resolving") from the > net-next tree. > > Resolve as follows: remove the old hunk with the __btf_resolve_helper_id() > function. Change the btf_resolve_helper_id() so it actually tests for a > NULL btf_vmlinux and bails out: ... Noted, thank you.