From: Jamal Hadi Salim <j...@mojatatu.com> Date: Sun, 7 Oct 2018 07:40:17 -0400
> From: Al Viro <v...@zeniv.linux.org.uk> > > cls_u32.c misuses refcounts for struct tc_u_hnode - it counts references > via ->hlist and via ->tp_root together. u32_destroy() drops the former > and, in case when there had been links, leaves the sucker on the list. > As the result, there's nothing to protect it from getting freed once links > are dropped. > That also makes the "is it busy" check incapable of catching the root > hnode - it *is* busy (there's a reference from tp), but we don't see it as > something separate. "Is it our root?" check partially covers that, but > the problem exists for others' roots as well. > > AFAICS, the minimal fix preserving the existing behaviour (where it doesn't > include oopsen, that is) would be this: > * count tp->root and tp_c->hlist as separate references. I.e. > have u32_init() set refcount to 2, not 1. > * in u32_destroy() we always drop the former; > in u32_destroy_hnode() - the latter. > > That way we have *all* references contributing to refcount. List > removal happens in u32_destroy_hnode() (called only when ->refcnt is 1) > an in u32_destroy() in case of tc_u_common going away, along with > everything reachable from it. IOW, that way we know that > u32_destroy_key() won't free something still on the list (or pointed to by > someone's ->root). > > Reproducer: ... > Signed-off-by: Al Viro <v...@zeniv.linux.org.uk> > Signed-off-by: Jamal Hadi Salim <j...@mojatatu.com> Applied and queued up for -stable.
