[LTS 9.2] netfilter: CVE-2024-0607, CVE-2024-0193, CVE-2024-42109, CVE-2024-54031 + non-CVE bugfixes #794
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jira VULN-42268 subsystem-update centos-stream-9 f875124 cve CVE-2024-0607 commit-author Florian Westphal <[email protected]> commit c301f09 upstream-diff ciqlts9_4 backport e7fce92 used for clean cherry-pick netfilter: nf_tables: fix pointer math issue in nft_byteorder_eval() The problem is in nft_byteorder_eval() where we are iterating through a loop and writing to dst[0], dst[1], dst[2] and so on... On each iteration we are writing 8 bytes. But dst[] is an array of u32 so each element only has space for 4 bytes. That means that every iteration overwrites part of the previous element. I spotted this bug while reviewing commit caf3ef7 ("netfilter: nf_tables: prevent OOB access in nft_byteorder_eval") which is a related issue. I think that the reason we have not detected this bug in testing is that most of time we only write one element. Fixes: ce1e798 ("netfilter: nft_byteorder: provide 64bit le/be conversion") Signed-off-by: Dan Carpenter <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit e7fce92) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-6825 subsystem-update centos-stream-9 f875124 cve CVE-2024-0193 commit-author Pablo Neira Ayuso <[email protected]> commit 7315dc1 NFT_MSG_DELSET deactivates all elements in the set, skip set->ops->commit() to avoid the unnecessary clone (for the pipapo case) as well as the sync GC cycle, which could deactivate again expired elements in such set. Fixes: 5f68718 ("netfilter: nf_tables: GC transaction API to avoid race with control plane") Reported-by: Kevin Rich <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 7315dc1) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-430 subsystem-update centos-stream-9 f875124 cve-bf CVE-2023-4244 commit-author Florian Westphal <[email protected]> commit 08e4c8c If a transaction is aborted, we should mark the to-be-released NEWSET dead, just like commit path does for DEL and DESTROYSET commands. In both cases all remaining elements will be released via set->ops->destroy(). The existing abort code does NOT post the actual release to the work queue. Also the entire __nf_tables_abort() function is wrapped in gc_seq begin/end pair. Therefore, async gc worker will never try to release the pending set elements, as gc sequence is always stale. It might be possible to speed up transaction aborts via work queue too, this would result in a race and a possible use-after-free. So fix this before it becomes an issue. Fixes: 5f68718 ("netfilter: nf_tables: GC transaction API to avoid race with control plane") Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 08e4c8c) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-158865 subsystem-update centos-stream-9 f875124 cve-bf CVE-2023-52923 commit-author Florian Westphal <[email protected]> commit ffb40fb No need to use GFP_ATOMIC here. Fixes: f6c383b ("netfilter: nf_tables: adapt set backend to use GC transaction API") Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit ffb40fb) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-430 cve-bf CVE-2023-4244 commit-author Pablo Neira Ayuso <[email protected]> commit 6b1ca88 Delete from packet path relies on the garbage collector to purge elements with NFT_SET_ELEM_DEAD_BIT on. Skip these dead elements from nf_tables_dump_setelem() path, I very rarely see tests/shell/testcases/maps/typeof_maps_add_delete reports [DUMP FAILED] showing a mismatch in the expected output with an element that should not be there. If the netlink dump happens before GC worker run, it might show dead elements in the ruleset listing. nft_rhash_get() already skips dead elements in nft_rhash_cmp(), therefore, it already does not show the element when getting a single element via netlink control plane. Fixes: 5f68718 ("netfilter: nf_tables: GC transaction API to avoid race with control plane") Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 6b1ca88) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-430 cve-bf CVE-2023-4244 commit-author Pablo Neira Ayuso <[email protected]> commit ab0beaf This has slipped through when reducing memory footprint for set elements, remove it. Fixes: 9dad402 ("netfilter: nf_tables: expose opaque set element as struct nft_elem_priv") Reported-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit ab0beaf) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-44483 cve CVE-2024-42109 commit-author Florian Westphal <[email protected]> commit 9f6958b syzbot reports: KASAN: slab-uaf in nft_ctx_update include/net/netfilter/nf_tables.h:1831 KASAN: slab-uaf in nft_commit_release net/netfilter/nf_tables_api.c:9530 KASAN: slab-uaf int nf_tables_trans_destroy_work+0x152b/0x1750 net/netfilter/nf_tables_api.c:9597 Read of size 2 at addr ffff88802b0051c4 by task kworker/1:1/45 [..] Workqueue: events nf_tables_trans_destroy_work Call Trace: nft_ctx_update include/net/netfilter/nf_tables.h:1831 [inline] nft_commit_release net/netfilter/nf_tables_api.c:9530 [inline] nf_tables_trans_destroy_work+0x152b/0x1750 net/netfilter/nf_tables_api.c:9597 Problem is that the notifier does a conditional flush, but its possible that the table-to-be-removed is still referenced by transactions being processed by the worker, so we need to flush unconditionally. We could make the flush_work depend on whether we found a table to delete in nf-next to avoid the flush for most cases. AFAICS this problem is only exposed in nf-next, with commit e169285 ("netfilter: nf_tables: do not store nft_ctx in transaction objects"), with this commit applied there is an unconditional fetch of table->family which is whats triggering the above splat. Fixes: 2c9f029 ("netfilter: nf_tables: flush pending destroy work before netlink notifier") Reported-and-tested-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=4fd66a69358fc15ae2ad Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 9f6958b) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-44483 cve-bf CVE-2024-42109 commit-author Florian Westphal <[email protected]> commit fb82865 upstream-diff Context conflicts only The call to flush_work before tearing down a table from the netlink notifier was supposed to make sure that all earlier updates (e.g. rule add) that might reference that table have been processed. Unfortunately, flush_work() waits for the last queued instance. This could be an instance that is different from the one that we must wait for. This is because transactions are protected with a pernet mutex, but the work item is global, so holding the transaction mutex doesn't prevent another netns from queueing more work. Make the work item pernet so that flush_work() will wait for all transactions queued from this netns. A welcome side effect is that we no longer need to wait for transaction objects from foreign netns. The gc work queue is still global. This seems to be ok because nft_set structures are reference counted and each container structure owns a reference on the net namespace. The destroy_list is still protected by a global spinlock rather than pernet one but the hold time is very short anyway. v2: call cancel_work_sync before reaping the remaining tables (Pablo). Fixes: 9f6958b ("netfilter: nf_tables: unconditionally flush pending work before notifier") Reported-by: [email protected] Signed-off-by: Florian Westphal <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit fb82865) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-158865 cve-bf CVE-2023-52923 commit-author Pablo Neira Ayuso <[email protected]> commit 7ffc748 rhashtable does not provide stable walk, duplicated elements are possible in case of resizing. I considered that checking for errors when calling rhashtable_walk_next() was sufficient to detect the resizing. However, rhashtable_walk_next() returns -EAGAIN only at the end of the iteration, which is too late, because a gc work containing duplicated elements could have been already scheduled for removal to the worker. Add a u32 gc worker sequence number per set, bump it on every workqueue run. Annotate gc worker sequence number on the expired element. Use it to skip those already seen in this gc workqueue run. Note that this new field is never reset in case gc transaction fails, so next gc worker run on the expired element overrides it. Wraparound of gc worker sequence number should not be an issue with stale gc worker sequence number in the element, that would just postpone the element removal in one gc run. Note that it is not possible to use flags to annotate that element is pending gc run to detect duplicates, given that gc transaction can be invalidated in case of update from the control plane, therefore, not allowing to clear such flag. On x86_64, pahole reports no changes in the size of nft_rhash_elem. Fixes: f6c383b ("netfilter: nf_tables: adapt set backend to use GC transaction API") Reported-by: Laurent Fasnacht <[email protected]> Tested-by: Laurent Fasnacht <[email protected]> Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 7ffc748) Signed-off-by: Marcin Wcisło <[email protected]>
jira VULN-5468 cve CVE-2024-54031 commit-author Pablo Neira Ayuso <[email protected]> commit 542ed81 Access to genmask field in struct nft_set_ext results in unaligned atomic read: [ 72.130109] Unable to handle kernel paging request at virtual address ffff0000c2bb708c [ 72.131036] Mem abort info: [ 72.131213] ESR = 0x0000000096000021 [ 72.131446] EC = 0x25: DABT (current EL), IL = 32 bits [ 72.132209] SET = 0, FnV = 0 [ 72.133216] EA = 0, S1PTW = 0 [ 72.134080] FSC = 0x21: alignment fault [ 72.135593] Data abort info: [ 72.137194] ISV = 0, ISS = 0x00000021, ISS2 = 0x00000000 [ 72.142351] CM = 0, WnR = 0, TnD = 0, TagAccess = 0 [ 72.145989] GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0 [ 72.150115] swapper pgtable: 4k pages, 48-bit VAs, pgdp=0000000237d27000 [ 72.154893] [ffff0000c2bb708c] pgd=0000000000000000, p4d=180000023ffff403, pud=180000023f84b403, pmd=180000023f835403, +pte=0068000102bb7707 [ 72.163021] Internal error: Oops: 0000000096000021 [ctrliq#1] SMP [...] [ 72.170041] CPU: 7 UID: 0 PID: 54 Comm: kworker/7:0 Tainted: G E 6.13.0-rc3+ ctrliq#2 [ 72.170509] Tainted: [E]=UNSIGNED_MODULE [ 72.170720] Hardware name: QEMU QEMU Virtual Machine, BIOS edk2-stable202302-for-qemu 03/01/2023 [ 72.171192] Workqueue: events_power_efficient nft_rhash_gc [nf_tables] [ 72.171552] pstate: 21400005 (nzCv daif +PAN -UAO -TCO +DIT -SSBS BTYPE=--) [ 72.171915] pc : nft_rhash_gc+0x200/0x2d8 [nf_tables] [ 72.172166] lr : nft_rhash_gc+0x128/0x2d8 [nf_tables] [ 72.172546] sp : ffff800081f2bce0 [ 72.172724] x29: ffff800081f2bd40 x28: ffff0000c2bb708c x27: 0000000000000038 [ 72.173078] x26: ffff0000c6780ef0 x25: ffff0000c643df00 x24: ffff0000c6778f78 [ 72.173431] x23: 000000000000001a x22: ffff0000c4b1f000 x21: ffff0000c6780f78 [ 72.173782] x20: ffff0000c2bb70dc x19: ffff0000c2bb7080 x18: 0000000000000000 [ 72.174135] x17: ffff0000c0a4e1c0 x16: 0000000000003000 x15: 0000ac26d173b978 [ 72.174485] x14: ffffffffffffffff x13: 0000000000000030 x12: ffff0000c6780ef0 [ 72.174841] x11: 0000000000000000 x10: ffff800081f2bcf8 x9 : ffff0000c3000000 [ 72.175193] x8 : 00000000000004be x7 : 0000000000000000 x6 : 0000000000000000 [ 72.175544] x5 : 0000000000000040 x4 : ffff0000c3000010 x3 : 0000000000000000 [ 72.175871] x2 : 0000000000003a98 x1 : ffff0000c2bb708c x0 : 0000000000000004 [ 72.176207] Call trace: [ 72.176316] nft_rhash_gc+0x200/0x2d8 [nf_tables] (P) [ 72.176653] process_one_work+0x178/0x3d0 [ 72.176831] worker_thread+0x200/0x3f0 [ 72.176995] kthread+0xe8/0xf8 [ 72.177130] ret_from_fork+0x10/0x20 [ 72.177289] Code: 54fff984 d503201f d2800080 91003261 (f820303f) [ 72.177557] ---[ end trace 0000000000000000 ]--- Align struct nft_set_ext to word size to address this and documentation it. pahole reports that this increases the size of elements for rhash and pipapo in 8 bytes on x86_64. Fixes: 7ffc748 ("netfilter: nft_set_hash: skip duplicated elements pending gc run") Signed-off-by: Pablo Neira Ayuso <[email protected]> (cherry picked from commit 542ed81) Signed-off-by: Marcin Wcisło <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
CVE-2024-0607 VULN-42268
CVE-2024-0193 VULN-6825
CVE-2024-42109 VULN-44483
CVE-2024-54031 VULN-5468
About
This PR aims to supplement the netfilter patch set #668 regarding the omitted bugfixes.
Unlike in the previous PR the CentOS 9 branches were not backported in full, instead just single commits were picked addressing specific issues. This approach was chosen because the main branches of concern - f875124, 3e3b830 - were loosely coupled and none of the picks required prerequisites.
Previous netfilter PR bugfixes
The following list follows the table from comment #668 (comment) thus indirectly addressing all
[FIXES]warnings indicated in #668 (comment).8daa8fd
netfilter: nf_tables: Introduce NFT_MSG_GETRULE_RESETThis commit was eventually omitted in the second revision of the PR, so the fix doesn't apply.
f80a612
netfilter: nf_tables: add support to destroy operationThis commit was eventually omitted in the second revision of the PR, so the fix doesn't apply.
079cd63
netfilter: nf_tables: Introduce NFT_MSG_GETSETELEM_RESETThis commit was eventually omitted in the second revision of the PR, so the fixes don't apply.
212ed75
netfilter: nf_tables: integrate pipapo into commit protocolThe fixing commit ebd032f was later reverted in the upstream with f86fb94. The fix may therefore be considered void.
2b84e21
netfilter: nft_set_pipapo: .walk does not deal with generationsThis fix will be covered in a separate PR along with the CVE-2024-27012 fix. See below.
628bd3e
netfilter: nf_tables: drop map element references from preparation phaseThe fix requires extensive adaptations to
ciqlts9_2which could not have been avoided with a reasonable number of prerequisites. It was decided to leave it for a separate PR.5f68718
netfilter: nf_tables: GC transaction API to avoid race with control planeAll fixes were included in the PR.
f6c383b
netfilter: nf_tables: adapt set backend to use GC transaction APIAll fixes were included in the PR.
The bugfix of
netfilter: nft_set_hash: skip duplicated elements pending gc run, with a separate CVE:2c9f029
netfilter: nf_tables: flush pending destroy work before netlink notifierThe fix was included in the PR.
9dad402
netfilter: nf_tables: expose opaque set element as struct nft_elem_privThe fix was included in the PR.
Additional fixes
Additional fix was included, which doesn't address any specific commit from the netfilter PR, but has CVE-2024-0607 assigned and is part of the CentOS 9 branch f875124 being (partially) backported here with the fixes 7315dc1, 08e4c8c and ffb40fb.
kABI check: passed
Boot test: passed
boot-test.log
Kselftests: passed
Reference
kselftests–ciqlts9_2–run1.log
kselftests–ciqlts9_2–run2.log
Patch
kselftests–ciqlts9_2-CVE-batch-14–run1.log
kselftests–ciqlts9_2-CVE-batch-14–run2.log
kselftests–ciqlts9_2-CVE-batch-14–run3.log
kselftests–ciqlts9_2-CVE-batch-14–run4.log
kselftests–ciqlts9_2-CVE-batch-14–run5.log
Comparison
The tests results for the reference and the patch are the same.