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.
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