Currently find_and_alloc_map() performs two separate functions: some argument sanity checking and partial map creation workflow hanling. Neither of those functions are self-sufficient and are augmented by further checks and initialization logic in the caller (map_create() function). So unify all the sanity checks, permission checks, and creation and initialization logic in one linear piece of code in map_create() instead. This also make it easier to further enhance permission checks and keep them located in one place.
Unified · Split
| @@ -109,37 +109,6 @@ const struct bpf_map_ops bpf_map_offload_ops = { · 108 unmodified lines | |||
| 109 | .map_mem_usage = bpf_map_offload_map_mem_usage, | 109 | .map_mem_usage = bpf_map_offload_map_mem_usage, |
| 110 | }; | 110 | }; |
| 111 | 111 | ||
| 112 | static struct bpf_map *find_and_alloc_map(union bpf_attr *attr) | ||
| 113 | { | ||
| 114 | const struct bpf_map_ops *ops; | ||
| 115 | u32 type = attr->map_type; | ||
| 116 | struct bpf_map *map; | ||
| 117 | int err; | ||
| 118 | |||
| 119 | if (type >= ARRAY_SIZE(bpf_map_types)) | ||
| 120 | return ERR_PTR(-EINVAL); | ||
| 121 | type = array_index_nospec(type, ARRAY_SIZE(bpf_map_types)); | ||
| 122 | ops = bpf_map_types[type]; | ||
| 123 | if (!ops) | ||
| 124 | return ERR_PTR(-EINVAL); | ||
| 125 | |||
| 126 | if (ops->map_alloc_check) { | ||
| 127 | err = ops->map_alloc_check(attr); | ||
| 128 | if (err) | ||
| 129 | return ERR_PTR(err); | ||
| 130 | } | ||
| 131 | if (attr->map_ifindex) | ||
| 132 | ops = &bpf_map_offload_ops; | ||
| 133 | if (!ops->map_mem_usage) | ||
| 134 | return ERR_PTR(-EINVAL); | ||
| 135 | map = ops->map_alloc(attr); | ||
| 136 | if (IS_ERR(map)) | ||
| 137 | return map; | ||
| 138 | map->ops = ops; | ||
| 139 | map->map_type = type; | ||
| 140 | return map; | ||
| 141 | } | ||
| 142 | |||
| 143 | static void bpf_map_write_active_inc(struct bpf_map *map) | 112 | static void bpf_map_write_active_inc(struct bpf_map *map) |
| 144 | { | 113 | { |
| 145 | atomic64_inc(&map->writecnt); | 114 | atomic64_inc(&map->writecnt); |
| @@ -1127,7 +1096,9 @@ free_map_tab: · 981 unmodified lines | |||
| 1127 | /* called via syscall */ | 1096 | /* called via syscall */ |
| 1128 | static int map_create(union bpf_attr *attr) | 1097 | static int map_create(union bpf_attr *attr) |
| 1129 | { | 1098 | { |
| 1099 | const struct bpf_map_ops *ops; | ||
| 1130 | int numa_node = bpf_map_attr_numa_node(attr); | 1100 | int numa_node = bpf_map_attr_numa_node(attr); |
| 1101 | u32 map_type = attr->map_type; | ||
| 1131 | struct bpf_map *map; | 1102 | struct bpf_map *map; |
| 1132 | int f_flags; | 1103 | int f_flags; |
| 1133 | int err; | 1104 | int err; |
| @@ -1157,6 +1128,25 @@ static int map_create(union bpf_attr *attr) · 23 unmodified lines | |||
| 1157 | !node_online(numa_node))) | 1128 | !node_online(numa_node))) |
| 1158 | return -EINVAL; | 1129 | return -EINVAL; |
| 1159 | 1130 | ||
| 1131 | /* find map type and init map: hashtable vs rbtree vs bloom vs ... */ | ||
| 1132 | map_type = attr->map_type; | ||
| 1133 | if (map_type >= ARRAY_SIZE(bpf_map_types)) | ||
| 1134 | return -EINVAL; | ||
| 1135 | map_type = array_index_nospec(map_type, ARRAY_SIZE(bpf_map_types)); | ||
| 1136 | ops = bpf_map_types[map_type]; | ||
| 1137 | if (!ops) | ||
| 1138 | return -EINVAL; | ||
| 1139 | |||
| 1140 | if (ops->map_alloc_check) { | ||
| 1141 | err = ops->map_alloc_check(attr); | ||
| 1142 | if (err) | ||
| 1143 | return err; | ||
| 1144 | } | ||
| 1145 | if (attr->map_ifindex) | ||
| 1146 | ops = &bpf_map_offload_ops; | ||
| 1147 | if (!ops->map_mem_usage) | ||
| 1148 | return -EINVAL; | ||
| 1149 | |||
| 1160 | /* Intent here is for unprivileged_bpf_disabled to block BPF map | 1150 | /* Intent here is for unprivileged_bpf_disabled to block BPF map |
| 1161 | * creation for unprivileged users; other actions depend | 1151 | * creation for unprivileged users; other actions depend |
| 1162 | * on fd availability and access to bpffs, so are dependent on | 1152 | * on fd availability and access to bpffs, so are dependent on |
| @@ -1166,10 +1156,11 @@ static int map_create(union bpf_attr *attr) · 3 unmodified lines | |||
| 1166 | if (sysctl_unprivileged_bpf_disabled && !bpf_capable()) | 1156 | if (sysctl_unprivileged_bpf_disabled && !bpf_capable()) |
| 1167 | return -EPERM; | 1157 | return -EPERM; |
| 1168 | 1158 | ||
| 1169 | /* find map type and init map: hashtable vs rbtree vs bloom vs ... */ | 1159 | map = ops->map_alloc(attr); |
| 1170 | map = find_and_alloc_map(attr); | ||
| 1171 | if (IS_ERR(map)) | 1160 | if (IS_ERR(map)) |
| 1172 | return PTR_ERR(map); | 1161 | return PTR_ERR(map); |
| 1162 | map->ops = ops; | ||
| 1163 | map->map_type = map_type; | ||
| 1173 | 1164 | ||
| 1174 | err = bpf_obj_name_cpy(map->name, attr->map_name, | 1165 | err = bpf_obj_name_cpy(map->name, attr->map_name, |
| 1175 | sizeof(attr->map_name)); | 1166 | sizeof(attr->map_name)); |