netdev - Re: [PATCH 4.14 0/4] netfilter: xt_connlimit: backport upstream fixes for race in connection counting

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: <20190102171705.sgm5dskcqx3dygoy@breakpoint.cc>
Date:   Wed, 2 Jan 2019 18:17:05 +0100
From:   Florian Westphal <fw@...len.de>
To:     Mauricio Faria de Oliveira <mfo@...onical.com>
Cc:     stable@...r.kernel.org, netdev@...r.kernel.org,
        Florian Westphal <fw@...len.de>,
        Alakesh Haloi <alakeshh@...zon.com>,
        nivedita.singhvi@...onical.com,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Jozsef Kadlecsik <kadlec@...ckhole.kfki.hu>,
        "David S. Miller" <davem@...emloft.net>,
        Yi-Hung Wei <yihung.wei@...il.com>
Subject: Re: [PATCH 4.14 0/4] netfilter: xt_connlimit: backport upstream
 fixes for race in connection counting

Mauricio Faria de Oliveira <mfo@...onical.com> wrote:
> Recently, Alakesh Haloi reported the following issue [1] with stable/4.14:
> 
>   """
>   An iptable rule like the following on a multicore systems will result in
>   accepting more connections than set in the rule.
> 
>   iptables  -A INPUT -p tcp -m tcp --syn --dport 7777 -m connlimit \
>         --connlimit-above 2000 --connlimit-mask 0 -j DROP
>   """
> 
> And proposed a fix that is not in Linus's tree. The discussion went on to
> confirm whether the issue was still reproducible with mainline/nf.git tip,
> and to either identify the upstream fix or re-submit the non-upstream fix.
> 
> Alakesh eventually was able to test with upstream, and reported that issue
> was still reproducible [2].
> On that, our findinds diverge, at least in my test environment:
> 
> First, I verified that the suggested mainline fix for the issue [3] indeed
> fixes it, by testing with it applied and reverted on v4.18, a clean revert.
> (The issue is reproducible with the commit reverted).
> 
> Then, with a consistent reproducer, I moved to nf.git, with HEAD on commit
> a007232 ("netfilter: nf_conncount: fix argument order to find_next_bit"),
> and the issues was not reproducible (even with 20+ threads on client side,
> the number Alakesh reported to achieve 2150+ connections [4], and I tried
> spreading the network interface IRQ affinity over more and more CPUs too.)
> 
> Either way, the suggested mainline fix does actually fix the issue in 4.14
> for at least one environment. So, it might well be the case that Alakesh's
> test environment has differences/subtleties that leads to more connections
> accepted, and more commits are needed for that particular environment type.

nf_conncount has a design flaw that is only closed in nf.git/net.git
at the time of this writing, so results with earlier kernels (including
4.20) might just fail with different bugs.

4.14 doesn't have those problems, so I think this series (aside from the
nit in patch 4/4) indeed should fix the issue reported.

> But for now, with one bare-metal environment (24-core server, 4-core client)
> verified, I thought of submitting the patches for review/comments/testing,
> then looking for additional fixes for that environment separately.

4.14 should be good after this afaics.

Thanks a lot for doing this backport and the details testing
information.