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:56:49 UTC 2023
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
>> Cascardo.
>>
>>>> Cascardo.
>>> --
>>> - Stefan
More information about the kernel-team
mailing list