On Wed, Jun 09, 2021 at 12:33:13PM +0200, Toke Høiland-Jørgensen wrote:
[ ... ]
> @@ -551,7 +551,8 @@ static void cpu_map_free(struct bpf_map *map)
> for (i = 0; i < cmap->map.max_entries; i++) {
> struct bpf_cpu_map_entry *rcpu;
>
> - rcpu = READ_ONCE(cmap->cpu_map[i]);
> + rcpu = rcu_dereference_check(cmap->cpu_map[i],
> + rcu_read_lock_bh_held());
Is rcu_read_lock_bh_held() true during map_free()?
[ ... ]
> @@ -149,7 +152,8 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> u64 map_flags)
> {
> struct xsk_map *m = container_of(map, struct xsk_map, map);
> - struct xdp_sock *xs, *old_xs, **map_entry;
> + struct xdp_sock __rcu **map_entry;
> + struct xdp_sock *xs, *old_xs;
> u32 i = *(u32 *)key, fd = *(u32 *)value;
> struct xsk_map_node *node;
> struct socket *sock;
> @@ -179,7 +183,7 @@ static int xsk_map_update_elem(struct bpf_map *map, void *key, void *value,
> }
>
> spin_lock_bh(&m->lock);
> - old_xs = READ_ONCE(*map_entry);
> + old_xs = rcu_dereference_check(*map_entry, rcu_read_lock_bh_held());
Is it actually protected by the m->lock at this point?
[ ... ]
> void xsk_map_try_sock_delete(struct xsk_map *map, struct xdp_sock *xs,
> - struct xdp_sock **map_entry)
> + struct xdp_sock __rcu **map_entry)
> {
> spin_lock_bh(&map->lock);
> - if (READ_ONCE(*map_entry) == xs) {
> - WRITE_ONCE(*map_entry, NULL);
> + if (rcu_dereference(*map_entry) == xs) {
nit. rcu_access_pointer()?
> + rcu_assign_pointer(*map_entry, NULL);
> xsk_map_sock_delete(xs, map_entry);
> }
> spin_unlock_bh(&map->lock);
> --
> 2.31.1
>
