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 12:31:10 UTC 2023
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?
Cascardo.
>
> >
> > Cascardo.
>
> --
> - Stefan
More information about the kernel-team
mailing list