Skip to content

[fips-legacy-8] perf: Disallow mis-matched inherited group reads #472

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Aug 8, 2025
Merged

[fips-legacy-8] perf: Disallow mis-matched inherited group reads #472

merged 2 commits into from
Aug 8, 2025
+37 −6

Conversation

shreeya-patel98
Copy link

  • Commit Message Requirements
  • Built against Vault/LTS Environment
  • kABI Check Passed, where Valid (Pre 9.4 RT does not have kABI stability)
  • Boot Test
  • Kernel SelfTest results
  • Additional Tests as determined relevant

Commit message

jira VULN-8890
cve CVE-2023-5717
commit-author Peter Zijlstra <[email protected]>
commit https://github.com/ctrliq/kernel-src-tree/commit/32671e3799ca2e4590773fd0e63aaa4229e50c06
upstream-diff This patch causes kABI breakage due to a change in the
struct perf_event layout after adding the group_generation field.
Hence, to preserve kABI compatibility, use RH_KABI_EXTEND macro
to safely append the new field without affecting the existing layout.

Because group consistency is non-atomic between parent (filedesc) and children
(inherited) events, it is possible for PERF_FORMAT_GROUP read() to try and sum
non-matching counter groups -- with non-sensical results.

Add group_generation to distinguish the case where a parent group removes and
adds an event and thus has the same number, but a different configuration of
events as inherited groups.

This became a problem when commit https://github.com/ctrliq/kernel-src-tree/commit/fa8c269353d560b7c28119ad7617029f92e40b15 ("perf/core: Invert
perf_read_group() loops") flipped the order of child_list and sibling_list.
Previously it would iterate the group (sibling_list) first, and for each
sibling traverse the child_list. In this order, only the group composition of
the parent is relevant. By flipping the order the group composition of the
child (inherited) events becomes an issue and the mis-match in group
composition becomes evident.

That said; even prior to this commit, while reading of a group that is not
equally inherited was not broken, it still made no sense.

(Ab)use ECHILD as error return to indicate issues with child process group
composition.

Fixes: https://github.com/ctrliq/kernel-src-tree/commit/fa8c269353d560b7c28119ad7617029f92e40b15 ("perf/core: Invert perf_read_group() loops")
	Reported-by: Budimir Markovic <[email protected]>
	Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
(cherry picked from commit https://github.com/ctrliq/kernel-src-tree/commit/32671e3799ca2e4590773fd0e63aaa4229e50c06)
	Signed-off-by: Shreeya Patel <[email protected]>
-------------------------------------------------------------------------------------------------------
perf/core: Fix potential NULL deref
jira VULN-8890
cve CVE-2023-5717
commit-author Peter Zijlstra <[email protected]>
commit https://github.com/ctrliq/kernel-src-tree/commit/a71ef31485bb51b846e8db8b3a35e432cc15afb5

Smatch is awesome.

Fixes: https://github.com/ctrliq/kernel-src-tree/commit/32671e3799ca2e4590773fd0e63aaa4229e50c06 ("perf: Disallow mis-matched inherited group reads")
	Reported-by: Dan Carpenter <[email protected]>
	Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
	Signed-off-by: Ingo Molnar <[email protected]>
(cherry picked from commit https://github.com/ctrliq/kernel-src-tree/commit/a71ef31485bb51b846e8db8b3a35e432cc15afb5)
	Signed-off-by: Shreeya Patel <[email protected]>

Kernel build logs

/mnt/scratch/workspace/fips-legacy-8-compliant/kernel-src-tree
Skipping make mrproper
[TIMER]{MRPROPER}: 0s
x86_64 architecture detected, copying config
'configs/kernel-x86_64.config' -> '.config'
Setting Local Version for build
CONFIG_LOCALVERSION="-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893"
Making olddefconfig
scripts/kconfig/conf  --olddefconfig Kconfig
#
# configuration written to .config
#
Starting Build
scripts/kconfig/conf  --syncconfig Kconfig
  UPD     include/config/kernel.release
  DESCEND objtool
  DESCEND bpf/resolve_btfids
  UPD     include/generated/utsrelease.h
  CC      arch/x86/kernel/asm-offsets.s
  CALL    scripts/checksyscalls.sh
  CC      arch/x86/ia32/sys_ia32.o
  CC      init/main.o
  CC      arch/x86/entry/common.o
  CC      arch/x86/events/core.o
  CC      arch/x86/hyperv/mmu.o
  CC      arch/x86/events/amd/core.o
  CC      arch/x86/mm/init.o
  CC      arch/x86/events/amd/uncore.o
  CC [M]  arch/x86/kvm/../../../virt/kvm/kvm_main.o
  AR      arch/x86/ia32/built-in.a
  CHK     include/generated/compile.h
  CC      kernel/fork.o
  CC      arch/x86/events/amd/ibs.o
  CC      arch/x86/events/intel/core.o
  CC [M]  arch/x86/kvm/../../../virt/kvm/eventfd.o
  CC      arch/x86/kernel/process_64.o
  AR      arch/x86/hyperv/built-in.a
  CC [M]  arch/x86/kvm/../../../virt/kvm/binary_stats.o
  CC      arch/x86/entry/vsyscall/vsyscall_64.o
  CC [M]  arch/x86/kvm/../../../virt/kvm/vfio.o
  CC      arch/x86/events/amd/iommu.o
  CC      arch/x86/mm/fault.o
  CC      init/do_mounts.o
  <--snip-->
    INSTALL sound/usb/hiface/snd-usb-hiface.ko
  INSTALL sound/usb/line6/snd-usb-line6.ko
  INSTALL sound/usb/line6/snd-usb-podhd.ko
  INSTALL sound/usb/line6/snd-usb-pod.ko
  INSTALL sound/usb/line6/snd-usb-toneport.ko
  INSTALL sound/usb/line6/snd-usb-variax.ko
  INSTALL sound/usb/snd-usb-audio.ko
  INSTALL sound/usb/snd-usbmidi-lib.ko
  INSTALL sound/usb/misc/snd-ua101.ko
  INSTALL sound/usb/usx2y/snd-usb-us122l.ko
  INSTALL sound/usb/usx2y/snd-usb-usx2y.ko
  INSTALL sound/virtio/virtio_snd.ko
  INSTALL sound/x86/snd-hdmi-lpe-audio.ko
  INSTALL sound/xen/snd_xen_front.ko
  INSTALL virt/lib/irqbypass.ko
  DEPMOD  4.18.0-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893+
[TIMER]{MODULES}: 9s
Making Install
sh ./arch/x86/boot/install.sh 4.18.0-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893+ arch/x86/boot/bzImage \
	System.map "/boot"
[TIMER]{INSTALL}: 15s
Checking kABI
kABI check passed
Setting Default Kernel to /boot/vmlinuz-4.18.0-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893+ and Index to 1
The default is /boot/loader/entries/1cf5e9bb44ed480cb5927d321c03acfc-4.18.0-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893+.conf with index 1 and kernel /boot/vmlinuz-4.18.0-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893+
The default is /boot/loader/entries/1cf5e9bb44ed480cb5927d321c03acfc-4.18.0-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893+.conf with index 1 and kernel /boot/vmlinuz-4.18.0-fips-legacy-8-compliant_4.18.0-425.13.1-98daf831f893+
Generating grub configuration file ...
done
Hopefully Grub2.0 took everything ... rebooting after time metrices
[TIMER]{MRPROPER}: 0s
[TIMER]{BUILD}: 387s
[TIMER]{MODULES}: 9s
[TIMER]{INSTALL}: 15s
[TIMER]{TOTAL} 414s
Rebooting in 10 seconds

kernel-build.log

Kselftests

shreeya@spatel-dev-bom:~/ciq/workspace/fips-legacy-8-compliant$ grep '^ok ' kselftest-before.log | wc -l && grep '^ok ' kselftest-after.log | wc -l
204
204
shreeya@spatel-dev-bom:~/ciq/workspace/fips-legacy-8-compliant$ grep '^not ok ' kselftest-before.log | wc -l && grep '^not ok ' kselftest-after.log | wc -l
53
53

kselftest-after.log
kselftest-before.log

@shreeya-patel98
perf: Disallow mis-matched inherited group reads
e38a481 
jira VULN-8890
cve CVE-2023-5717
commit-author Peter Zijlstra <[email protected]>
commit 32671e3
upstream-diff This patch causes kABI breakage due to a change in the
struct perf_event layout after adding the group_generation field.
Hence, to preserve kABI compatibility, use RH_KABI_EXTEND macro
to safely append the new field without affecting the existing layout.

Because group consistency is non-atomic between parent (filedesc) and children
(inherited) events, it is possible for PERF_FORMAT_GROUP read() to try and sum
non-matching counter groups -- with non-sensical results.

Add group_generation to distinguish the case where a parent group removes and
adds an event and thus has the same number, but a different configuration of
events as inherited groups.

This became a problem when commit fa8c269 ("perf/core: Invert
perf_read_group() loops") flipped the order of child_list and sibling_list.
Previously it would iterate the group (sibling_list) first, and for each
sibling traverse the child_list. In this order, only the group composition of
the parent is relevant. By flipping the order the group composition of the
child (inherited) events becomes an issue and the mis-match in group
composition becomes evident.

That said; even prior to this commit, while reading of a group that is not
equally inherited was not broken, it still made no sense.

(Ab)use ECHILD as error return to indicate issues with child process group
composition.

Fixes: fa8c269 ("perf/core: Invert perf_read_group() loops")
	Reported-by: Budimir Markovic <[email protected]>
	Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Link: https://lkml.kernel.org/r/[email protected]
(cherry picked from commit 32671e3)
	Signed-off-by: Shreeya Patel <[email protected]>
@shreeya-patel98
perf/core: Fix potential NULL deref
98daf83 
jira VULN-8890
cve-bf CVE-2023-5717
commit-author Peter Zijlstra <[email protected]>
commit a71ef31

Smatch is awesome.

Fixes: 32671e3 ("perf: Disallow mis-matched inherited group reads")
	Reported-by: Dan Carpenter <[email protected]>
	Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
	Signed-off-by: Ingo Molnar <[email protected]>
(cherry picked from commit a71ef31)
	Signed-off-by: Shreeya Patel <[email protected]>
@shreeya-patel98 shreeya-patel98 self-assigned this Aug 7, 2025
bmastbergen
bmastbergen approved these changes Aug 8, 2025
View reviewed changes
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.

🥌

thefossguy-ciq
thefossguy-ciq approved these changes Aug 8, 2025
View reviewed changes
Copy link

@thefossguy-ciq thefossguy-ciq 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 approved these changes Aug 8, 2025
View reviewed changes
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:

@shreeya-patel98 shreeya-patel98 merged commit b5a9628 into fips-legacy-8-compliant/4.18.0-425.13.1 Aug 8, 2025
1 check passed
@shreeya-patel98 shreeya-patel98 deleted the {shreeya}_fips-legacy-8-compliant/4.18.0-425.13.1 branch August 8, 2025 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PlaidCat PlaidCat PlaidCat approved these changes

@bmastbergen bmastbergen bmastbergen approved these changes

@thefossguy-ciq thefossguy-ciq thefossguy-ciq approved these changes

@jainanmol84 jainanmol84 Awaiting requested review from jainanmol84

Assignees

@shreeya-patel98 shreeya-patel98

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@shreeya-patel98 @PlaidCat @bmastbergen @thefossguy-ciq