| Message ID | 20211130012948.380602-9-haoluo@google.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Introduce composable bpf types | expand |
On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote: > > + > struct bpf_reg_types { > const enum bpf_reg_type types[10]; > u32 *btf_id; > + > + /* Certain types require customized type matching function. */ > + bool (*type_match_fn)(enum bpf_arg_type arg_type, > + enum bpf_reg_type type, > + enum bpf_reg_type expected); > }; > > static const struct bpf_reg_types map_key_value_types = { > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = { > }; > #endif > > +static bool mem_type_match(enum bpf_arg_type arg_type, > + enum bpf_reg_type type, enum bpf_reg_type expected) > +{ > + /* If arg_type is tagged with MEM_RDONLY, type is compatible with both > + * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before > + * comparison. > + */ > + if ((arg_type & MEM_RDONLY) != 0) > + type &= ~MEM_RDONLY; > + > + return type == expected; > +} > + > static const struct bpf_reg_types mem_types = { > .types = { > PTR_TO_STACK, > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = { > PTR_TO_MAP_VALUE, > PTR_TO_MEM, > PTR_TO_BUF, > - PTR_TO_BUF | MEM_RDONLY, > }, > + .type_match_fn = mem_type_match, why add a callback for this logic? Isn't it a universal rule for MEM_RDONLY?
On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote: > > > > + > > struct bpf_reg_types { > > const enum bpf_reg_type types[10]; > > u32 *btf_id; > > + > > + /* Certain types require customized type matching function. */ > > + bool (*type_match_fn)(enum bpf_arg_type arg_type, > > + enum bpf_reg_type type, > > + enum bpf_reg_type expected); > > }; > > > > static const struct bpf_reg_types map_key_value_types = { > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = { > > }; > > #endif > > > > +static bool mem_type_match(enum bpf_arg_type arg_type, > > + enum bpf_reg_type type, enum bpf_reg_type expected) > > +{ > > + /* If arg_type is tagged with MEM_RDONLY, type is compatible with both > > + * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before > > + * comparison. > > + */ > > + if ((arg_type & MEM_RDONLY) != 0) > > + type &= ~MEM_RDONLY; > > + > > + return type == expected; > > +} > > + > > static const struct bpf_reg_types mem_types = { > > .types = { > > PTR_TO_STACK, > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = { > > PTR_TO_MAP_VALUE, > > PTR_TO_MEM, > > PTR_TO_BUF, > > - PTR_TO_BUF | MEM_RDONLY, > > }, > > + .type_match_fn = mem_type_match, > > why add a callback for this logic? > Isn't it a universal rule for MEM_RDONLY? Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but all flags can be checked in the same way? Like the following static const struct bpf_reg_types int_ptr_types = { @@ -5097,6 +5116,13 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno, if (expected == NOT_INIT) break; + flag = bpf_type_flag(arg_type); - if (type == expected) + if ((type & ~flag) == expected) goto found; }
On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote: > > On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote: > > > > > > + > > > struct bpf_reg_types { > > > const enum bpf_reg_type types[10]; > > > u32 *btf_id; > > > + > > > + /* Certain types require customized type matching function. */ > > > + bool (*type_match_fn)(enum bpf_arg_type arg_type, > > > + enum bpf_reg_type type, > > > + enum bpf_reg_type expected); > > > }; > > > > > > static const struct bpf_reg_types map_key_value_types = { > > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = { > > > }; > > > #endif > > > > > > +static bool mem_type_match(enum bpf_arg_type arg_type, > > > + enum bpf_reg_type type, enum bpf_reg_type expected) > > > +{ > > > + /* If arg_type is tagged with MEM_RDONLY, type is compatible with both > > > + * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before > > > + * comparison. > > > + */ > > > + if ((arg_type & MEM_RDONLY) != 0) > > > + type &= ~MEM_RDONLY; > > > + > > > + return type == expected; > > > +} > > > + > > > static const struct bpf_reg_types mem_types = { > > > .types = { > > > PTR_TO_STACK, > > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = { > > > PTR_TO_MAP_VALUE, > > > PTR_TO_MEM, > > > PTR_TO_BUF, > > > - PTR_TO_BUF | MEM_RDONLY, > > > }, > > > + .type_match_fn = mem_type_match, > > > > why add a callback for this logic? > > Isn't it a universal rule for MEM_RDONLY? > > Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but > all flags can be checked in the same way? Like the following > > static const struct bpf_reg_types int_ptr_types = { > @@ -5097,6 +5116,13 @@ static int check_reg_type(struct > bpf_verifier_env *env, u32 regno, > if (expected == NOT_INIT) > break; > > + flag = bpf_type_flag(arg_type); > > - if (type == expected) > + if ((type & ~flag) == expected) > goto found; > } I think for MAYBE_NULL and MEM_RDONLY that would be correct, but not necessarily apply to future flags. I would open code it for these specific flags. Also what do you think about dropping bpf_ prefix from type_flag()? It won't conflict with anything and less verbose.
On Wed, Dec 1, 2021 at 7:53 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote: > > > > On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote: > > > > > > > > + > > > > struct bpf_reg_types { > > > > const enum bpf_reg_type types[10]; > > > > u32 *btf_id; > > > > + > > > > + /* Certain types require customized type matching function. */ > > > > + bool (*type_match_fn)(enum bpf_arg_type arg_type, > > > > + enum bpf_reg_type type, > > > > + enum bpf_reg_type expected); > > > > }; > > > > > > > > static const struct bpf_reg_types map_key_value_types = { > > > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = { > > > > }; > > > > #endif > > > > > > > > +static bool mem_type_match(enum bpf_arg_type arg_type, > > > > + enum bpf_reg_type type, enum bpf_reg_type expected) > > > > +{ > > > > + /* If arg_type is tagged with MEM_RDONLY, type is compatible with both > > > > + * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before > > > > + * comparison. > > > > + */ > > > > + if ((arg_type & MEM_RDONLY) != 0) > > > > + type &= ~MEM_RDONLY; > > > > + > > > > + return type == expected; > > > > +} > > > > + > > > > static const struct bpf_reg_types mem_types = { > > > > .types = { > > > > PTR_TO_STACK, > > > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = { > > > > PTR_TO_MAP_VALUE, > > > > PTR_TO_MEM, > > > > PTR_TO_BUF, > > > > - PTR_TO_BUF | MEM_RDONLY, > > > > }, > > > > + .type_match_fn = mem_type_match, > > > > > > why add a callback for this logic? > > > Isn't it a universal rule for MEM_RDONLY? > > > > Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but > > all flags can be checked in the same way? Like the following > > > > static const struct bpf_reg_types int_ptr_types = { > > @@ -5097,6 +5116,13 @@ static int check_reg_type(struct > > bpf_verifier_env *env, u32 regno, > > if (expected == NOT_INIT) > > break; > > > > + flag = bpf_type_flag(arg_type); > > > > - if (type == expected) > > + if ((type & ~flag) == expected) > > goto found; > > } > > I think for MAYBE_NULL and MEM_RDONLY that would be correct, > but not necessarily apply to future flags. > I would open code it for these specific flags. ack. > Also what do you think about dropping bpf_ prefix from type_flag()? > It won't conflict with anything and less verbose. Sounds good as long as it won't conflict. IMO it would be good to have an internal header in kernel/bpf. It appears to me that we put everything in linux/bpf.h now. In sched, there is a kernel/sched/sched.h used internally in kernel/sched and a linux/sched.h that is public to other subsystems.
On Thu, Dec 2, 2021 at 10:42 AM Hao Luo <haoluo@google.com> wrote: > > On Wed, Dec 1, 2021 at 7:53 PM Alexei Starovoitov > <alexei.starovoitov@gmail.com> wrote: > > > > On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote: > > > > > > On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov > > > <alexei.starovoitov@gmail.com> wrote: > > > > > > > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote: > > > > > > > > > > + > > > > > struct bpf_reg_types { > > > > > const enum bpf_reg_type types[10]; > > > > > u32 *btf_id; > > > > > + > > > > > + /* Certain types require customized type matching function. */ > > > > > + bool (*type_match_fn)(enum bpf_arg_type arg_type, > > > > > + enum bpf_reg_type type, > > > > > + enum bpf_reg_type expected); > > > > > }; > > > > > > > > > > static const struct bpf_reg_types map_key_value_types = { > > > > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = { > > > > > }; > > > > > #endif > > > > > > > > > > +static bool mem_type_match(enum bpf_arg_type arg_type, > > > > > + enum bpf_reg_type type, enum bpf_reg_type expected) > > > > > +{ > > > > > + /* If arg_type is tagged with MEM_RDONLY, type is compatible with both > > > > > + * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before > > > > > + * comparison. > > > > > + */ > > > > > + if ((arg_type & MEM_RDONLY) != 0) > > > > > + type &= ~MEM_RDONLY; > > > > > + > > > > > + return type == expected; > > > > > +} > > > > > + > > > > > static const struct bpf_reg_types mem_types = { > > > > > .types = { > > > > > PTR_TO_STACK, > > > > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = { > > > > > PTR_TO_MAP_VALUE, > > > > > PTR_TO_MEM, > > > > > PTR_TO_BUF, > > > > > - PTR_TO_BUF | MEM_RDONLY, > > > > > }, > > > > > + .type_match_fn = mem_type_match, > > > > > > > > why add a callback for this logic? > > > > Isn't it a universal rule for MEM_RDONLY? > > > > > > Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but > > > all flags can be checked in the same way? Like the following > > > > > > static const struct bpf_reg_types int_ptr_types = { > > > @@ -5097,6 +5116,13 @@ static int check_reg_type(struct > > > bpf_verifier_env *env, u32 regno, > > > if (expected == NOT_INIT) > > > break; > > > > > > + flag = bpf_type_flag(arg_type); > > > > > > - if (type == expected) > > > + if ((type & ~flag) == expected) > > > goto found; > > > } > > > > I think for MAYBE_NULL and MEM_RDONLY that would be correct, > > but not necessarily apply to future flags. > > I would open code it for these specific flags. > > ack. > > > Also what do you think about dropping bpf_ prefix from type_flag()? > > It won't conflict with anything and less verbose. > > Sounds good as long as it won't conflict. IMO it would be good to have > an internal header in kernel/bpf. It appears to me that we put > everything in linux/bpf.h now. In sched, there is a > kernel/sched/sched.h used internally in kernel/sched and a > linux/sched.h that is public to other subsystems. yeah. That's long overdue. We have include/linux/bpf_verifier.h that might work for this case too. Or even directly in verifier.c (if possible).
On Thu, Dec 2, 2021 at 1:13 PM Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote: > > On Thu, Dec 2, 2021 at 10:42 AM Hao Luo <haoluo@google.com> wrote: > > > > On Wed, Dec 1, 2021 at 7:53 PM Alexei Starovoitov > > <alexei.starovoitov@gmail.com> wrote: > > > > > > On Wed, Dec 1, 2021 at 2:21 PM Hao Luo <haoluo@google.com> wrote: > > > > > > > > On Wed, Dec 1, 2021 at 12:34 PM Alexei Starovoitov > > > > <alexei.starovoitov@gmail.com> wrote: > > > > > > > > > > On Mon, Nov 29, 2021 at 05:29:47PM -0800, Hao Luo wrote: > > > > > > > > > > > > + > > > > > > struct bpf_reg_types { > > > > > > const enum bpf_reg_type types[10]; > > > > > > u32 *btf_id; > > > > > > + > > > > > > + /* Certain types require customized type matching function. */ > > > > > > + bool (*type_match_fn)(enum bpf_arg_type arg_type, > > > > > > + enum bpf_reg_type type, > > > > > > + enum bpf_reg_type expected); > > > > > > }; > > > > > > > > > > > > static const struct bpf_reg_types map_key_value_types = { > > > > > > @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = { > > > > > > }; > > > > > > #endif > > > > > > > > > > > > +static bool mem_type_match(enum bpf_arg_type arg_type, > > > > > > + enum bpf_reg_type type, enum bpf_reg_type expected) > > > > > > +{ > > > > > > + /* If arg_type is tagged with MEM_RDONLY, type is compatible with both > > > > > > + * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before > > > > > > + * comparison. > > > > > > + */ > > > > > > + if ((arg_type & MEM_RDONLY) != 0) > > > > > > + type &= ~MEM_RDONLY; > > > > > > + > > > > > > + return type == expected; > > > > > > +} > > > > > > + > > > > > > static const struct bpf_reg_types mem_types = { > > > > > > .types = { > > > > > > PTR_TO_STACK, > > > > > > @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = { > > > > > > PTR_TO_MAP_VALUE, > > > > > > PTR_TO_MEM, > > > > > > PTR_TO_BUF, > > > > > > - PTR_TO_BUF | MEM_RDONLY, > > > > > > }, > > > > > > + .type_match_fn = mem_type_match, > > > > > > > > > > why add a callback for this logic? > > > > > Isn't it a universal rule for MEM_RDONLY? > > > > > > > > Ah, good point, I didn't realize that. Maybe, not only MEM_RDONLY, but > > > > all flags can be checked in the same way? Like the following > > > > > > > > static const struct bpf_reg_types int_ptr_types = { > > > > @@ -5097,6 +5116,13 @@ static int check_reg_type(struct > > > > bpf_verifier_env *env, u32 regno, > > > > if (expected == NOT_INIT) > > > > break; > > > > > > > > + flag = bpf_type_flag(arg_type); > > > > > > > > - if (type == expected) > > > > + if ((type & ~flag) == expected) > > > > goto found; > > > > } > > > > > > I think for MAYBE_NULL and MEM_RDONLY that would be correct, > > > but not necessarily apply to future flags. > > > I would open code it for these specific flags. > > > > ack. > > > > > Also what do you think about dropping bpf_ prefix from type_flag()? > > > It won't conflict with anything and less verbose. > > > > Sounds good as long as it won't conflict. IMO it would be good to have > > an internal header in kernel/bpf. It appears to me that we put > > everything in linux/bpf.h now. In sched, there is a > > kernel/sched/sched.h used internally in kernel/sched and a > > linux/sched.h that is public to other subsystems. > > yeah. That's long overdue. We have include/linux/bpf_verifier.h > that might work for this case too. > Or even directly in verifier.c (if possible). Ack. Directly in verifier.c would be best, but bpf_verifier.h works. There is only one place outside verifier.c (i.e. btf_ctx_access() in btf.c) that needs base_type, the rest are all in verifier.c. Anyway, I am going to put these functions in bpf_verifer.h for now.
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index 776b90de0971..f0da2f997937 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -6351,7 +6351,7 @@ const struct bpf_func_proto bpf_btf_find_by_name_kind_proto = { .func = bpf_btf_find_by_name_kind, .gpl_only = false, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_MEM, + .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, .arg3_type = ARG_ANYTHING, .arg4_type = ARG_ANYTHING, diff --git a/kernel/bpf/cgroup.c b/kernel/bpf/cgroup.c index 2ca643af9a54..b8fe671af13c 100644 --- a/kernel/bpf/cgroup.c +++ b/kernel/bpf/cgroup.c @@ -1789,7 +1789,7 @@ static const struct bpf_func_proto bpf_sysctl_set_new_value_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, }; diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index a5e349c9d3e3..66b466903a4e 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -530,7 +530,7 @@ const struct bpf_func_proto bpf_strtol_proto = { .func = bpf_strtol, .gpl_only = false, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_MEM, + .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, .arg3_type = ARG_ANYTHING, .arg4_type = ARG_PTR_TO_LONG, @@ -558,7 +558,7 @@ const struct bpf_func_proto bpf_strtoul_proto = { .func = bpf_strtoul, .gpl_only = false, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_MEM, + .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, .arg3_type = ARG_ANYTHING, .arg4_type = ARG_PTR_TO_LONG, @@ -630,7 +630,7 @@ const struct bpf_func_proto bpf_event_output_data_proto = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -1011,7 +1011,7 @@ const struct bpf_func_proto bpf_snprintf_proto = { .arg1_type = ARG_PTR_TO_MEM_OR_NULL, .arg2_type = ARG_CONST_SIZE_OR_ZERO, .arg3_type = ARG_PTR_TO_CONST_STR, - .arg4_type = ARG_PTR_TO_MEM_OR_NULL, + .arg4_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c index 9e0c10c6892a..638d7fd7b375 100644 --- a/kernel/bpf/ringbuf.c +++ b/kernel/bpf/ringbuf.c @@ -444,7 +444,7 @@ const struct bpf_func_proto bpf_ringbuf_output_proto = { .func = bpf_ringbuf_output, .ret_type = RET_INTEGER, .arg1_type = ARG_CONST_MAP_PTR, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE_OR_ZERO, .arg4_type = ARG_ANYTHING, }; diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index 50f96ea4452a..301afb44e47f 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -4757,7 +4757,7 @@ static const struct bpf_func_proto bpf_sys_bpf_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_ANYTHING, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, }; diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index e2663544362a..11ec26c1caa4 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -4976,9 +4976,15 @@ static int resolve_map_arg_type(struct bpf_verifier_env *env, return 0; } + struct bpf_reg_types { const enum bpf_reg_type types[10]; u32 *btf_id; + + /* Certain types require customized type matching function. */ + bool (*type_match_fn)(enum bpf_arg_type arg_type, + enum bpf_reg_type type, + enum bpf_reg_type expected); }; static const struct bpf_reg_types map_key_value_types = { @@ -5013,6 +5019,19 @@ static const struct bpf_reg_types btf_id_sock_common_types = { }; #endif +static bool mem_type_match(enum bpf_arg_type arg_type, + enum bpf_reg_type type, enum bpf_reg_type expected) +{ + /* If arg_type is tagged with MEM_RDONLY, type is compatible with both + * RDONLY and RDWR mem, fold the MEM_RDONLY flag in 'type' before + * comparison. + */ + if ((arg_type & MEM_RDONLY) != 0) + type &= ~MEM_RDONLY; + + return type == expected; +} + static const struct bpf_reg_types mem_types = { .types = { PTR_TO_STACK, @@ -5022,8 +5041,8 @@ static const struct bpf_reg_types mem_types = { PTR_TO_MAP_VALUE, PTR_TO_MEM, PTR_TO_BUF, - PTR_TO_BUF | MEM_RDONLY, }, + .type_match_fn = mem_type_match, }; static const struct bpf_reg_types int_ptr_types = { @@ -5097,6 +5116,13 @@ static int check_reg_type(struct bpf_verifier_env *env, u32 regno, if (expected == NOT_INIT) break; + if (compatible->type_match_fn) { + if (compatible->type_match_fn(arg_type, type, expected)) + goto found; + + continue; + } + if (type == expected) goto found; } diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 25ea521fb8f1..79404049b70f 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -345,7 +345,7 @@ static const struct bpf_func_proto bpf_probe_write_user_proto = { .gpl_only = true, .ret_type = RET_INTEGER, .arg1_type = ARG_ANYTHING, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, }; @@ -394,7 +394,7 @@ static const struct bpf_func_proto bpf_trace_printk_proto = { .func = bpf_trace_printk, .gpl_only = true, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_MEM, + .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, }; @@ -450,9 +450,9 @@ static const struct bpf_func_proto bpf_trace_vprintk_proto = { .func = bpf_trace_vprintk, .gpl_only = true, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_MEM, + .arg1_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE, - .arg3_type = ARG_PTR_TO_MEM_OR_NULL, + .arg3_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -492,9 +492,9 @@ static const struct bpf_func_proto bpf_seq_printf_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_BTF_ID, .arg1_btf_id = &btf_seq_file_ids[0], - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, - .arg4_type = ARG_PTR_TO_MEM_OR_NULL, + .arg4_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -509,7 +509,7 @@ static const struct bpf_func_proto bpf_seq_write_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_BTF_ID, .arg1_btf_id = &btf_seq_file_ids[0], - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -533,7 +533,7 @@ static const struct bpf_func_proto bpf_seq_printf_btf_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_BTF_ID, .arg1_btf_id = &btf_seq_file_ids[0], - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE_OR_ZERO, .arg4_type = ARG_ANYTHING, }; @@ -694,7 +694,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -1004,7 +1004,7 @@ const struct bpf_func_proto bpf_snprintf_btf_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_MEM, .arg2_type = ARG_CONST_SIZE, - .arg3_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE, .arg5_type = ARG_ANYTHING, }; @@ -1289,7 +1289,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto_tp = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -1515,7 +1515,7 @@ static const struct bpf_func_proto bpf_perf_event_output_proto_raw_tp = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -1569,7 +1569,7 @@ static const struct bpf_func_proto bpf_get_stack_proto_raw_tp = { .gpl_only = true, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE_OR_ZERO, .arg4_type = ARG_ANYTHING, }; diff --git a/net/core/filter.c b/net/core/filter.c index 26e0276aa00d..eadca10b436d 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -1713,7 +1713,7 @@ static const struct bpf_func_proto bpf_skb_store_bytes_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE, .arg5_type = ARG_ANYTHING, }; @@ -2018,9 +2018,9 @@ static const struct bpf_func_proto bpf_csum_diff_proto = { .gpl_only = false, .pkt_access = true, .ret_type = RET_INTEGER, - .arg1_type = ARG_PTR_TO_MEM_OR_NULL, + .arg1_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY, .arg2_type = ARG_CONST_SIZE_OR_ZERO, - .arg3_type = ARG_PTR_TO_MEM_OR_NULL, + .arg3_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE_OR_ZERO, .arg5_type = ARG_ANYTHING, }; @@ -2541,7 +2541,7 @@ static const struct bpf_func_proto bpf_redirect_neigh_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_ANYTHING, - .arg2_type = ARG_PTR_TO_MEM_OR_NULL, + .arg2_type = ARG_PTR_TO_MEM | PTR_MAYBE_NULL | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE_OR_ZERO, .arg4_type = ARG_ANYTHING, }; @@ -4174,7 +4174,7 @@ static const struct bpf_func_proto bpf_skb_event_output_proto = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -4188,7 +4188,7 @@ const struct bpf_func_proto bpf_skb_output_proto = { .arg1_btf_id = &bpf_skb_output_btf_ids[0], .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -4371,7 +4371,7 @@ static const struct bpf_func_proto bpf_skb_set_tunnel_key_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, }; @@ -4397,7 +4397,7 @@ static const struct bpf_func_proto bpf_skb_set_tunnel_opt_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, }; @@ -4567,7 +4567,7 @@ static const struct bpf_func_proto bpf_xdp_event_output_proto = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -4581,7 +4581,7 @@ const struct bpf_func_proto bpf_xdp_output_proto = { .arg1_btf_id = &bpf_xdp_output_btf_ids[0], .arg2_type = ARG_CONST_MAP_PTR, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE_OR_ZERO, }; @@ -5067,7 +5067,7 @@ const struct bpf_func_proto bpf_sk_setsockopt_proto = { .arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON, .arg2_type = ARG_ANYTHING, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE, }; @@ -5101,7 +5101,7 @@ static const struct bpf_func_proto bpf_sock_addr_setsockopt_proto = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE, }; @@ -5135,7 +5135,7 @@ static const struct bpf_func_proto bpf_sock_ops_setsockopt_proto = { .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, .arg3_type = ARG_ANYTHING, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE, }; @@ -5310,7 +5310,7 @@ static const struct bpf_func_proto bpf_bind_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, }; @@ -5898,7 +5898,7 @@ static const struct bpf_func_proto bpf_lwt_in_push_encap_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE }; @@ -5908,7 +5908,7 @@ static const struct bpf_func_proto bpf_lwt_xmit_push_encap_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE }; @@ -5951,7 +5951,7 @@ static const struct bpf_func_proto bpf_lwt_seg6_store_bytes_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE }; @@ -6039,7 +6039,7 @@ static const struct bpf_func_proto bpf_lwt_seg6_action_proto = { .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, .arg2_type = ARG_ANYTHING, - .arg3_type = ARG_PTR_TO_MEM, + .arg3_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg4_type = ARG_CONST_SIZE }; @@ -6264,7 +6264,7 @@ static const struct bpf_func_proto bpf_skc_lookup_tcp_proto = { .pkt_access = true, .ret_type = RET_PTR_TO_SOCK_COMMON_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6283,7 +6283,7 @@ static const struct bpf_func_proto bpf_sk_lookup_tcp_proto = { .pkt_access = true, .ret_type = RET_PTR_TO_SOCKET_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6302,7 +6302,7 @@ static const struct bpf_func_proto bpf_sk_lookup_udp_proto = { .pkt_access = true, .ret_type = RET_PTR_TO_SOCKET_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6339,7 +6339,7 @@ static const struct bpf_func_proto bpf_xdp_sk_lookup_udp_proto = { .pkt_access = true, .ret_type = RET_PTR_TO_SOCKET_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6362,7 +6362,7 @@ static const struct bpf_func_proto bpf_xdp_skc_lookup_tcp_proto = { .pkt_access = true, .ret_type = RET_PTR_TO_SOCK_COMMON_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6385,7 +6385,7 @@ static const struct bpf_func_proto bpf_xdp_sk_lookup_tcp_proto = { .pkt_access = true, .ret_type = RET_PTR_TO_SOCKET_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6404,7 +6404,7 @@ static const struct bpf_func_proto bpf_sock_addr_skc_lookup_tcp_proto = { .gpl_only = false, .ret_type = RET_PTR_TO_SOCK_COMMON_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6423,7 +6423,7 @@ static const struct bpf_func_proto bpf_sock_addr_sk_lookup_tcp_proto = { .gpl_only = false, .ret_type = RET_PTR_TO_SOCKET_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6442,7 +6442,7 @@ static const struct bpf_func_proto bpf_sock_addr_sk_lookup_udp_proto = { .gpl_only = false, .ret_type = RET_PTR_TO_SOCKET_OR_NULL, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, .arg5_type = ARG_ANYTHING, @@ -6755,9 +6755,9 @@ static const struct bpf_func_proto bpf_tcp_check_syncookie_proto = { .pkt_access = true, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE, }; @@ -6824,9 +6824,9 @@ static const struct bpf_func_proto bpf_tcp_gen_syncookie_proto = { .pkt_access = true, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_BTF_ID_SOCK_COMMON, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, - .arg4_type = ARG_PTR_TO_MEM, + .arg4_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg5_type = ARG_CONST_SIZE, }; @@ -7055,7 +7055,7 @@ static const struct bpf_func_proto bpf_sock_ops_store_hdr_opt_proto = { .gpl_only = false, .ret_type = RET_INTEGER, .arg1_type = ARG_PTR_TO_CTX, - .arg2_type = ARG_PTR_TO_MEM, + .arg2_type = ARG_PTR_TO_MEM | MEM_RDONLY, .arg3_type = ARG_CONST_SIZE, .arg4_type = ARG_ANYTHING, };
Some helper functions may modify its arguments, for example, bpf_d_path, bpf_get_stack etc. Previously, their argument types were marked as ARG_PTR_TO_MEM, which is compatible with read-only mem types, such as PTR_TO_RDONLY_BUF. Therefore it's legitimate to modify a read-only memory by passing it into one of such helper functions. This patch tags the args that point to immutable memory with MEM_RDONLY flag. The arguments that don't have this flag will be only compatible with mutable memory types. The arguments that have MEM_RDONLY flag are compatible with both mutable memory and immutable memory. Signed-off-by: Hao Luo <haoluo@google.com> --- kernel/bpf/btf.c | 2 +- kernel/bpf/cgroup.c | 2 +- kernel/bpf/helpers.c | 8 ++--- kernel/bpf/ringbuf.c | 2 +- kernel/bpf/syscall.c | 2 +- kernel/bpf/verifier.c | 28 +++++++++++++++++- kernel/trace/bpf_trace.c | 26 ++++++++-------- net/core/filter.c | 64 ++++++++++++++++++++-------------------- 8 files changed, 80 insertions(+), 54 deletions(-)