[Devel] [PATCH rh7] fib_rules: mark default fib reles as BLACK

[Stanislav Kinsburskiy]

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 <skinsbursky at 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

BTW, first 16 bytes representing list structure. And while first address 
if 0xffff880068050660 somehow reminds kernel address (but it's not), the 
second one: 0xdead000000200200 - is something, which is dead.
I'm not familiar with kmemcheck technologies, but it look like poisoning.

>
>
>
>
>> +	kmemleak_ignore(r);
>>   	return 0;
>>   }
>>   EXPORT_SYMBOL(fib_default_rule_add);
>>