AZL-97244

In the Linux kernel, the following vulnerability has been resolved:

net/sched: clsapi: Always acquire rtnllock when destroying locked classifiers

Another challenge with unlocked filters. There is a short window in tcnewtfilter where a tcf_proto can be found and briefly referenced by a totally unrelated, unlocked classifier's request and cause a race.

Feng created a poc which created this race with two threads, one creating a u32 filter and other a flower filter in the same chain/prio:

1. Both threads enter tcnewtfilter, both find the chain empty, both drop filterchainlock
2. u32 finishes tcfprotocreate("u32") first, calls tcfchaintpinsertunique() -> inserts u32_tp into the chain
3. flower finishes tcfprotocreate("flower") later, calls tcfchaintpinsertunique() -> tcfchaintpfind() now sees u32tp already there, takes a reference on it, destroys flower's own tpnew and returns u32tp to the caller.

Flower then hits the kind mismatch check (because it requested for kind "flower" but tp->ops->kind is "u32") and goes through the errout path which calls tcfprotoput() on u32tp. If the u32 thread has already gone through its own errout (its change() call failed on the PoC's empty options) and dropped its create and insert refs, flower's put is the last one and drops u32tp's refcnt to zero.

At this point tp->ops->destroy() runs in a context that never took rtnl_lock. When that happens, it might cause a UAF like the following (illustrated by the PoC):

[ +0.000710] BUG: KASAN: slab-use-after-free in u32init (net/sched/clsu32.c:393) [ +0.000281] Read of size 8 at addr ffff888120022f00 by task pocfengxue/524

Call Trace: u32init (net/sched/clsu32.c:393) tcnewtfilter (net/sched/cls_api.c:2378)

Allocated by task 526: u32init (net/sched/clsu32.c:378) tcnewtfilter (net/sched/cls_api.c:2378)

Freed by task 522: kfree u32destroy (net/sched/clsu32.c:662) tcfprotodestroy (net/sched/clsapi.c:446) tcfprotoput (net/sched/clsapi.c:459) tcnewtfilter (net/sched/cls_api.c:2459)

Fix this by having tcfprotodestroy() take rtnl_lock around tp->ops->destroy() for locked classifiers whenever rtnl is not held.

To explain why I used a temp variable "notlockless" I'd like to point to a semi-related note on rtnlheld vs TCFPROTOOPSDOITUNLOCKED (adding here for future cleanup if deemed necessary): The rtnlheld parameter and the TCFPROTOOPSDOITUNLOCKED flag are redundant sources of truth for whether rtnllock is held. Among the nine classifier destroy(..rtnlheld..) callbacks, only flower consults the rtnlheld parameter which it propagates to tcsetupcbdestroy() and tcsetupcbcall(). The other eight (u32, flow, bpf, cgroup, route, basic, fw, mall) ignore it entirely;-> those that call tcsetupcb_destroy() (u32, bpf, mall) hardcode true always instead of forwarding the parameter.

A future cleanup should remove the rtnl_held parameter from the destroy callback signature entirely and have callers rely solely on their knowledge whether they are running in an unlocked context.