From: Greg Kroah-Hartman Date: Tue, 8 Apr 2025 10:31:35 +0000 (+0200) Subject: 5.10-stable patches X-Git-Tag: v5.4.292~26 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=3e939466133a2d884140ddb88bfbb6960eac621e;p=thirdparty%2Fkernel%2Fstable-queue.git 5.10-stable patches added patches: netfilter-conntrack-fix-crash-due-to-confirmed-bit-load-reordering.patch x86-kexec-fix-double-free-of-elf-header-buffer.patch --- diff --git a/queue-5.10/netfilter-conntrack-fix-crash-due-to-confirmed-bit-load-reordering.patch b/queue-5.10/netfilter-conntrack-fix-crash-due-to-confirmed-bit-load-reordering.patch new file mode 100644 index 0000000000..0fb01b611d --- /dev/null +++ b/queue-5.10/netfilter-conntrack-fix-crash-due-to-confirmed-bit-load-reordering.patch @@ -0,0 +1,181 @@ +From 0ed8f619b412b52360ccdfaf997223ccd9319569 Mon Sep 17 00:00:00 2001 +From: Florian Westphal +Date: Wed, 6 Jul 2022 16:50:04 +0200 +Subject: netfilter: conntrack: fix crash due to confirmed bit load reordering + +From: Florian Westphal + +commit 0ed8f619b412b52360ccdfaf997223ccd9319569 upstream. + +Kajetan Puchalski reports crash on ARM, with backtrace of: + +__nf_ct_delete_from_lists +nf_ct_delete +early_drop +__nf_conntrack_alloc + +Unlike atomic_inc_not_zero, refcount_inc_not_zero is not a full barrier. +conntrack uses SLAB_TYPESAFE_BY_RCU, i.e. it is possible that a 'newly' +allocated object is still in use on another CPU: + +CPU1 CPU2 + encounter 'ct' during hlist walk + delete_from_lists + refcount drops to 0 + kmem_cache_free(ct); + __nf_conntrack_alloc() // returns same object + refcount_inc_not_zero(ct); /* might fail */ + + /* If set, ct is public/in the hash table */ + test_bit(IPS_CONFIRMED_BIT, &ct->status); + +In case CPU1 already set refcount back to 1, refcount_inc_not_zero() +will succeed. + +The expected possibilities for a CPU that obtained the object 'ct' +(but no reference so far) are: + +1. refcount_inc_not_zero() fails. CPU2 ignores the object and moves to + the next entry in the list. This happens for objects that are about + to be free'd, that have been free'd, or that have been reallocated + by __nf_conntrack_alloc(), but where the refcount has not been + increased back to 1 yet. + +2. refcount_inc_not_zero() succeeds. CPU2 checks the CONFIRMED bit + in ct->status. If set, the object is public/in the table. + + If not, the object must be skipped; CPU2 calls nf_ct_put() to + un-do the refcount increment and moves to the next object. + +Parallel deletion from the hlists is prevented by a +'test_and_set_bit(IPS_DYING_BIT, &ct->status);' check, i.e. only one +cpu will do the unlink, the other one will only drop its reference count. + +Because refcount_inc_not_zero is not a full barrier, CPU2 may try to +delete an object that is not on any list: + +1. refcount_inc_not_zero() successful (refcount inited to 1 on other CPU) +2. CONFIRMED test also successful (load was reordered or zeroing + of ct->status not yet visible) +3. delete_from_lists unlinks entry not on the hlist, because + IPS_DYING_BIT is 0 (already cleared). + +2) is already wrong: CPU2 will handle a partially initited object +that is supposed to be private to CPU1. + +Add needed barriers when refcount_inc_not_zero() is successful. + +It also inserts a smp_wmb() before the refcount is set to 1 during +allocation. + +Because other CPU might still see the object, refcount_set(1) +"resurrects" it, so we need to make sure that other CPUs will also observe +the right content. In particular, the CONFIRMED bit test must only pass +once the object is fully initialised and either in the hash or about to be +inserted (with locks held to delay possible unlink from early_drop or +gc worker). + +I did not change flow_offload_alloc(), as far as I can see it should call +refcount_inc(), not refcount_inc_not_zero(): the ct object is attached to +the skb so its refcount should be >= 1 in all cases. + +v2: prefer smp_acquire__after_ctrl_dep to smp_rmb (Will Deacon). +v3: keep smp_acquire__after_ctrl_dep close to refcount_inc_not_zero call + add comment in nf_conntrack_netlink, no control dependency there + due to locks. + +Cc: Peter Zijlstra +Link: https://lore.kernel.org/all/Yr7WTfd6AVTQkLjI@e126311.manchester.arm.com/ +Reported-by: Kajetan Puchalski +Diagnosed-by: Will Deacon +Fixes: 719774377622 ("netfilter: conntrack: convert to refcount_t api") +Signed-off-by: Florian Westphal +Acked-by: Will Deacon +Signed-off-by: Greg Kroah-Hartman +--- + net/netfilter/nf_conntrack_core.c | 22 ++++++++++++++++++++++ + net/netfilter/nf_conntrack_netlink.c | 1 + + net/netfilter/nf_conntrack_standalone.c | 3 +++ + 3 files changed, 26 insertions(+) + +--- a/net/netfilter/nf_conntrack_core.c ++++ b/net/netfilter/nf_conntrack_core.c +@@ -719,6 +719,9 @@ static void nf_ct_gc_expired(struct nf_c + if (!refcount_inc_not_zero(&ct->ct_general.use)) + return; + ++ /* load ->status after refcount increase */ ++ smp_acquire__after_ctrl_dep(); ++ + if (nf_ct_should_gc(ct)) + nf_ct_kill(ct); + +@@ -785,6 +788,9 @@ __nf_conntrack_find_get(struct net *net, + */ + ct = nf_ct_tuplehash_to_ctrack(h); + if (likely(refcount_inc_not_zero(&ct->ct_general.use))) { ++ /* re-check key after refcount */ ++ smp_acquire__after_ctrl_dep(); ++ + if (likely(nf_ct_key_equal(h, tuple, zone, net))) + goto found; + +@@ -1284,6 +1290,9 @@ static unsigned int early_drop_list(stru + if (!refcount_inc_not_zero(&tmp->ct_general.use)) + continue; + ++ /* load ->ct_net and ->status after refcount increase */ ++ smp_acquire__after_ctrl_dep(); ++ + /* kill only if still in same netns -- might have moved due to + * SLAB_TYPESAFE_BY_RCU rules. + * +@@ -1400,6 +1409,9 @@ static void gc_worker(struct work_struct + if (!refcount_inc_not_zero(&tmp->ct_general.use)) + continue; + ++ /* load ->status after refcount increase */ ++ smp_acquire__after_ctrl_dep(); ++ + if (gc_worker_skip_ct(tmp)) { + nf_ct_put(tmp); + continue; +@@ -1610,6 +1622,16 @@ init_conntrack(struct net *net, struct n + if (!exp) + __nf_ct_try_assign_helper(ct, tmpl, GFP_ATOMIC); + ++ /* Other CPU might have obtained a pointer to this object before it was ++ * released. Because refcount is 0, refcount_inc_not_zero() will fail. ++ * ++ * After refcount_set(1) it will succeed; ensure that zeroing of ++ * ct->status and the correct ct->net pointer are visible; else other ++ * core might observe CONFIRMED bit which means the entry is valid and ++ * in the hash table, but its not (anymore). ++ */ ++ smp_wmb(); ++ + /* Now it is inserted into the unconfirmed list, set refcount to 1. */ + refcount_set(&ct->ct_general.use, 1); + nf_ct_add_to_unconfirmed_list(ct); +--- a/net/netfilter/nf_conntrack_netlink.c ++++ b/net/netfilter/nf_conntrack_netlink.c +@@ -1149,6 +1149,7 @@ restart: + continue; + ct = nf_ct_tuplehash_to_ctrack(h); + if (nf_ct_is_expired(ct)) { ++ /* need to defer nf_ct_kill() until lock is released */ + if (i < ARRAY_SIZE(nf_ct_evict) && + refcount_inc_not_zero(&ct->ct_general.use)) + nf_ct_evict[i++] = ct; +--- a/net/netfilter/nf_conntrack_standalone.c ++++ b/net/netfilter/nf_conntrack_standalone.c +@@ -303,6 +303,9 @@ static int ct_seq_show(struct seq_file * + if (unlikely(!refcount_inc_not_zero(&ct->ct_general.use))) + return 0; + ++ /* load ->status after refcount increase */ ++ smp_acquire__after_ctrl_dep(); ++ + if (nf_ct_should_gc(ct)) { + nf_ct_kill(ct); + goto release; diff --git a/queue-5.10/series b/queue-5.10/series index b150d3f932..cead5efa1c 100644 --- a/queue-5.10/series +++ b/queue-5.10/series @@ -223,3 +223,5 @@ jfs-fix-slab-out-of-bounds-read-in-ea_get.patch jfs-add-index-corruption-check-to-dt_getpage.patch nfsd-put-dl_stid-if-fail-to-queue-dl_recall.patch nfsd-skip-sending-cb_recall_any-when-the-backchannel-isn-t-up.patch +netfilter-conntrack-fix-crash-due-to-confirmed-bit-load-reordering.patch +x86-kexec-fix-double-free-of-elf-header-buffer.patch diff --git a/queue-5.10/x86-kexec-fix-double-free-of-elf-header-buffer.patch b/queue-5.10/x86-kexec-fix-double-free-of-elf-header-buffer.patch new file mode 100644 index 0000000000..f528e7c4c7 --- /dev/null +++ b/queue-5.10/x86-kexec-fix-double-free-of-elf-header-buffer.patch @@ -0,0 +1,46 @@ +From d00dd2f2645dca04cf399d8fc692f3f69b6dd996 Mon Sep 17 00:00:00 2001 +From: Takashi Iwai +Date: Tue, 22 Nov 2022 12:51:22 +0100 +Subject: x86/kexec: Fix double-free of elf header buffer + +From: Takashi Iwai + +commit d00dd2f2645dca04cf399d8fc692f3f69b6dd996 upstream. + +After + + b3e34a47f989 ("x86/kexec: fix memory leak of elf header buffer"), + +freeing image->elf_headers in the error path of crash_load_segments() +is not needed because kimage_file_post_load_cleanup() will take +care of that later. And not clearing it could result in a double-free. + +Drop the superfluous vfree() call at the error path of +crash_load_segments(). + +Fixes: b3e34a47f989 ("x86/kexec: fix memory leak of elf header buffer") +Signed-off-by: Takashi Iwai +Signed-off-by: Borislav Petkov (AMD) +Acked-by: Baoquan He +Acked-by: Vlastimil Babka +Cc: +Link: https://lore.kernel.org/r/20221122115122.13937-1-tiwai@suse.de +Signed-off-by: Greg Kroah-Hartman +--- + arch/x86/kernel/crash.c | 4 +--- + 1 file changed, 1 insertion(+), 3 deletions(-) + +--- a/arch/x86/kernel/crash.c ++++ b/arch/x86/kernel/crash.c +@@ -399,10 +399,8 @@ int crash_load_segments(struct kimage *i + kbuf.buf_align = ELF_CORE_HEADER_ALIGN; + kbuf.mem = KEXEC_BUF_MEM_UNKNOWN; + ret = kexec_add_buffer(&kbuf); +- if (ret) { +- vfree((void *)image->arch.elf_headers); ++ if (ret) + return ret; +- } + image->arch.elf_load_addr = kbuf.mem; + pr_debug("Loaded ELF headers at 0x%lx bufsz=0x%lx memsz=0x%lx\n", + image->arch.elf_load_addr, kbuf.bufsz, kbuf.bufsz);