On 11/23/2015 06:02 PM, Stanislav Kinsburskiy wrote: > > > 23.11.2015 14:49, Andrey Ryabinin пишет: >> On 11/23/2015 03:47 PM, Stanislav Kinsburskiy wrote: >>> This patch fixed flase positive, reported by KASan. >>> >> s/flase/false >> s/KASan/kmemleak >> >>> https://jira.sw.ru/browse/PSBM-41453 >>> >>> Signed-off-by: Stanislav Kinsburskiy <skinsbur...@virtuozzo.com> >>> --- >>> net/core/fib_rules.c | 4 ++++ >>> 1 file changed, 4 insertions(+) >>> >>> diff --git a/net/core/fib_rules.c b/net/core/fib_rules.c >>> index 0e91311..bc69cef 100644 >>> --- a/net/core/fib_rules.c >>> +++ b/net/core/fib_rules.c >>> @@ -36,6 +36,10 @@ int fib_default_rule_add(struct fib_rules_ops *ops, >>> /* The lock is not required here, the list in unreacheable >>> * at the moment this function is called */ >>> list_add_tail(&r->list, &ops->rules_list); >>> + >>> + /* This object is not referenced by any user and will be removed on net >>> + * ns stop in fib_rules_cleanup_ops */ >> If it's not referenced, than how it can be removed? >> Changelog and this comment is rather poor, and doesn't explain why it's a >> false-positive. >> >> To me, it doesn't look so: >> >> >> static void fib_rules_cleanup_ops(struct fib_rules_ops *ops) >> ... >> list_for_each_entry_safe(rule, tmp, &ops->rules_list, list) { >> list_del_rcu(&rule->list); // remove from list, so object >> will become unerferenced >> >> fib_rule_put(rule) => >> if (atomic_dec_and_test(&rule->refcnt)) >> call_rcu(&rule->rcu, fib_rule_put_rcu); //release rule iff >> refcnt was 1 >> >> >> >> So if 'rule->refcnt > 1' fib_rules_cleanups_ops() will remove rule from >> list, but won't free it. >> >> If you look at hex dump from report, you will see that refcnt is 4: >> >> unreferenced object 0xffff880255d42520 (size 128): >> comm "vzctl", pid 184011, jiffies 4324028137 (age 193.266s) >> hex dump (first 32 bytes): >> 50 06 05 68 00 88 ff ff 00 02 20 00 00 00 ad de P..h...... ..... >> 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ >> ^^^^^^^^^^^ >> fib_rule->refcnt > > Ok, 4 is awesome. > Then I have to notice: > 1) The idea of this object, is that it's added to global rules list with > initial counter, equal to 1. Which means, it's hold by the kernel itself. And > this doesn't allow to destroy this rule before network namespace is alive. At > least this was the intention in the original patch. > 2) It is removed in the place you mentioned in fib_rules_cleanup_ops. If > refcnt is really equal to 4, then something is seriously broken, and we have > to leak such rules all the time.
Yes, and fib_rules_lookup() looks very suspicious to me. It could increment refcnt, but I'm failed to find corresponding decrement. > 3) This object holds network namespace. And it also have to be leaked leaked > after CT stop. Is it? > >> >> >> >>> + kmemleak_ignore(r); >>> return 0; >>> } >>> EXPORT_SYMBOL(fib_default_rule_add); >>> > _______________________________________________ Devel mailing list Devel@openvz.org https://lists.openvz.org/mailman/listinfo/devel
