Re: [bug report] Potential order bug in 'net/xfrm/xfrm_state.c', primarily in 'xfrm_state_walk_done()'

[Florian Westphal]

Ginger <ginger.jzllee@xxxxxxxxx> wrote:
> Potential concurrent triggering executions:
> T0:
> xfrm_state_walk_done
>     --> kfree(walk->filter); [t0]
>     --> list_del(&walk->all); [t3]

list_del() uses same spinlock as iterator.

> T1:
> xfrm_state_walk

2652 int xfrm_state_walk(struct net *net, struct xfrm_state_walk *walk,
2653                     int (*func)(struct xfrm_state *, int, void*),
2654                     void *data)
2655 {
[..]
2663         spin_lock_bh(&net->xfrm.xfrm_state_lock);
2668         list_for_each_entry_from(x, &net->xfrm.state_all, all) {
2669                 if (x->state == XFRM_STATE_DEAD)
2670                         continue;

... and walker has STATE_DEAD, no? So I don't see how UaF is possible.

Even if parallel invocation (pfkey+netlink?) is possible, then we have:

T0: walk_done() -> free filter -> blocks on spinlock for list_del
T1: list_for_each ... -> walker is valid memory, checks x->state -> SKIP
to next entry

(or list_del already finished, but then _walk() is blocked on
spinlock).