| Message ID | 0ff8927166f6e18e72adab8a94cb6d694c610cc0.1607973529.git.me@ubique.spb.ru (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | BPF |
| Headers | show |
| Series | Add support of pointer to struct in global functions | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 17 this patch: 17 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: line length of 82 exceeds 80 columns |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 17 this patch: 17 |
| netdev/header_inline | success | Link |
| netdev/stable | success | Stable not CCed |
On Mon, Dec 14, 2020 at 11:53 AM Dmitrii Banshchikov <me@ubique.spb.ru> wrote: > > Factor out helper function for conversion nullable register type to its > corresponding type with value. > > Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru> > --- > kernel/bpf/verifier.c | 77 ++++++++++++++++++++++++------------------- > 1 file changed, 44 insertions(+), 33 deletions(-) > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > index 93def76cf32b..dee296dbc7a1 100644 > --- a/kernel/bpf/verifier.c > +++ b/kernel/bpf/verifier.c > @@ -1073,6 +1073,43 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env, > __mark_reg_known_zero(regs + regno); > } [...] > - if (is_null) { > /* We don't need id and ref_obj_id from this point > * onwards anymore, thus we should better reset it, > * so that state pruning has chances to take effect. > */ > reg->id = 0; > reg->ref_obj_id = 0; nit: I'd just return here and reduce further nesting of the else branch. > - } else if (!reg_may_point_to_spin_lock(reg)) { > - /* For not-NULL ptr, reg->ref_obj_id will be reset > + } else { > + mark_ptr_not_null_reg(reg); Now that this can return -EINVAL, I think some WARN or error message is due. > + > + if (!reg_may_point_to_spin_lock(reg)) { > + /* For not-NULL ptr, reg->ref_obj_id will be reset > * in release_reg_references(). > * > * reg->id is still used by spin_lock ptr. Other > * than spin_lock ptr type, reg->id can be reset. > */ > - reg->id = 0; > + reg->id = 0; > + } > } > } > } > -- > 2.25.1 >
On Mon, Dec 14, 2020 at 11:52:48PM +0400, Dmitrii Banshchikov wrote: > + } else if (reg->type == PTR_TO_SOCKET_OR_NULL) { > + reg->type = PTR_TO_SOCKET; > + } else if (reg->type == PTR_TO_SOCK_COMMON_OR_NULL) { > + reg->type = PTR_TO_SOCK_COMMON; > + } else if (reg->type == PTR_TO_TCP_SOCK_OR_NULL) { > + reg->type = PTR_TO_TCP_SOCK; > + } else if (reg->type == PTR_TO_BTF_ID_OR_NULL) { > + reg->type = PTR_TO_BTF_ID; > + } else if (reg->type == PTR_TO_MEM_OR_NULL) { > + reg->type = PTR_TO_MEM; > + } else if (reg->type == PTR_TO_RDONLY_BUF_OR_NULL) { > + reg->type = PTR_TO_RDONLY_BUF; > + } else if (reg->type == PTR_TO_RDWR_BUF_OR_NULL) { > + reg->type = PTR_TO_RDWR_BUF; > + } else { > + return -EINVAL; In other places we've converted such sequences of if-s into switch statements. Probably makes sense to do it here as well.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 93def76cf32b..dee296dbc7a1 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -1073,6 +1073,43 @@ static void mark_reg_known_zero(struct bpf_verifier_env *env, __mark_reg_known_zero(regs + regno); } +static int mark_ptr_not_null_reg(struct bpf_reg_state *reg) +{ + if (reg->type == PTR_TO_MAP_VALUE_OR_NULL) { + const struct bpf_map *map = reg->map_ptr; + + if (map->inner_map_meta) { + reg->type = CONST_PTR_TO_MAP; + reg->map_ptr = map->inner_map_meta; + } else if (map->map_type == BPF_MAP_TYPE_XSKMAP) { + reg->type = PTR_TO_XDP_SOCK; + } else if (map->map_type == BPF_MAP_TYPE_SOCKMAP || + map->map_type == BPF_MAP_TYPE_SOCKHASH) { + reg->type = PTR_TO_SOCKET; + } else { + reg->type = PTR_TO_MAP_VALUE; + } + } else if (reg->type == PTR_TO_SOCKET_OR_NULL) { + reg->type = PTR_TO_SOCKET; + } else if (reg->type == PTR_TO_SOCK_COMMON_OR_NULL) { + reg->type = PTR_TO_SOCK_COMMON; + } else if (reg->type == PTR_TO_TCP_SOCK_OR_NULL) { + reg->type = PTR_TO_TCP_SOCK; + } else if (reg->type == PTR_TO_BTF_ID_OR_NULL) { + reg->type = PTR_TO_BTF_ID; + } else if (reg->type == PTR_TO_MEM_OR_NULL) { + reg->type = PTR_TO_MEM; + } else if (reg->type == PTR_TO_RDONLY_BUF_OR_NULL) { + reg->type = PTR_TO_RDONLY_BUF; + } else if (reg->type == PTR_TO_RDWR_BUF_OR_NULL) { + reg->type = PTR_TO_RDWR_BUF; + } else { + return -EINVAL; + } + + return 0; +} + static bool reg_is_pkt_pointer(const struct bpf_reg_state *reg) { return type_is_pkt_pointer(reg->type); @@ -7323,50 +7360,24 @@ static void mark_ptr_or_null_reg(struct bpf_func_state *state, } if (is_null) { reg->type = SCALAR_VALUE; - } else if (reg->type == PTR_TO_MAP_VALUE_OR_NULL) { - const struct bpf_map *map = reg->map_ptr; - - if (map->inner_map_meta) { - reg->type = CONST_PTR_TO_MAP; - reg->map_ptr = map->inner_map_meta; - } else if (map->map_type == BPF_MAP_TYPE_XSKMAP) { - reg->type = PTR_TO_XDP_SOCK; - } else if (map->map_type == BPF_MAP_TYPE_SOCKMAP || - map->map_type == BPF_MAP_TYPE_SOCKHASH) { - reg->type = PTR_TO_SOCKET; - } else { - reg->type = PTR_TO_MAP_VALUE; - } - } else if (reg->type == PTR_TO_SOCKET_OR_NULL) { - reg->type = PTR_TO_SOCKET; - } else if (reg->type == PTR_TO_SOCK_COMMON_OR_NULL) { - reg->type = PTR_TO_SOCK_COMMON; - } else if (reg->type == PTR_TO_TCP_SOCK_OR_NULL) { - reg->type = PTR_TO_TCP_SOCK; - } else if (reg->type == PTR_TO_BTF_ID_OR_NULL) { - reg->type = PTR_TO_BTF_ID; - } else if (reg->type == PTR_TO_MEM_OR_NULL) { - reg->type = PTR_TO_MEM; - } else if (reg->type == PTR_TO_RDONLY_BUF_OR_NULL) { - reg->type = PTR_TO_RDONLY_BUF; - } else if (reg->type == PTR_TO_RDWR_BUF_OR_NULL) { - reg->type = PTR_TO_RDWR_BUF; - } - if (is_null) { /* We don't need id and ref_obj_id from this point * onwards anymore, thus we should better reset it, * so that state pruning has chances to take effect. */ reg->id = 0; reg->ref_obj_id = 0; - } else if (!reg_may_point_to_spin_lock(reg)) { - /* For not-NULL ptr, reg->ref_obj_id will be reset + } else { + mark_ptr_not_null_reg(reg); + + if (!reg_may_point_to_spin_lock(reg)) { + /* For not-NULL ptr, reg->ref_obj_id will be reset * in release_reg_references(). * * reg->id is still used by spin_lock ptr. Other * than spin_lock ptr type, reg->id can be reset. */ - reg->id = 0; + reg->id = 0; + } } } }
Factor out helper function for conversion nullable register type to its corresponding type with value. Signed-off-by: Dmitrii Banshchikov <me@ubique.spb.ru> --- kernel/bpf/verifier.c | 77 ++++++++++++++++++++++++------------------- 1 file changed, 44 insertions(+), 33 deletions(-)