Skip to content

Commit 02adf9e

Browse files
author
Alexei Starovoitov
committed
Merge branch 'error checking where helpers call bpf_map_ops'
JP Kobryn says: ==================== Within bpf programs, the bpf helper functions can make inline calls to kernel functions. In this scenario there can be a disconnect between the register the kernel function writes a return value to and the register the bpf program uses to evaluate that return value. As an example, this bpf code: long err = bpf_map_update_elem(...); if (err && err != -EEXIST) // got some error other than -EEXIST ...can result in the bpf assembly: ; err = bpf_map_update_elem(&mymap, &key, &val, BPF_NOEXIST); 37: movabs $0xffff976a10730400,%rdi 41: mov $0x1,%ecx 46: call 0xffffffffe103291c ; htab_map_update_elem ; if (err && err != -EEXIST) { 4b: cmp $0xffffffffffffffef,%rax ; cmp -EEXIST,%rax 4f: je 0x000000000000008e 51: test %rax,%rax 54: je 0x000000000000008e The compare operation here evaluates %rax, while in the preceding call to htab_map_update_elem the corresponding assembly returns -EEXIST via %eax (the lower 32 bits of %rax): movl $0xffffffef, %r9d ... movl %r9d, %eax ...since it's returning int (32-bit). So the resulting comparison becomes: cmp $0xffffffffffffffef, $0x00000000ffffffef ...making it not possible to check for negative errors or specific errors, since the sign value is left at the 32nd bit. It means in the original example, the conditional branch will be entered even when the error is -EEXIST, which was not intended. The selftests added cover these cases for the different bpf_map_ops functions. When the second patch is applied, changing the return type of those functions to long, the comparison works as intended and the tests pass. ==================== Signed-off-by: Alexei Starovoitov <[email protected]>
2 parents d9d93f3 + d7ba4cc commit 02adf9e

23 files changed

+410
-110
lines changed

‎include/linux/bpf.h

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -96,11 +96,11 @@ struct bpf_map_ops {
9696

9797
/* funcs callable from userspace and from eBPF programs */
9898
void *(*map_lookup_elem)(struct bpf_map *map, void *key);
99-
int (*map_update_elem)(struct bpf_map *map, void *key, void *value, u64 flags);
100-
int (*map_delete_elem)(struct bpf_map *map, void *key);
101-
int (*map_push_elem)(struct bpf_map *map, void *value, u64 flags);
102-
int (*map_pop_elem)(struct bpf_map *map, void *value);
103-
int (*map_peek_elem)(struct bpf_map *map, void *value);
99+
long (*map_update_elem)(struct bpf_map *map, void *key, void *value, u64 flags);
100+
long (*map_delete_elem)(struct bpf_map *map, void *key);
101+
long (*map_push_elem)(struct bpf_map *map, void *value, u64 flags);
102+
long (*map_pop_elem)(struct bpf_map *map, void *value);
103+
long (*map_peek_elem)(struct bpf_map *map, void *value);
104104
void *(*map_lookup_percpu_elem)(struct bpf_map *map, void *key, u32 cpu);
105105

106106
/* funcs called by prog_array and perf_event_array map */
@@ -139,7 +139,7 @@ struct bpf_map_ops {
139139
struct bpf_local_storage __rcu ** (*map_owner_storage_ptr)(void *owner);
140140

141141
/* Misc helpers.*/
142-
int (*map_redirect)(struct bpf_map *map, u64 key, u64 flags);
142+
long (*map_redirect)(struct bpf_map *map, u64 key, u64 flags);
143143

144144
/* map_meta_equal must be implemented for maps that can be
145145
* used as an inner map. It is a runtime check to ensure
@@ -157,7 +157,7 @@ struct bpf_map_ops {
157157
int (*map_set_for_each_callback_args)(struct bpf_verifier_env *env,
158158
struct bpf_func_state *caller,
159159
struct bpf_func_state *callee);
160-
int (*map_for_each_callback)(struct bpf_map *map,
160+
long (*map_for_each_callback)(struct bpf_map *map,
161161
bpf_callback_t callback_fn,
162162
void *callback_ctx, u64 flags);
163163

‎include/linux/filter.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1504,9 +1504,9 @@ static inline bool bpf_sk_lookup_run_v6(struct net *net, int protocol,
15041504
}
15051505
#endif /* IS_ENABLED(CONFIG_IPV6) */
15061506

1507-
static __always_inline int __bpf_xdp_redirect_map(struct bpf_map *map, u64 index,
1508-
u64 flags, const u64 flag_mask,
1509-
void *lookup_elem(struct bpf_map *map, u32 key))
1507+
static __always_inline long __bpf_xdp_redirect_map(struct bpf_map *map, u64 index,
1508+
u64 flags, const u64 flag_mask,
1509+
void *lookup_elem(struct bpf_map *map, u32 key))
15101510
{
15111511
struct bpf_redirect_info *ri = this_cpu_ptr(&bpf_redirect_info);
15121512
const u64 action_mask = XDP_ABORTED | XDP_DROP | XDP_PASS | XDP_TX;

‎kernel/bpf/arraymap.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,8 @@ static int array_map_get_next_key(struct bpf_map *map, void *key, void *next_key
307307
}
308308

309309
/* Called from syscall or from eBPF program */
310-
static int array_map_update_elem(struct bpf_map *map, void *key, void *value,
311-
u64 map_flags)
310+
static long array_map_update_elem(struct bpf_map *map, void *key, void *value,
311+
u64 map_flags)
312312
{
313313
struct bpf_array *array = container_of(map, struct bpf_array, map);
314314
u32 index = *(u32 *)key;
@@ -386,7 +386,7 @@ int bpf_percpu_array_update(struct bpf_map *map, void *key, void *value,
386386
}
387387

388388
/* Called from syscall or from eBPF program */
389-
static int array_map_delete_elem(struct bpf_map *map, void *key)
389+
static long array_map_delete_elem(struct bpf_map *map, void *key)
390390
{
391391
return -EINVAL;
392392
}
@@ -686,8 +686,8 @@ static const struct bpf_iter_seq_info iter_seq_info = {
686686
.seq_priv_size = sizeof(struct bpf_iter_seq_array_map_info),
687687
};
688688

689-
static int bpf_for_each_array_elem(struct bpf_map *map, bpf_callback_t callback_fn,
690-
void *callback_ctx, u64 flags)
689+
static long bpf_for_each_array_elem(struct bpf_map *map, bpf_callback_t callback_fn,
690+
void *callback_ctx, u64 flags)
691691
{
692692
u32 i, key, num_elems = 0;
693693
struct bpf_array *array;
@@ -871,7 +871,7 @@ int bpf_fd_array_map_update_elem(struct bpf_map *map, struct file *map_file,
871871
return 0;
872872
}
873873

874-
static int fd_array_map_delete_elem(struct bpf_map *map, void *key)
874+
static long fd_array_map_delete_elem(struct bpf_map *map, void *key)
875875
{
876876
struct bpf_array *array = container_of(map, struct bpf_array, map);
877877
void *old_ptr;

‎kernel/bpf/bloom_filter.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ static u32 hash(struct bpf_bloom_filter *bloom, void *value,
4141
return h & bloom->bitset_mask;
4242
}
4343

44-
static int bloom_map_peek_elem(struct bpf_map *map, void *value)
44+
static long bloom_map_peek_elem(struct bpf_map *map, void *value)
4545
{
4646
struct bpf_bloom_filter *bloom =
4747
container_of(map, struct bpf_bloom_filter, map);
@@ -56,7 +56,7 @@ static int bloom_map_peek_elem(struct bpf_map *map, void *value)
5656
return 0;
5757
}
5858

59-
static int bloom_map_push_elem(struct bpf_map *map, void *value, u64 flags)
59+
static long bloom_map_push_elem(struct bpf_map *map, void *value, u64 flags)
6060
{
6161
struct bpf_bloom_filter *bloom =
6262
container_of(map, struct bpf_bloom_filter, map);
@@ -73,12 +73,12 @@ static int bloom_map_push_elem(struct bpf_map *map, void *value, u64 flags)
7373
return 0;
7474
}
7575

76-
static int bloom_map_pop_elem(struct bpf_map *map, void *value)
76+
static long bloom_map_pop_elem(struct bpf_map *map, void *value)
7777
{
7878
return -EOPNOTSUPP;
7979
}
8080

81-
static int bloom_map_delete_elem(struct bpf_map *map, void *value)
81+
static long bloom_map_delete_elem(struct bpf_map *map, void *value)
8282
{
8383
return -EOPNOTSUPP;
8484
}
@@ -177,8 +177,8 @@ static void *bloom_map_lookup_elem(struct bpf_map *map, void *key)
177177
return ERR_PTR(-EINVAL);
178178
}
179179

180-
static int bloom_map_update_elem(struct bpf_map *map, void *key,
181-
void *value, u64 flags)
180+
static long bloom_map_update_elem(struct bpf_map *map, void *key,
181+
void *value, u64 flags)
182182
{
183183
/* The eBPF program should use map_push_elem instead */
184184
return -EINVAL;

‎kernel/bpf/bpf_cgrp_storage.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,8 +93,8 @@ static void *bpf_cgrp_storage_lookup_elem(struct bpf_map *map, void *key)
9393
return sdata ? sdata->data : NULL;
9494
}
9595

96-
static int bpf_cgrp_storage_update_elem(struct bpf_map *map, void *key,
97-
void *value, u64 map_flags)
96+
static long bpf_cgrp_storage_update_elem(struct bpf_map *map, void *key,
97+
void *value, u64 map_flags)
9898
{
9999
struct bpf_local_storage_data *sdata;
100100
struct cgroup *cgroup;
@@ -125,7 +125,7 @@ static int cgroup_storage_delete(struct cgroup *cgroup, struct bpf_map *map)
125125
return 0;
126126
}
127127

128-
static int bpf_cgrp_storage_delete_elem(struct bpf_map *map, void *key)
128+
static long bpf_cgrp_storage_delete_elem(struct bpf_map *map, void *key)
129129
{
130130
struct cgroup *cgroup;
131131
int err, fd;

‎kernel/bpf/bpf_inode_storage.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,8 @@ static void *bpf_fd_inode_storage_lookup_elem(struct bpf_map *map, void *key)
9191
return sdata ? sdata->data : NULL;
9292
}
9393

94-
static int bpf_fd_inode_storage_update_elem(struct bpf_map *map, void *key,
95-
void *value, u64 map_flags)
94+
static long bpf_fd_inode_storage_update_elem(struct bpf_map *map, void *key,
95+
void *value, u64 map_flags)
9696
{
9797
struct bpf_local_storage_data *sdata;
9898
struct file *f;
@@ -127,7 +127,7 @@ static int inode_storage_delete(struct inode *inode, struct bpf_map *map)
127127
return 0;
128128
}
129129

130-
static int bpf_fd_inode_storage_delete_elem(struct bpf_map *map, void *key)
130+
static long bpf_fd_inode_storage_delete_elem(struct bpf_map *map, void *key)
131131
{
132132
struct file *f;
133133
int fd, err;

‎kernel/bpf/bpf_struct_ops.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -349,8 +349,8 @@ int bpf_struct_ops_prepare_trampoline(struct bpf_tramp_links *tlinks,
349349
model, flags, tlinks, NULL);
350350
}
351351

352-
static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
353-
void *value, u64 flags)
352+
static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
353+
void *value, u64 flags)
354354
{
355355
struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
356356
const struct bpf_struct_ops *st_ops = st_map->st_ops;
@@ -524,7 +524,7 @@ static int bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
524524
return err;
525525
}
526526

527-
static int bpf_struct_ops_map_delete_elem(struct bpf_map *map, void *key)
527+
static long bpf_struct_ops_map_delete_elem(struct bpf_map *map, void *key)
528528
{
529529
enum bpf_struct_ops_state prev_state;
530530
struct bpf_struct_ops_map *st_map;

‎kernel/bpf/bpf_task_storage.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,8 +120,8 @@ static void *bpf_pid_task_storage_lookup_elem(struct bpf_map *map, void *key)
120120
return ERR_PTR(err);
121121
}
122122

123-
static int bpf_pid_task_storage_update_elem(struct bpf_map *map, void *key,
124-
void *value, u64 map_flags)
123+
static long bpf_pid_task_storage_update_elem(struct bpf_map *map, void *key,
124+
void *value, u64 map_flags)
125125
{
126126
struct bpf_local_storage_data *sdata;
127127
struct task_struct *task;
@@ -173,7 +173,7 @@ static int task_storage_delete(struct task_struct *task, struct bpf_map *map,
173173
return 0;
174174
}
175175

176-
static int bpf_pid_task_storage_delete_elem(struct bpf_map *map, void *key)
176+
static long bpf_pid_task_storage_delete_elem(struct bpf_map *map, void *key)
177177
{
178178
struct task_struct *task;
179179
unsigned int f_flags;

‎kernel/bpf/cpumap.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ static void __cpu_map_entry_replace(struct bpf_cpu_map *cmap,
540540
}
541541
}
542542

543-
static int cpu_map_delete_elem(struct bpf_map *map, void *key)
543+
static long cpu_map_delete_elem(struct bpf_map *map, void *key)
544544
{
545545
struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
546546
u32 key_cpu = *(u32 *)key;
@@ -553,8 +553,8 @@ static int cpu_map_delete_elem(struct bpf_map *map, void *key)
553553
return 0;
554554
}
555555

556-
static int cpu_map_update_elem(struct bpf_map *map, void *key, void *value,
557-
u64 map_flags)
556+
static long cpu_map_update_elem(struct bpf_map *map, void *key, void *value,
557+
u64 map_flags)
558558
{
559559
struct bpf_cpu_map *cmap = container_of(map, struct bpf_cpu_map, map);
560560
struct bpf_cpumap_val cpumap_value = {};
@@ -667,7 +667,7 @@ static int cpu_map_get_next_key(struct bpf_map *map, void *key, void *next_key)
667667
return 0;
668668
}
669669

670-
static int cpu_map_redirect(struct bpf_map *map, u64 index, u64 flags)
670+
static long cpu_map_redirect(struct bpf_map *map, u64 index, u64 flags)
671671
{
672672
return __bpf_xdp_redirect_map(map, index, flags, 0,
673673
__cpu_map_lookup_elem);

‎kernel/bpf/devmap.c

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -809,7 +809,7 @@ static void __dev_map_entry_free(struct rcu_head *rcu)
809809
kfree(dev);
810810
}
811811

812-
static int dev_map_delete_elem(struct bpf_map *map, void *key)
812+
static long dev_map_delete_elem(struct bpf_map *map, void *key)
813813
{
814814
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
815815
struct bpf_dtab_netdev *old_dev;
@@ -826,7 +826,7 @@ static int dev_map_delete_elem(struct bpf_map *map, void *key)
826826
return 0;
827827
}
828828

829-
static int dev_map_hash_delete_elem(struct bpf_map *map, void *key)
829+
static long dev_map_hash_delete_elem(struct bpf_map *map, void *key)
830830
{
831831
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
832832
struct bpf_dtab_netdev *old_dev;
@@ -897,8 +897,8 @@ static struct bpf_dtab_netdev *__dev_map_alloc_node(struct net *net,
897897
return ERR_PTR(-EINVAL);
898898
}
899899

900-
static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
901-
void *key, void *value, u64 map_flags)
900+
static long __dev_map_update_elem(struct net *net, struct bpf_map *map,
901+
void *key, void *value, u64 map_flags)
902902
{
903903
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
904904
struct bpf_dtab_netdev *dev, *old_dev;
@@ -939,15 +939,15 @@ static int __dev_map_update_elem(struct net *net, struct bpf_map *map,
939939
return 0;
940940
}
941941

942-
static int dev_map_update_elem(struct bpf_map *map, void *key, void *value,
943-
u64 map_flags)
942+
static long dev_map_update_elem(struct bpf_map *map, void *key, void *value,
943+
u64 map_flags)
944944
{
945945
return __dev_map_update_elem(current->nsproxy->net_ns,
946946
map, key, value, map_flags);
947947
}
948948

949-
static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
950-
void *key, void *value, u64 map_flags)
949+
static long __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
950+
void *key, void *value, u64 map_flags)
951951
{
952952
struct bpf_dtab *dtab = container_of(map, struct bpf_dtab, map);
953953
struct bpf_dtab_netdev *dev, *old_dev;
@@ -999,21 +999,21 @@ static int __dev_map_hash_update_elem(struct net *net, struct bpf_map *map,
999999
return err;
10001000
}
10011001

1002-
static int dev_map_hash_update_elem(struct bpf_map *map, void *key, void *value,
1003-
u64 map_flags)
1002+
static long dev_map_hash_update_elem(struct bpf_map *map, void *key, void *value,
1003+
u64 map_flags)
10041004
{
10051005
return __dev_map_hash_update_elem(current->nsproxy->net_ns,
10061006
map, key, value, map_flags);
10071007
}
10081008

1009-
static int dev_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
1009+
static long dev_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
10101010
{
10111011
return __bpf_xdp_redirect_map(map, ifindex, flags,
10121012
BPF_F_BROADCAST | BPF_F_EXCLUDE_INGRESS,
10131013
__dev_map_lookup_elem);
10141014
}
10151015

1016-
static int dev_hash_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
1016+
static long dev_hash_map_redirect(struct bpf_map *map, u64 ifindex, u64 flags)
10171017
{
10181018
return __bpf_xdp_redirect_map(map, ifindex, flags,
10191019
BPF_F_BROADCAST | BPF_F_EXCLUDE_INGRESS,

0 commit comments

Comments
 (0)