On Mon, Apr 27, 2020 at 10:49:36PM -0700, Andrii Nakryiko wrote:
> +int bpf_link_settle(struct bpf_link_primer *primer)
> +{
> + /* make bpf_link fetchable by ID */
> + WRITE_ONCE(primer->link->id, primer->id);
what does WRITE_ONCE serve here?
bpf_link_settle can only be called at the end of attach.
If attach is slow than parallel get_fd_by_id can get an new FD
instance for link with zero id.
In such case deref of link->id will race with above assignment?
But I don't see READ_ONCE in patch 3.
It's under link_idr_lock there.
How about grabbing link_idr_lock here as well ?
otherwise it's still racy since WRITE_ONCE is not paired.
The mix of spin_lock_irqsave(&link_idr_lock)
and spin_lock_bh(&link_idr_lock) looks weird.
We do the same for map_idr because maps have complicated freeing logic,
but prog_idr is consistent.
If you see the need for irqsave variant then please use it in all cases.
