[linux-yocto] [PATCH] netfilter: Fix kmemleak false positive reports

[Bruce Ashfield]

On 2018-10-25 4:27 PM, Mark Asselstine wrote:
> On Thursday, October 25, 2018 6:41:09 AM EDT He Zhe wrote:
>> On 2018/10/25 18:29, Bruce Ashfield wrote:
>>> On 10/23/18 6:33 AM, zhe.he at windriver.com wrote:
>>>> From: He Zhe <zhe.he at windriver.com>
>>>>
>>>> unreferenced object 0xffff9643edb89900 (size 256):
>>>>     comm "sd-resolve", pid 220, jiffies 4295016710 (age 208.256s)
>>>>     hex dump (first 32 bytes):
>>>>       01 00 00 00 00 00 00 00 03 00 74 f3 ba b1 b6 b5  ..........t.....
>>>>       65 3e 00 00 00 00 00 00 90 f9 a0 ed 43 96 ff ff  e>..........C...
>>>>     backtrace:
>>>>       [<0000000070d5b185>] kmem_cache_alloc+0x146/0x200
>>>>       [<0000000007a27faa>] __nf_conntrack_alloc.isra.13+0x4d/0x170
>>>> [nf_conntrack] [<00000000ecc5b0ec>] init_conntrack+0x6a/0x2f0
>>>> [nf_conntrack] [<000000003d38809f>] nf_conntrack_in+0x2c5/0x360
>>>> [nf_conntrack] [<000000001fe154e3>] ipv4_conntrack_local+0x5d/0x70
>>>> [nf_conntrack_ipv4] [<0000000027adadb2>] nf_hook_slow+0x48/0xd0
>>>>       [<000000009893511f>] __ip_local_out+0xbd/0xf0
>>>>       [<00000000d68cbd2f>] ip_local_out+0x1c/0x50
>>>>       [<00000000995e2f37>] ip_send_skb+0x19/0x40
>>>>       [<000000003d95f220>] udp_send_skb.isra.5+0x157/0x360
>>>>       [<00000000ebc25968>] udp_sendmsg+0x9d8/0xc10
>>>>       [<000000003bef56ec>] inet_sendmsg+0x3e/0xf0
>>>>       [<000000008d23e405>] sock_sendmsg+0x1d/0x30
>>>>       [<000000008c297097>] ___sys_sendmsg+0x108/0x2b0
>>>>       [<00000000f15a806c>] __sys_sendmmsg+0xba/0x1c0
>>>>       [<00000000e195d2cf>] __x64_sys_sendmmsg+0x24/0x30
>>>>
>>>> In __nf_conntrack_confirm, object ct can be referenced to by the stack
>>>> variable ct and the members of ct->tuplehash. kmemleak needs at least
>>>> one of them to find the ct object during scan.
>>>>
>>>> When the ct object is moved from the unconfirmed hlist to the confirmed
>>>> hlist. kmemleak cannot see ct object if things happen in the following
>>>> order and thus give the above false positive report.
>>>> 1) ct object is removed from unconfirmed hlist.
>>>> 2) kmemleak scans data/bss sections.
>>>> 3) ct object is added to confirmed hlist and ct is destroyed as the
>>>> function returns.
>>>> 4) kmemleak scans task stacks.
>>>>
>>>> This patch marks the ct object as not a leak.
>>>
>>> Has this also been submitted upstream ?
>>>
>>> Due to travel, I haven't gotten to it yet. But if it has been submitted
>>> upstream, I can get it merged by Monday
>>
>> This has been submitted to lkml but has not got any attention. Mark
>> and I still are working on it to pinpoint the bad code.
>>
>> Zhe
> 
> I finally finished my analysis and found the direct cause and potential fix.
> With this knowledge I was actually able to find a 4.19 commit which had
> nothing to do with kmemleak but happens to fix this. So I have submitted a
> backport request to DaveM via netdev.
> 
> https://marc.info/?l=linux-netdev&m=154049877715352&w=2
> 
> All the details are there and hopefully folks agree and this will show up in
> the next 4.18.y stable shortly. It is a mainline backport so I am thinking/
> hoping it will not run into considerable resistance. Since we are fast
> approaching the release I am not sure what approach you want to take Bruce,
> but I will leave it in your hands.

Either way, I'm not sending a SRCREV bump before the release. So we can
wait for it to cycle through -stable.

I saw your post on netdev, and I assume you'll let me know if it doesn't
make -stable, and we need to cherry pick it ourselves.

Cheers,

Bruce

> 
> MarkA
> 
>>
>>> Bruce
>>>
>>>> Signed-off-by: He Zhe <zhe.he at windriver.com>
>>>> ---
>>>> This is only for v4.18
>>>>
>>>>    net/netfilter/nf_conntrack_core.c | 3 +++
>>>>    1 file changed, 3 insertions(+)
>>>>
>>>> diff --git a/net/netfilter/nf_conntrack_core.c
>>>> b/net/netfilter/nf_conntrack_core.c index 3d52804..9b554c7 100644
>>>> --- a/net/netfilter/nf_conntrack_core.c
>>>> +++ b/net/netfilter/nf_conntrack_core.c
>>>> @@ -35,6 +35,7 @@
>>>>    #include <linux/mm.h>
>>>>    #include <linux/nsproxy.h>
>>>>    #include <linux/rculist_nulls.h>
>>>> +#include <linux/kmemleak.h>
>>>>      #include <net/netfilter/nf_conntrack.h>
>>>>    #include <net/netfilter/nf_conntrack_l3proto.h>
>>>> @@ -1138,6 +1139,8 @@ __nf_conntrack_alloc(struct net *net,
>>>>        if (ct == NULL)
>>>>            goto out;
>>>>    +    kmemleak_not_leak(ct);
>>>> +
>>>>        spin_lock_init(&ct->lock);
>>>>        ct->tuplehash[IP_CT_DIR_ORIGINAL].tuple = *orig;
>>>>        ct->tuplehash[IP_CT_DIR_ORIGINAL].hnnode.pprev = NULL;
> 
> 
> 
>