netdev - Re: [PATCH v2 4.14 0/5] 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] [day] [month] [year] [list]

Message-ID: <20190106175610.GE166797@sasha-vm>
Date:   Sun, 6 Jan 2019 12:56:10 -0500
From:   Sasha Levin <sashal@...nel.org>
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 v2 4.14 0/5] netfilter: xt_connlimit: backport upstream
 fixes for race in connection counting

On Wed, Jan 02, 2019 at 06:41:59PM -0200, Mauricio Faria de Oliveira wrote:
>From: Ubuntu <ubuntu@...ilil.segmaas.1ss>
>
>[changelog]
> - v2: include patch 5/5 (a very recent fix to patch 4/5) which is
>       not yet in Linus's tree but it's in nf.git + linux-next.git,
>       thus should make it shortly.  Test results still consistent.
>       Thanks Florian Westphal for reviewing and pointing that out.
>
>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.
>
>(v2 update: see Florian's reply to v1 thread [1]; these different results
> are probably explained by very recent fixes still missing back then.)
>
>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.
>
>The fix is PATCH 4 (needs fix in PATCH 5), and PATCHes 1-3 are helpers for
>a cleaner backport.
>All backports are simple, and essentially consist of refresh context lines
>and use older struct/file names.
>
>Reviews from netfilter maintainers are very appreciated, as I've no previous
>experience in this area, and although the backports look simple and build/run
>correctly, there's usually stuff that only more experienced people may notice.

Queued for 4.14, thank you.

--
Thanks,
Sasha