netdev - Re: [PATCH] ipv4: Delete uncached routes upon unregistration of loopback device.

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: <f6f770a7-17af-d51f-3ffb-4edba9b28101@gmail.com>
Date:   Sat, 4 May 2019 11:56:38 -0400
From:   Eric Dumazet <eric.dumazet@...il.com>
To:     Tetsuo Handa <penguin-kernel@...ove.sakura.ne.jp>,
        "David S. Miller" <davem@...emloft.net>
Cc:     David Ahern <dsahern@...il.com>,
        Eric Dumazet <eric.dumazet@...il.com>,
        Julian Anastasov <ja@....bg>,
        Cong Wang <xiyou.wangcong@...il.com>,
        syzbot <syzbot+30209ea299c09d8785c9@...kaller.appspotmail.com>,
        ddstreet@...e.org, dvyukov@...gle.com,
        linux-kernel@...r.kernel.org, netdev@...r.kernel.org,
        syzkaller-bugs@...glegroups.com,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Mahesh Bandewar <maheshb@...gle.com>
Subject: Re: [PATCH] ipv4: Delete uncached routes upon unregistration of
 loopback device.



On 5/4/19 10:52 AM, Tetsuo Handa wrote:
> syzbot is hitting infinite loop when a loopback device in a namespace is
> unregistered [1]. This is because rt_flush_dev() is moving the refcount of
> "any device to unregister" to "a loopback device in that namespace" but
> nobody can drop the refcount moved from non loopback devices when the
> loopback device in that namespace is unregistered.
> 
> This behavior was introduced by commit caacf05e5ad1abf0 ("ipv4: Properly
> purge netdev references on uncached routes.") but there is no description
> why we have to temporarily move the refcount to "a loopback device in that
> namespace" and why it is safe to do so, for rt_flush_dev() becomes a no-op
> when "a loopback device in that namespace" is about to be unregistered.
> 
> Since I don't know the reason, this patch breaks the infinite loop by
> deleting the uncached route (which eventually drops the refcount via
> dst_destroy()) when "a loopback device in that namespace" is unregistered
> rather than when "non-loopback devices in that namespace" is unregistered.

Well, you have not fixed a bug, you simply made sure that whatever cpu is using the
routes you forcibly deleted is going to crash the host very soon (use-after-frees have
undefined behavior, but KASAN should crash most of the times)

Please do not send patches like that with a huge CC list, keep networking patches
to netdev mailing list.

Mahesh has an alternative patch, adding a fake device that can not be dismantled
to make sure we fully intercept skbs sent through a dead route, instead of relying
on loopback dropping them later at some point.