netdev - Re: [PATCH] [PATCH net] net: Do not hold the reference for the same sk_rx

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_iQpX3jZBAmJQ=9OsObJQdoXzWLODbJLfEsPMyw27Ap5Byzw@mail.gmail.com>
Date:   Thu, 16 Mar 2017 11:42:10 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     Kaiwen Xu <kaiwen.xu@...u.com>
Cc:     Jakub Sitnicki <jkbs@...hat.com>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: Re: [PATCH] [PATCH net] net: Do not hold the reference for the same sk_rx_dst.

On Thu, Mar 16, 2017 at 11:18 AM, Kaiwen Xu <kaiwen.xu@...u.com> wrote:
> On Thu, Mar 16, 2017 at 11:45:03AM +0100, Jakub Sitnicki wrote:
>> On Thu, Mar 16, 2017 at 10:12 AM GMT, Kevin Xu wrote:
>> > Do you mean the message looping endlessly?
>>
>> No, the message is emitted just once. Around 100 seconds after
>> destroying a few namespaces. Occurs not so often, maybe once per ten
>> runs.
>>
>> -Jakub
>>
>
> I saw that happening from time to time during my test as well, I suspect
> it was because some TCP sockets stays in either TCP_TIME_WAIT or
> TCP_FIN_WAIT1. But eventually those sockets get destroyed and lo gets
> deleted as well.

But TIMEWAIT sockets are purged by inet_twsk_purge() during netns
destroy, apparently before lo is destroyed.

>
> The patch was fixing an issue I am seeing, that the message gets looped
> forever, and causing a deadlock on new network ns creation.
>

But as DaveM said, the race still could happen after your patch,
the if check you add is not atomic at all.

Also, we should already lock the sock at the time we call
inet_sk_rx_dst_set(), but perhaps not for TCP_LISTEN case...