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

[Stanislav Kinsburskiy]

23.11.2015 16:09, 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.

Sorry, the first address belongs to kernel space.