netdev - Re: loopback device reference count leakage

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAM_iQpWJmXnsZ4D870aEZ+HL3pfaZU3HyunyxnhG9=EHS0Tcdg@mail.gmail.com>
Date:   Thu, 26 Jan 2017 17:01:38 -0800
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Kaiwen Xu <kevin@...xu.net>
Cc:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: loopback device reference count leakage

On Thu, Jan 26, 2017 at 2:51 PM, Kaiwen Xu <kevin@...xu.net> wrote:
> Hi Cong,
>
> I tested out your patch, it does seem to be preventing the issue from
> happening. Here are the dev_put/dev_hold() calls with your patch
> applied.

Good. Now we narrow down the bug to those dst's referring loopback_dev.

> However, what confuses me is that when the issue didn't occur, there
> were always multiple dst_ifdown() calls at the end continuously holding
> and releasing the loopback device reference count (sometimes it will be
> looping for about a minute), until the last dst_destroy() happens. E.g.
>
> Jan 11 16:14:59 <hostname> kernel: [ 2033.429563] lo: dev_hold 2 dst_ifdown
> Jan 11 16:14:59 <hostname> kernel: [ 2033.429565] lo: dev_put 2 dst_ifdown
> Jan 11 16:15:00 <hostname> kernel: [ 2034.453484] lo: dev_hold 2 dst_ifdown
> Jan 11 16:15:00 <hostname> kernel: [ 2034.453487] lo: dev_put 2 dst_ifdown
>
> (this continues...)
>
> Jan 11 16:15:01 <hostname> kernel: [ 2035.129452] lo: dev_put 1 dst_destroy
>
> And when the issue did occur, the last dst_destroy() call never occurs.

Yeah, I noticed that too. So we have two cases here:

1) If these dst's (referring to loopback_dev) really need to stay in
GC for a long
time, then we should really just releasing loopback references as what my patch
does.

2) If they don't not, that is, if they are supposed to be GC'ed soon
in this case,
then we should investigate why they are still there.

2) seems more likely than 1), because at the point when loopback device is
being unregistered, the whole network namespace is being gone, all other
devices are already gone, no one should a take reference to this netns,
therefore no one should take a reference to any dst referring to any device
in it too, even though the dst GC is global.

I'd suggest to you add some debugging printk's to the dst refcount functions,
or maybe just inside dst_gc_task(). I think the last dst referring to
the loopback
dev is still being referred at that point, which prevents GC from destroying it.

Meanwhile, if it would be also helpful if you can share how you managed to
reproduce this reliably, I saw this bug in our data center before but never
know how to reproduce it.

Thanks!