Skip to content

[LTS 8.8] Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm #41

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

pvts-mat
Copy link
Contributor

@pvts-mat pvts-mat commented Jan 6, 2025

jira VULN-206
cve CVE-2022-42896
commit-author Marcin Wcisło [email protected]
commit f937b75

Solution

The bug fix in the mainline is provided in two commits:

  • f937b758a188d6fd328a81367087eddbb2fce50f
  • 711f8c3fb3db61897080468586b970c87c61d9e4

Of these the 711f8c3 is already applied on ciqlts8_8.

Build

Kernel built on Rocky 9 machine with

CVE=CVE-2022-42896 ./ninja.sh _compile_ciqlts8_8-CVE-2022-42896

from the https://gitlab.conclusive.pl/devices/rocky-patching project. This translates to the environment of Rocky-8-GenericCloud-Base-8.8-20230518.0.x86_64.qcow2 base image ran with install_build_machine.sh script and customized with cloud-init user data file user-data-ciqlts8_8.yml, with the compilation itself done with compile-kernel.sh, install-kernel.sh, set-default-kernel.sh scripts run in succession.

kABI check: passed

kABI ran with

python3 /mnt/code/kernel-dist-git/SOURCES/check-kabi \
        -k /mnt/code/kernel-dist-git/SOURCES/Module.kabi_$(uname -m) \
        -s /mnt/build_files/kernel-src-tree-ciqlts8_8-CVE-2022-42896/Module.symvers

For /mnt/code/kernel-dist-git repo on el-8.8 branch (3120f78031909f15cb48295d2de8d020fd9ec1d7)

Boot test: passed

boot-test.log

Kselftests (partial): passed

Kselftests ran with

modprobe bluetooth
/usr/libexec/kselftests/run_kselftest.sh --summary $($(echo sed $(for t in net:xfrm_policy.sh net:ip_defrag.sh net:gro.sh net/mptcp:simult_flows.sh vm:run_vmtests.sh; do echo ${t}; done | sed -e 's|/|\\/|g' | awk '{print "-e /^" $1 "$/d"}')) /usr/libexec/kselftests/kselftest-list.txt | awk '{print "--test " $1}') 2>&1 | tee summary.log

(This runs all selftests provided by kernel-selftests-internal package as they would be ran by

/usr/libexec/kselftests/run_kselftest.sh --summary

except net:xfrm_policy.sh, net:ip_defrag.sh, net:gro.sh, net/mptcp:simult_flows.sh, vm:run_vmtests.sh. The baseline for these tests could not have been established as the results were indeterministic. To be done)

Results:

kselftests-ciqlts8_8-CVE-2022-42896.summary.log
kselftests-ciqlts8_8-CVE-2022-42896.full.log

Reference results for the ciqlts8_8 (369d0f6e5) version:

kselftests-ciqlts8_8.summary.log
kselftests-ciqlts8_8.full.log

Additional tests: to be added on request

A proof of concept is given in GHSA-pf87-6c9q-jvm4. It was not replicated.

Copy link

@gvrose8192 gvrose8192 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems very straightforward. LGTM - Thanks!

Copy link
Collaborator

@bmastbergen bmastbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is f937b75 being added just because it came in the same series as 711f8c3 ? I don't see any indication that f937b75 is an actual fix for CVE-2022-42896. I don't think it hurts to add it, I am just wondering if I missed something

@pvts-mat
Copy link
Contributor Author

pvts-mat commented Jan 7, 2025

Good point, @bmastbergen. I based the fix on this statement from GHSA-pf87-6c9q-jvm4

The vulnerability was fixed by not accepting 0 as a valid PSM value in commit 711f8c3 and by preventing l2cap_global_chan_by_psm to give back L2CAP_CHAN_FIXED channels in commit f937b75.

It's therefore a form of appeal to authority (of Google), so it's less than ideal. The ideal would be to grok and replicate the exploit POC given there, which falls under the "Additional tests" that were not done, as admitted in the PR. Then I would be able to answer you precisely what role the f937b75 commit plays in fixing the bug. Please let me know @PlaidCat whether I should continue working on POC for this CVE.

@PlaidCat
Copy link
Collaborator

PlaidCat commented Jan 7, 2025

I am just now getting to this, I will let you know when I'm done looking at all the reviews

@PlaidCat
Copy link
Collaborator

PlaidCat commented Jan 7, 2025

Ok this is my take on applicability.

@bmastbergen You are correct its not included in the "official" CVE numbering authorities:

Major RPM EL distros also refer to the MITRE/NIST data

HOWEVER our friends over at Debian have actually read the Google Advisory and disclosure as @pvts-mat did.

This is one of the things I hate about prior to Feb-2024 Kernel CVEs, when Kernel.org became the CNA authority. I suspect as a part of the CVE process this specific commit fell out due to not meeting specific criteria, wasn't tagged with CVE in the commit message like the previousc commit 711f8c3 (which is super unusual to begin with).
I suspect the amount of spelunking to figure this out is not worth the effort.

In this case I think this is a condition of us trying to hold a Higher Bar and following the advisory and Debian model

Proof Of Concept from Google

@pvts-mat I do not believe you need to spend any more than a couple hours at best (which sounds like you already did). Depending on the system and its configuration that google worked with it may be difficult to reproduce and I'm not sure if VMs will provide all the details to emulate bluetooth or pass through or what have you.
I appreciate the effort though.

Requested Changes to the Commit

There are some changes needed to be done to the Commit message itself (github does not let me comment on commit message ... because of course we never need to comment on those). I see its missing an assumed piece as well, this is my mistake and gap.

So @pvts-mat you will be the committer to achieve this you have to use git cherry-pick -nsx <upstream sha>.

Additionally the commit-author is intended to be the upstream commit author

Finally I do not have it written down in the wiki so I will get that updated.

We indent all the emails at the bottom to make sure if email bots (originally brought to our attention by a former RedHat Kernel engineer) so that people don't get mass bombarded from a kernel tree they likely do not care about.

I know this wasn't available when you started but we have opened up some of our tooling and a goal I have is that we in CIQ transition all of our tools that are not specific to CIQ internal needs be from our internal tooling repo to this repo:
https://github.com/ctrliq/kernel-src-tree-tools

In there is ciq-cherry-pick which will do much of the setup and formatting for you.

Original Commit Message from your tree

git log

commit 39e65d4df845983fa17420d16e1f34e28d40b001 (HEAD -> ciqlts8_8-CVE-2022-42896, origin/ciqlts8_8-CVE-2022-42896)
Author: Luiz Augusto von Dentz <[email protected]>
Date:   Mon Oct 31 16:10:33 2022 -0700

    Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm

    jira VULN-206
    cve CVE-2022-42896
    commit-author Marcin Wcisło <[email protected]>
    commit f937b758a188d6fd328a81367087eddbb2fce50f

    l2cap_global_chan_by_psm shall not return fixed channels as they are not
    meant to be connected by (S)PSM.

    Signed-off-by: Luiz Augusto von Dentz <[email protected]>
    Reviewed-by: Tedd Ho-Jeong An <[email protected]>

Proposed change to Commit message.

commit 39e65d4df845983fa17420d16e1f34e28d40b001 (HEAD -> ciqlts8_8-CVE-2022-42896, origin/ciqlts8_8-CVE-2022-42896)
Author: Marcin Wcisło <[email protected]>
Date:   <when you did the backport>

    Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm

    jira VULN-206
    cve CVE-2022-42896
    commit-author Luiz Augusto von Dentz <[email protected]>
    commit f937b758a188d6fd328a81367087eddbb2fce50f

    l2cap_global_chan_by_psm shall not return fixed channels as they are not
    meant to be connected by (S)PSM.

        Signed-off-by: Luiz Augusto von Dentz <[email protected]>
        Reviewed-by: Tedd Ho-Jeong An <[email protected]>

^notice the "tab" indent

Copy link
Collaborator

@PlaidCat PlaidCat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See previous comment

@bmastbergen
Copy link
Collaborator

Good point, @bmastbergen. I based the fix on this statement from GHSA-pf87-6c9q-jvm4

The vulnerability was fixed by not accepting 0 as a valid PSM value in commit 711f8c3 and by preventing l2cap_global_chan_by_psm to give back L2CAP_CHAN_FIXED channels in commit f937b75.

It's therefore a form of appeal to authority (of Google), so it's less than ideal. The ideal would be to grok and replicate the exploit POC given there, which falls under the "Additional tests" that were not done, as admitted in the PR. Then I would be able to answer you precisely what role the f937b75 commit plays in fixing the bug. Please let me know @PlaidCat whether I should continue working on POC for this CVE.

@pvts-mat Ah, so I was missing something! Thanks for pointing that out (and good find). With the commit message fixups requested by @PlaidCat this should be good to go.

@pvts-mat pvts-mat force-pushed the ciqlts8_8-CVE-2022-42896 branch from 39e65d4 to 5249fcf Compare January 7, 2025 20:50
Copy link
Collaborator

@PlaidCat PlaidCat left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

The changes in the Commits tab and your forked repo look good for the changes to the commit message.

Thanks for this.

Copy link
Collaborator

@bmastbergen bmastbergen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🥌

@PlaidCat PlaidCat changed the title Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm [LTS 8.8] Bluetooth: L2CAP: Fix l2cap_global_chan_by_psm Jan 15, 2025
jira VULN-206
cve CVE-2022-42896
commit-author Luiz Augusto von Dentz <[email protected]>
commit f937b75

l2cap_global_chan_by_psm shall not return fixed channels as they are not
meant to be connected by (S)PSM.

	Signed-off-by: Luiz Augusto von Dentz <[email protected]>
	Reviewed-by: Tedd Ho-Jeong An <[email protected]>
(cherry picked from commit f937b75)
	Signed-off-by: Marcin Wcisło <[email protected]>
@pvts-mat pvts-mat force-pushed the ciqlts8_8-CVE-2022-42896 branch from 5249fcf to 28cccf1 Compare January 15, 2025 23:16
@PlaidCat
Copy link
Collaborator

Tests succeeded after pulling the latest ciqlts8_8 so I'm going to merge.

@PlaidCat PlaidCat merged commit 4d9ae9c into ctrliq:ciqlts8_8 Jan 16, 2025
2 checks passed
PlaidCat pushed a commit that referenced this pull request Apr 2, 2025
commit af288a4 upstream.

Commit b15c872 ("hwpoison, memory_hotplug: allow hwpoisoned pages to
be offlined) add page poison checks in do_migrate_range in order to make
offline hwpoisoned page possible by introducing isolate_lru_page and
try_to_unmap for hwpoisoned page.  However folio lock must be held before
calling try_to_unmap.  Add it to fix this problem.

Warning will be produced if folio is not locked during unmap:

  ------------[ cut here ]------------
  kernel BUG at ./include/linux/swapops.h:400!
  Internal error: Oops - BUG: 00000000f2000800 [#1] PREEMPT SMP
  Modules linked in:
  CPU: 4 UID: 0 PID: 411 Comm: bash Tainted: G        W          6.13.0-rc1-00016-g3c434c7ee82a-dirty #41
  Tainted: [W]=WARN
  Hardware name: QEMU QEMU Virtual Machine, BIOS 0.0.0 02/06/2015
  pstate: 40400005 (nZcv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--)
  pc : try_to_unmap_one+0xb08/0xd3c
  lr : try_to_unmap_one+0x3dc/0xd3c
  Call trace:
   try_to_unmap_one+0xb08/0xd3c (P)
   try_to_unmap_one+0x3dc/0xd3c (L)
   rmap_walk_anon+0xdc/0x1f8
   rmap_walk+0x3c/0x58
   try_to_unmap+0x88/0x90
   unmap_poisoned_folio+0x30/0xa8
   do_migrate_range+0x4a0/0x568
   offline_pages+0x5a4/0x670
   memory_block_action+0x17c/0x374
   memory_subsys_offline+0x3c/0x78
   device_offline+0xa4/0xd0
   state_store+0x8c/0xf0
   dev_attr_store+0x18/0x2c
   sysfs_kf_write+0x44/0x54
   kernfs_fop_write_iter+0x118/0x1a8
   vfs_write+0x3a8/0x4bc
   ksys_write+0x6c/0xf8
   __arm64_sys_write+0x1c/0x28
   invoke_syscall+0x44/0x100
   el0_svc_common.constprop.0+0x40/0xe0
   do_el0_svc+0x1c/0x28
   el0_svc+0x30/0xd0
   el0t_64_sync_handler+0xc8/0xcc
   el0t_64_sync+0x198/0x19c
  Code: f9407be0 b5fff320 d4210000 17ffff97 (d4210000)
  ---[ end trace 0000000000000000 ]---

Link: https://lkml.kernel.org/r/[email protected]
Fixes: b15c872 ("hwpoison, memory_hotplug: allow hwpoisoned pages to be offlined")
Signed-off-by: Ma Wupeng <[email protected]>
Acked-by: David Hildenbrand <[email protected]>
Acked-by: Miaohe Lin <[email protected]>
Cc: Michal Hocko <[email protected]>
Cc: Naoya Horiguchi <[email protected]>
Cc: Oscar Salvador <[email protected]>
Cc: <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
github-actions bot pushed a commit that referenced this pull request Apr 30, 2025
Commit ddd0a42 only increments scomp_scratch_users when it was 0,
causing a panic when using ipcomp:

    Oops: general protection fault, probably for non-canonical address 0xdffffc0000000000: 0000 [#1] SMP KASAN NOPTI
    KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
    CPU: 1 UID: 0 PID: 619 Comm: ping Tainted: G                 N  6.15.0-rc3-net-00032-ga79be02bba5c #41 PREEMPT(full)
    Tainted: [N]=TEST
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Arch Linux 1.16.3-1-1 04/01/2014
    RIP: 0010:inflate_fast+0x5a2/0x1b90
    [...]
    Call Trace:
     <IRQ>
     zlib_inflate+0x2d60/0x6620
     deflate_sdecompress+0x166/0x350
     scomp_acomp_comp_decomp+0x45f/0xa10
     scomp_acomp_decompress+0x21/0x120
     acomp_do_req_chain+0x3e5/0x4e0
     ipcomp_input+0x212/0x550
     xfrm_input+0x2de2/0x72f0
    [...]
    Kernel panic - not syncing: Fatal exception in interrupt
    Kernel Offset: disabled
    ---[ end Kernel panic - not syncing: Fatal exception in interrupt ]---

Instead, let's keep the old increment, and decrement back to 0 if the
scratch allocation fails.

Fixes: ddd0a42 ("crypto: scompress - Fix scratch allocation failure handling")
Signed-off-by: Sabrina Dubroca <[email protected]>
Signed-off-by: Herbert Xu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants