On Mon, 2026-05-18 at 11:02 +0800, Kaitao Cheng wrote:
[...]
> > > > The patch does have a bug, however. To fix the issues we are seeing now,
> > > > I propose the additional changes below and would appreciate feedback.
> > > >
> > > > --- a/kernel/bpf/helpers.c
> > > > +++ b/kernel/bpf/helpers.c
> > > > @@ -2263,8 +2263,10 @@ void bpf_list_head_free(const struct btf_field
> > > > *field, void *list_head,
> > > > if (!head->next || list_empty(head))
> > > > goto unlock;
> > > > list_for_each_safe(pos, n, head) {
> > > > - WRITE_ONCE(container_of(pos,
> > > > - struct bpf_list_node_kern, list_head)->owner,
> > > > NULL);
> > > > + struct bpf_list_node_kern *node;
> > > > +
> > > > + node = container_of(pos, struct bpf_list_node_kern,
> > > > list_head);
> > > > + WRITE_ONCE(node->owner, BPF_PTR_POISON);
> > > > list_move_tail(pos, &drain);
> > > > }
> > > > unlock:
> > > > @@ -2272,8 +2274,12 @@ void bpf_list_head_free(const struct btf_field
> > > > *field, void *list_head,
> > > > __bpf_spin_unlock_irqrestore(spin_lock);
> > > >
> > > > while (!list_empty(&drain)) {
> > > > + struct bpf_list_node_kern *node;
> > > > +
> > > > pos = drain.next;
> > > > + node = container_of(pos, struct bpf_list_node_kern,
> > > > list_head);
> > > > list_del_init(pos);
> > > > + WRITE_ONCE(node->owner, NULL);
Is CPU allowed to reorder the stores in list_del_init() and WRITE_ONCE()?
If it is, I think there is a race here.
Thread #1:
enter bpf_list_head_free()
acquire H1 lock
list_move_tail(pos, &drain); // reordered
<-- ip here -->
WRITE_ONCE(node->owner, BPF_PTR_POISON); // reordered
Thread #2:
acquire H1 lock
n = bpf_refcount_acquire()
release H1 lock
acquire H2 lock
enter __bpf_list_add()
<-- ip here -->
cmpxchg(&node->owner, NULL, BPF_PTR_POISON)
[...]