REJECT[L/J]: [SRU Jammy, OEM-6.0, OEM-6.1, Lunar] CVE-2023-4244
Cengiz Can
cengiz.can at canonical.com
Mon Oct 2 11:32:05 UTC 2023
On 29/09/2023 16:56, Roxana Nicolescu wrote:
>
> On 29/09/2023 15:15, Roxana Nicolescu wrote:
>>
>> 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",
>>
> Same for jammy. Changes are here
> git://git.launchpad.net/~roxanan/+git/jammy master-next branch.
> Last 13 commits are of interest. Same for lunar, I forgot to mention.
>
> Roxana
Thank you all for the feedback.
Although I've taken all of the precautions to prevent any conflicts here
(applied and re-applied
to all of the trees using the single liner mentioned on cover letter) it
left a bitter taste in my
mouth as well.
Let's hope we don't receive such large patchsets in the future.
Thanks again.
>>> Cascardo.
>>>
>>>>> Cascardo.
>>>> --
>>>> - Stefan
More information about the kernel-team
mailing list