REJECT[L/J]: [SRU Jammy, OEM-6.0, OEM-6.1, Lunar] CVE-2023-4244
Roxana Nicolescu
roxana.nicolescu at canonical.com
Fri Sep 29 13:15:41 UTC 2023
On 29/09/2023 14:31, Thadeu Lima de Souza Cascardo wrote:
> On Fri, Sep 29, 2023 at 02:01:35PM +0200, Stefan Bader wrote:
>> On 29.09.23 13:12, Thadeu Lima de Souza Cascardo wrote:
>>> On Fri, Sep 29, 2023 at 12:21:36PM +0200, Stefan Bader wrote:
>>>> On 16.09.23 02:48, Cengiz Can wrote:
>>>>> [Impact]
>>>>> A use-after-free vulnerability in the Linux kernel's netfilter: nf_tables
>>>>> component can be exploited to achieve local privilege escalation. Due to a race
>>>>> condition between nf_tables netlink control plane transaction and nft_set
>>>>> element garbage collection, it is possible to underflow the reference counter
>>>>> causing a use-after-free vulnerability. We recommend upgrading past commit
>>>>> 3e91b0ebd994635df2346353322ac51ce84ce6d8.
>>>>>
>>>>> [Fix]
>>>>> This was a mess. First CVE-2023-4563 was announced with no information. Then
>>>>> someone pointed out to two threads in netdev trees that are possibly fixing the
>>>>> issue. Initially 5 commits were included in the fix. Then another one came. Then
>>>>> came 7 Fixes commits to those commits. Thus, applying those to our trees was not
>>>>> easy.
>>>>>
>>>>> While I was doing that, CVE-2023-4563 disappaered from CNA websites and
>>>>> CVE-2023-4244 was announced. It came with a subset of those fix commits but
>>>>> obviously Fixes commits and prerequisites were still needed. So I kept my
>>>>> progress and changed my tags to indicate CVE-2023-4244 instead.
>>>>>
>>>>> These are the Fix commits that I extracted from merge:
>>>>>
>>>>> - 24138933b97b ("netfilter: nf_tables: don't skip expired elements during walk")
>>>>> - 5f68718b34a5 ("netfilter: nf_tables: GC transaction API to avoid race with control plane")
>>>>> - f6c383b8c31a ("netfilter: nf_tables: adapt set backend to use GC transaction API")
>>>>> - c92db3030492 ("netfilter: nft_set_hash: mark set element as dead when deleting from packet path")
>>>>> - a2dd0233cbc4 ("netfilter: nf_tables: remove busy mark and gc batch API")
>>>>> - 23185c6aed1f ("netfilter: nft_dynset: disallow object maps")
>>>>>
>>>>> These are the Fixes to those fix commits:
>>>>>
>>>>> - 7845914f45f0 ("netfilter: nf_tables: don't fail inserts if duplicate has expired")
>>>>> - 08713cb006b6 ("netfilter: nf_tables: fix kdoc warnings after gc rework")
>>>>> - 6a33d8b73dfa ("netfilter: nf_tables: fix GC transaction races with netns and netlink event exit path")
>>>>> - 02c6c24402bf ("netfilter: nf_tables: GC transaction race with netns dismantle")
>>>>> - 720344340fb9 ("netfilter: nf_tables: GC transaction race with abort path")
>>>>> - 8357bc946a2a ("netfilter: nf_tables: use correct lock to protect gc_list")
>>>>> - 8e51830e29e1 ("netfilter: nf_tables: defer gc run if previous batch is still pending")
>>>>>
>>>>> The rest are there to massage those to allow cleaner picks.
>>>>>
>>>>> [Test case]
>>>>> Compile, boot and nftables test suite tested.
>>>>>
>>>>> This is the testing formula that was used:
>>>>>
>>>>> ```
>>>>> sudo apt update && sudo apt install -y build-essential autoconf libtool bison flex libgmp3-dev libedit-dev pkg-config
>>>>>
>>>>> git clone --depth=1 git://git.netfilter.org/libmnl
>>>>> cd libmnl
>>>>> sh autogen.sh
>>>>> ./configure
>>>>> make -j8
>>>>> sudo make install
>>>>> cd ~
>>>>>
>>>>> git clone --depth=1 git://git.netfilter.org/libnftnl
>>>>> cd libnftnl
>>>>> sh autogen.sh
>>>>> ./configure
>>>>> make -j8
>>>>> sudo make install
>>>>> cd ~
>>>>>
>>>>> git clone --depth=1 git://git.netfilter.org/nftables
>>>>> cd nftables
>>>>> sh autogen.sh
>>>>> ./configure --disable-man-doc
>>>>> make -j8
>>>>> cd tests/shell/
>>>>> ./run_tests.sh
>>>>> ```
>>>>>
>>>>> Test results were always better with the patches, but never 100%.
>>>>>
>>>>> Lunar:
>>>>> unpatched: I: results: [OK] 335 [SKIPPED] 17 [FAILED] 21 [TOTAL] 373
>>>>> patched: I: results: [OK] 340 [SKIPPED] 16 [FAILED] 17 [TOTAL] 373
>>>>>
>>>>> OEM-6.1 (installed to jammy)
>>>>> unpatched: I: results: [OK] 204 [SKIPPED] 11 [FAILED] 158 [TOTAL] 373
>>>>> patched: I: results: [OK] 209 [SKIPPED] 11 [FAILED] 153 [TOTAL] 373
>>>>>
>>>>> OEM-6.0 (installed to jammy)
>>>>> unpatched: I: results: [OK] 200 [SKIPPED] 11 [FAILED] 162 [TOTAL] 373
>>>>> patched: I: results: [OK] 209 [SKIPPED] 11 [FAILED] 153 [TOTAL] 373
>>>>>
>>>>> Jammy 5.15
>>>>> unpatched: I: results: [OK] 202 [SKIPPED] 11 [FAILED] 160 [TOTAL] 373
>>>>> patched: I: results: [OK] 207 [SKIPPED] 11 [FAILED] 155 [TOTAL] 373
>>>>>
>>>>> [Potential regression]
>>>>> Medium regression potential. Many of those commits are clean cherry picks but I
>>>>> had to adjust and backport 5f68718b34a5 ("netfilter: nf_tables: GC transaction
>>>>> API to avoid race with control plane") to anything older than 6.2 because it's
>>>>> extremely hard to backport commit f80a612dd77c ("netfilter: nf_tables: add
>>>>> support to destroy operation") to older kernels.
>>>>>
>>>>> Florian Westphal (6):
>>>>> netfilter: nf_tables: don't skip expired elements during walk
>>>>> netfilter: nft_set_rbtree: fix null deref on element insertion
>>>>> netfilter: nft_set_rbtree: fix overlap expiration walk
>>>>> netfilter: nf_tables: don't fail inserts if duplicate has expired
>>>>> netfilter: nf_tables: fix kdoc warnings after gc rework
>>>>> netfilter: nf_tables: defer gc run if previous batch is still pending
>>>>>
>>>>> Pablo Neira Ayuso (19):
>>>>> netfilter: nf_tables: consolidate set description
>>>>> netfilter: nf_tables: add function to create set stateful expressions
>>>>> netfilter: nf_tables: perform type checking for existing sets
>>>>> netfilter: nf_tables: do not set up extensions for end interval
>>>>> netfilter: nf_tables: honor set timeout and garbage collection updates
>>>>> netfilter: nf_tables: integrate pipapo into commit protocol
>>>>> netfilter: nf_tables: validate catch-all set elements
>>>>> netfilter: nf_tables: drop map element references from preparation
>>>>> phase
>>>>> netfilter: nf_tables: GC transaction API to avoid race with control
>>>>> plane
>>>>> netfilter: nft_set_rbtree: Switch to node list walk for overlap
>>>>> detection
>>>>> netfilter: nft_set_rbtree: skip elements in transaction from garbage
>>>>> collection
>>>>> netfilter: nf_tables: adapt set backend to use GC transaction API
>>>>> netfilter: nft_set_hash: mark set element as dead when deleting from
>>>>> packet path
>>>>> netfilter: nf_tables: remove busy mark and gc batch API
>>>>> netfilter: nf_tables: fix GC transaction races with netns and netlink
>>>>> event exit path
>>>>> netfilter: nf_tables: GC transaction race with netns dismantle
>>>>> netfilter: nf_tables: GC transaction race with abort path
>>>>> netfilter: nf_tables: use correct lock to protect gc_list
>>>>> netfilter: nft_dynset: disallow object maps
>>>>>
>>>>> include/net/netfilter/nf_tables.h | 164 +++---
>>>>> net/netfilter/nf_tables_api.c | 860 ++++++++++++++++++++++++------
>>>>> net/netfilter/nft_dynset.c | 3 +
>>>>> net/netfilter/nft_lookup.c | 36 +-
>>>>> net/netfilter/nft_set_bitmap.c | 5 +-
>>>>> net/netfilter/nft_set_hash.c | 111 ++--
>>>>> net/netfilter/nft_set_pipapo.c | 138 +++--
>>>>> net/netfilter/nft_set_rbtree.c | 466 ++++++++++------
>>>>> 8 files changed, 1249 insertions(+), 534 deletions(-)
>>>>>
>>>> This is impossible to process. There is no numbering in the patches which
>>>> causes exports to be randomly ordered. One would have to check each patch
>>>> individually. And attempting this for Lunar I now hit the second case where
>>>> a patch for Lunar would not apply. Some need to be skipped because they came
>>>> via stable. Please provide us with individual sets for Lunar and Jammy.
>>>>
>>>> --
>>>> - Stefan
>>> Hey, Stefan.
>>>
>>> This was submitted very early in the cycle and was a very complicated backport
>>> that Cengiz worked very hard to get done. He even asked for advice on how best
>>> to submit it given all the complications in how to get interleaving patches
>>> applying in different series.
>>>
>>> In hindsight, in a case like this, I think it would have served us much better
>>> to have different submissions per series, even if that meant people would have
>>> to review certain changes more than once.
>>>
>>> I understand interactions with the stable patches need to be taken into
>>> consideration, but I don't find it fair that an early submission gets punished
>>> and needs to be redone. Specially this late in the cycle and considering this
>>> is a fix we want to apply in the underlying security cycle.
>>>
>>> What is done is done, and now we will need to fix this up.
>>>
>>> If it applies clean to 2023.09.04, but not to master-next, I think it is only
>>> fair that we apply it in the security cycle and the SRU cycle gets delayed
>>> until this gets fixed. I can certainly try to help, but that won't get done
>>> before Monday.
>>>
>>> Let me get back to you in case this applieds just fine.
>>>
>>> But my conclusion here is that we ought to apply fixes like this (complicated
>>> ones and that are submitted early) earlier and the stable patches should then
>>> be prepared on top.
>> This would usually be what at least I would do (apply security before any
>> stable). This case suffered from one additional issue which is unrelated to
>> the content. That now was just the final nail on the coffin. The bigger
>> issue was that saving the emails into a folder would produce no ordering
>> info. And with the mixed series there is the additional danger of missing
>> one or picking one too many. That was already something I was afraid of but
>> hoped that things would work out better. In hindsight, yes such big and
>> complicated sets would be better individual submissions or have a pull
>> request per series.
> Okay, since this was submitted early in the process in good faith, and even had
> 2 ACKs (which was telling Cengiz all was okay), would you be willing to apply
> it if we get a new submission by today or early Monday?
I managed to apply it for lunar only, will try jammy next. It is pushed here
git://git.launchpad.net/~roxanan/+git/lunar master-next branch.
@Cengiz could you
take a quick look and even test what's there to make sure nothing is missed?
I had to include "netfilter: nf_tables: GC transaction API to avoid race
with control plane".
And these were already applied from upstream:
"netfilter: nf_tables: integrate pipapo into commit protocol",
" netfilter: nft_set_rbtree: fix overlap expiration walk",
> Cascardo.
>
>>> Cascardo.
>> --
>> - Stefan
More information about the kernel-team
mailing list