| Message ID | YiWYLnAkEZXBP/gH@syu-laptop (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 44e9a741cad824f45112b79c21d88c201d5aec13 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] bpf: determine buf_info inside check_buffer_access() | expand |
| Context | Check | Description |
|---|---|---|
| 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 | warning | WARNING: line length of 81 exceeds 80 columns |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/source_inline | success | Was 0 now: 0 |
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
On 3/6/22 9:29 PM, Shung-Hsi Yu wrote: > Instead of determining buf_info string in the caller of > check_buffer_access(), we can determine whether the register type is read-only > through type_is_rdonly_mem() helper inside check_buffer_access() and construct > buf_info, making the code slightly cleaner. > > Signed-off-by: Shung-Hsi Yu <shung-hsi.yu@suse.com> LGTM. Thanks. Acked-by: Yonghong Song <yhs@fb.com>
Hello: This patch was applied to bpf/bpf-next.git (master) by Daniel Borkmann <daniel@iogearbox.net>: On Mon, 7 Mar 2022 13:29:18 +0800 you wrote: > Instead of determining buf_info string in the caller of > check_buffer_access(), we can determine whether the register type is read-only > through type_is_rdonly_mem() helper inside check_buffer_access() and construct > buf_info, making the code slightly cleaner. > > Signed-off-by: Shung-Hsi Yu <shung-hsi.yu@suse.com> > > [...] Here is the summary with links: - [bpf-next] bpf: determine buf_info inside check_buffer_access() https://git.kernel.org/bpf/bpf-next/c/44e9a741cad8 You are awesome, thank you!
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index a39eedecc93a..518238029e46 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4047,9 +4047,9 @@ static int check_buffer_access(struct bpf_verifier_env *env, const struct bpf_reg_state *reg, int regno, int off, int size, bool zero_size_allowed, - const char *buf_info, u32 *max_access) { + const char *buf_info = type_is_rdonly_mem(reg->type) ? "rdonly" : "rdwr"; int err; err = __check_buffer_access(env, buf_info, reg, regno, off, size); @@ -4543,7 +4543,6 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn value_regno); } else if (base_type(reg->type) == PTR_TO_BUF) { bool rdonly_mem = type_is_rdonly_mem(reg->type); - const char *buf_info; u32 *max_access; if (rdonly_mem) { @@ -4552,15 +4551,13 @@ static int check_mem_access(struct bpf_verifier_env *env, int insn_idx, u32 regn regno, reg_type_str(env, reg->type)); return -EACCES; } - buf_info = "rdonly"; max_access = &env->prog->aux->max_rdonly_access; } else { - buf_info = "rdwr"; max_access = &env->prog->aux->max_rdwr_access; } err = check_buffer_access(env, reg, regno, off, size, false, - buf_info, max_access); + max_access); if (!err && value_regno >= 0 && (rdonly_mem || t == BPF_READ)) mark_reg_unknown(env, regs, value_regno); @@ -4823,7 +4820,6 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno, struct bpf_call_arg_meta *meta) { struct bpf_reg_state *regs = cur_regs(env), *reg = ®s[regno]; - const char *buf_info; u32 *max_access; switch (base_type(reg->type)) { @@ -4850,15 +4846,13 @@ static int check_helper_mem_access(struct bpf_verifier_env *env, int regno, if (meta && meta->raw_mode) return -EACCES; - buf_info = "rdonly"; max_access = &env->prog->aux->max_rdonly_access; } else { - buf_info = "rdwr"; max_access = &env->prog->aux->max_rdwr_access; } return check_buffer_access(env, reg, regno, reg->off, access_size, zero_size_allowed, - buf_info, max_access); + max_access); case PTR_TO_STACK: return check_stack_range_initialized( env,
Instead of determining buf_info string in the caller of check_buffer_access(), we can determine whether the register type is read-only through type_is_rdonly_mem() helper inside check_buffer_access() and construct buf_info, making the code slightly cleaner. Signed-off-by: Shung-Hsi Yu <shung-hsi.yu@suse.com> --- Initially I tried to remove the buf_info argument from __check_buffer_access(), however check_tp_buffer_access() uses "tracepoint" (rather than the usual "rdonly"/"rdwr") as it's buf_info, thus I decide to leave __check_buffer_access() as-is, and only change check_buffer_access() instead. kernel/bpf/verifier.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)