REJECT[L/J]: [SRU Jammy, OEM-6.0, OEM-6.1, Lunar] CVE-2023-4244

Thadeu Lima de Souza Cascardo cascardo at canonical.com
Fri Sep 29 11:12:13 UTC 2023


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.

Cascardo.



More information about the kernel-team mailing list