On 2019-08-15 01:17, Daniel Borkmann wrote:
Seems reasonable to me, and inc as opposed to inc_not_zero is also fine here since at this point in time we're still holding one reference to the map.
Ok, good.
But I think there's a catch with the current code that still needs fixing: Imagine you do a xsk_map_update_elem() where we have a situation where xs == old_xs. There, we first do the xsk_map_sock_add() to add the new xsk map node at the tail of the socket's xs->map_list. We do the xchg() and then xsk_map_sock_delete() for old_xs which then walks xs->map_list again and purges all entries including the just newly created one. This means we'll end up with an xs socket at the given map slot, but the xs socket has empty xs->map_list. This means we could release the xs sock and the xsk_delete_from_maps() won't need to clean up anything anymore but yet the xs is still in the map slot, so if you redirect to that socket, it would be use-after-free, no?
Ah, correct. Checking against self-assignment, or doing the delete prior add. I'll spin a v5.
...and again, thanks for detailed review, Daniel! Björn
