[LTS 9.2] perf: Disallow mis-matched inherited group reads #452

shreeya-patel98 · 2025-07-30T14:54:34Z

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-6760
pre-cve CVE-2023-5717
commit-author Peter Zijlstra <peterz@infradead.org>
commit 32671e3799ca2e4590773fd0e63aaa4229e50c06

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 fa8c269353d5 ("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: fa8c269353d5 ("perf/core: Invert perf_read_group() loops")
        Reported-by: Budimir Markovic <markovicbudimir@gmail.com>
        Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20231018115654.GK33217@noisy.programming.kicks-ass.net
(cherry picked from commit 32671e3799ca2e4590773fd0e63aaa4229e50c06)
        Signed-off-by: Shreeya Patel <spatel@ciq.com>

-------------------------------------------------------------------------------------------------------
perf: Fix kABI breakage in struct perf_event

jira VULN-6760
cve CVE-2023-5717
commit-author Shreeya Patel <spatel@ciq.com>

Fix kabi breakage in commit 32671e3 ("[Backport] perf: Disallow
mis-matched inherited group reads")

Fixes: 32671e3 ("perf: Disallow mis-matched inherited group reads")
Signed-off-by: Shreeya Patel <spatel@ciq.com>

Kernel build logs

/mnt/scratch/kernel-src-tree
Running make mrproper...
  CLEAN   arch/x86/crypto
  CLEAN   arch/x86/entry/vdso
  CLEAN   arch/x86/kernel/cpu
  CLEAN   arch/x86/kernel
  CLEAN   arch/x86/purgatory
  CLEAN   arch/x86/realmode/rm
  CLEAN   arch/x86/lib
  CLEAN   certs
  CLEAN   crypto/asymmetric_keys
  CLEAN   drivers/firmware/efi/libstub
  CLEAN   drivers/gpu/drm/radeon
  CLEAN   drivers/scsi
  CLEAN   drivers/tty/vt
  CLEAN   drivers/video/logo
  CLEAN   kernel/debug/kdb
  CLEAN   kernel
  CLEAN   lib/raid6
  CLEAN   lib
  CLEAN   net/wireless
  CLEAN   security/selinux
  CLEAN   usr/include
  CLEAN   usr
  CLEAN   arch/x86/boot/compressed
  CLEAN   arch/x86/boot
  CLEAN   arch/x86/tools
  CLEAN   vmlinux.symvers modules-only.symvers modules.builtin modules.builtin.modinfo
  CLEAN   scripts/basic
  CLEAN   scripts/genksyms
  CLEAN   scripts/kconfig
  CLEAN   scripts/mod
  CLEAN   scripts/selinux/genheaders
  CLEAN   scripts/selinux/mdp
  CLEAN   scripts
  CLEAN   include/config include/generated arch/x86/include/generated .config .config.old .version Module.symvers certs/signing_key.pem certs/signing_key.x509 certs/x509.genkey
[TIMER]{MRPROPER}: 8s
x86_64 architecture detected, copying config
'configs/kernel-x86_64-rhel.config' -> '.config'
Setting Local Version for build
CONFIG_LOCALVERSION="-spatel_ciqlts9_2-0622cb855261"
Making olddefconfig
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/confdata.o
  HOSTCC  scripts/kconfig/expr.o
  LEX     scripts/kconfig/lexer.lex.c
  YACC    scripts/kconfig/parser.tab.[ch]
  <--snip-->
    SIGN    /lib/modules/5.14.0-spatel_ciqlts9_2-0622cb855261+/kernel/sound/usb/line6/snd-usb-podhd.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_2-0622cb855261+/kernel/sound/usb/line6/snd-usb-toneport.ko
  INSTALL /lib/modules/5.14.0-spatel_ciqlts9_2-0622cb855261+/kernel/net/key/af_key.ko
  STRIP   /lib/modules/5.14.0-spatel_ciqlts9_2-0622cb855261+/kernel/net/key/af_key.ko
  SIGN    /lib/modules/5.14.0-spatel_ciqlts9_2-0622cb855261+/kernel/net/key/af_key.ko
  DEPMOD  /lib/modules/5.14.0-spatel_ciqlts9_2-0622cb855261+
[TIMER]{MODULES}: 8s
Making Install
sh ./arch/x86/boot/install.sh \
	5.14.0-spatel_ciqlts9_2-0622cb855261+ arch/x86/boot/bzImage \
	System.map "/boot"
[TIMER]{INSTALL}: 20s
Checking kABI
kABI check passed
Setting Default Kernel to /boot/vmlinuz-5.14.0-spatel_ciqlts9_2-0622cb855261+ and Index to 1
The default is /boot/loader/entries/2d84c760132f4bab9836f9dd9e3ac547-5.14.0-spatel_ciqlts9_2-0622cb855261+.conf with index 1 and kernel /boot/vmlinuz-5.14.0-spatel_ciqlts9_2-0622cb855261+
The default is /boot/loader/entries/2d84c760132f4bab9836f9dd9e3ac547-5.14.0-spatel_ciqlts9_2-0622cb855261+.conf with index 1 and kernel /boot/vmlinuz-5.14.0-spatel_ciqlts9_2-0622cb855261+
Generating grub configuration file ...
Adding boot menu entry for UEFI Firmware Settings ...
done
Hopefully Grub2.0 took everything ... rebooting after time metrices
[TIMER]{MRPROPER}: 8s
[TIMER]{BUILD}: 1325s
[TIMER]{MODULES}: 8s
[TIMER]{INSTALL}: 20s
[TIMER]{TOTAL} 1367s
Rebooting in 10 seconds

kernel-build.log

Kselftests

shreeya@spatel-dev-bom:~/ciq/ciqlts9_2$ grep '^ok ' kselftest-before.log | wc -l && grep '^ok ' kselftest-after.log | wc -l
296
297
shreeya@spatel-dev-bom:~/ciq/ciqlts9_2$ grep '^not ok ' kselftest-before.log | wc -l && grep '^not ok ' kselftest-after.log | wc -l
71
70

kselftest-before.log
kselftest-after.log

shreeya-patel98 · 2025-07-30T15:02:26Z

Do we want to have the kABI fix patch as a separate commit or should just fix it in the commit we cherry-picked?
For now I added as a separate one for everyone to review, will merge it if required

thefossguy-ciq · 2025-07-30T15:21:38Z

Do we want to have the kABI fix patch as a separate commit or should just fix it in the commit we cherry-picked? For now I added as a separate one for everyone to review, will merge it if required

I don't know if we have an official consensus about this but so far, I've seen it done in a single commit and specify the changes using upstream-diff.

bmastbergen · 2025-07-30T15:49:26Z

include/linux/perf_event.h

@@ -691,6 +692,7 @@ struct perf_event {
 	/* The cumulative AND of all event_caps for events in this group. */
 	int				group_caps;

+	RH_KABI_EXTEND(unsigned int     group_generation)


From rh_kabi.h

* RH_KABI_EXTEND * Adds a new field to a struct. This must always be added to the end of * the struct. Before using this macro, make sure this is actually safe * to do - there is a number of conditions under which it is *not* safe. * In particular (but not limited to), this macro cannot be used: * - if the struct in question is embedded in another struct, or * - if the struct is allocated by drivers either statically or * dynamically, or * - if the struct is allocated together with driver data (an example of * such behavior is struct net_device or struct request). *

From the comment it seems like RH_KABI_EXTEND is meant to be used for adding new fields to the end of a struct, but here it is in the middle. In some cases adding a field to the end of a struct might be acceptable kabi breakage so I imagine that is what this macro is for. Here it may be placating check-kabi but the kabi is straight up broken. I think that would be RH_KABI_BROKEN_INSERT IFF a and b are true

* RH_KABI_BROKEN_INSERT * RH_KABI_BROKEN_REMOVE * Insert a field to the middle of a struct / delete a field from a struct. * Note that this breaks kABI! It can be done only when it's certain that * no 3rd party driver can validly reach into the struct. A typical * example is a struct that is: both (a) referenced only through a long * chain of pointers from another struct that is part of a whitelisted * symbol and (b) kernel internal only, it should have never been visible * to genksyms in the first place. * * Another example are structs that are explicitly exempt from kABI * guarantee but we did not have enough foresight to use RH_KABI_EXCLUDE. * In this case, the warning for RH_KABI_EXCLUDE applies. * * A detailed explanation of correctness of every RH_KABI_BROKEN_* macro * use is especially important.

@bmastbergen I am wondering if we can just move the field to the end?

I think putting it at the end would work here.

Yes, putting it at the end of the struct works in this case because both of these statements are true:

The perf event API forces all users to dynamically allocate the struct through its own function perf_event_alloc(), because there's no API to accept a preallocated struct.

The struct is never allocated as an array, so expanding the struct size won't break anything.

bmastbergen · 2025-07-30T16:28:55Z

🔍 Upstream Linux Kernel Commit Check

⚠ PR commit 8b7eacd9347a (perf: Disallow mis-matched inherited group reads) references upstream commit
32671e3799ca which has been referenced by a Fixes: tag in the upstream
Linux kernel:

    a71ef31485bb perf/core: Fix potential NULL deref (Peter Zijlstra)

This is an automated message from the kernel commit checker workflow.

bmastbergen · 2025-07-30T16:29:25Z

🔍 Upstream Linux Kernel Commit Check

⚠ PR commit 8b7eacd9347a (perf: Disallow mis-matched inherited group reads) references upstream commit
32671e3799ca which has been referenced by a Fixes: tag in the upstream
Linux kernel:
    a71ef31485bb perf/core: Fix potential NULL deref (Peter Zijlstra)
This is an automated message from the kernel commit checker workflow.

This wasn't automated, I ran my upstream commit checker script locally

bmastbergen · 2025-07-31T13:18:57Z

Do we want to have the kABI fix patch as a separate commit or should just fix it in the commit we cherry-picked? For now I added as a separate one for everyone to review, will merge it if required

Sorry I didn't comment on this earlier. As Pratham said, usually we just do one commit with an upstream-diff explaining the macro usage. I think that would be my preference here.

bmastbergen · 2025-07-31T13:27:14Z

🔍 Upstream Linux Kernel Commit Check

⚠ PR commit 8b7eacd9347a (perf: Disallow mis-matched inherited group reads) references upstream commit
32671e3799ca which has been referenced by a Fixes: tag in the upstream
Linux kernel:
    a71ef31485bb perf/core: Fix potential NULL deref (Peter Zijlstra)
This is an automated message from the kernel commit checker workflow.

I think we should pick this up as well. Usually we'd tag this with 'cve-bf CVE-2023-5717'

shreeya-patel98 · 2025-07-31T14:33:24Z

This is an automated message from the kernel commit checker workflow.

I think we should pick this up as well. Usually we'd tag this with 'cve-bf CVE-2023-5717'

So your script checks if there were any fixes upstreamed for this particular patch. That's cool :)

jira VULN-6760 cve CVE-2023-5717 commit-author Peter Zijlstra <peterz@infradead.org> 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. Also, add an upstream patch 28a6c6e ("perf/core: Fix potential NULL deref") which fixes a NULL pointer deref issue in the existing CVE fix. 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 <markovicbudimir@gmail.com> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Link: https://lkml.kernel.org/r/20231018115654.GK33217@noisy.programming.kicks-ass.net (cherry picked from commit 32671e3) Signed-off-by: Shreeya Patel <spatel@ciq.com>

jira VULN-6760 cve-bf CVE-2023-5717 commit-author Peter Zijlstra <peterz@infradead.org> commit a71ef31 Smatch is awesome. Fixes: 32671e3 ("perf: Disallow mis-matched inherited group reads") Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by: Ingo Molnar <mingo@kernel.org> (cherry picked from commit a71ef31) Signed-off-by: Shreeya Patel <spatel@ciq.com>

bmastbergen

🥌

thefossguy-ciq

🚤

shreeya-patel98 requested review from kerneltoast, jainanmol84, PlaidCat, bmastbergen and thefossguy-ciq July 30, 2025 14:54

shreeya-patel98 self-assigned this Jul 30, 2025

shreeya-patel98 force-pushed the {spatel}_ciqlts9_2 branch from 5a61cf0 to 6237664 Compare July 30, 2025 14:56

bmastbergen reviewed Jul 30, 2025

View reviewed changes

shreeya-patel98 force-pushed the {spatel}_ciqlts9_2 branch from 6237664 to 8968285 Compare July 31, 2025 09:56

shreeya-patel98 requested a review from bmastbergen July 31, 2025 10:20

shreeya-patel98 force-pushed the {spatel}_ciqlts9_2 branch from 8968285 to 28a6c6e Compare July 31, 2025 15:59

shreeya-patel98 added 2 commits July 31, 2025 16:01

shreeya-patel98 force-pushed the {spatel}_ciqlts9_2 branch from 28a6c6e to df61ae5 Compare July 31, 2025 16:10

bmastbergen approved these changes Jul 31, 2025

View reviewed changes

thefossguy-ciq approved these changes Jul 31, 2025

View reviewed changes

shreeya-patel98 merged commit aaf8ba2 into ciqlts9_2 Aug 1, 2025
3 of 5 checks passed

shreeya-patel98 deleted the {spatel}_ciqlts9_2 branch August 1, 2025 10:02

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[LTS 9.2] perf: Disallow mis-matched inherited group reads #452

[LTS 9.2] perf: Disallow mis-matched inherited group reads #452

Uh oh!

shreeya-patel98 commented Jul 30, 2025 •

edited

Loading

Uh oh!

shreeya-patel98 commented Jul 30, 2025 •

edited

Loading

Uh oh!

thefossguy-ciq commented Jul 30, 2025

Uh oh!

bmastbergen Jul 30, 2025

Uh oh!

shreeya-patel98 Jul 30, 2025

Uh oh!

bmastbergen Jul 30, 2025

Uh oh!

kerneltoast Jul 30, 2025

Uh oh!

bmastbergen commented Jul 30, 2025

Uh oh!

bmastbergen commented Jul 30, 2025

🔍 Upstream Linux Kernel Commit Check

Uh oh!

bmastbergen commented Jul 31, 2025

Uh oh!

bmastbergen commented Jul 31, 2025

🔍 Upstream Linux Kernel Commit Check

Uh oh!

shreeya-patel98 commented Jul 31, 2025

Uh oh!

bmastbergen left a comment

Uh oh!

thefossguy-ciq left a comment

Uh oh!

Uh oh!

Uh oh!

[LTS 9.2] perf: Disallow mis-matched inherited group reads #452

[LTS 9.2] perf: Disallow mis-matched inherited group reads #452

Uh oh!

Conversation

shreeya-patel98 commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Commit message

Kernel build logs

Kselftests

Uh oh!

shreeya-patel98 commented Jul 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thefossguy-ciq commented Jul 30, 2025

Uh oh!

bmastbergen Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

shreeya-patel98 Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

bmastbergen Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

kerneltoast Jul 30, 2025

Choose a reason for hiding this comment

Uh oh!

bmastbergen commented Jul 30, 2025

🔍 Upstream Linux Kernel Commit Check

Uh oh!

bmastbergen commented Jul 30, 2025

🔍 Upstream Linux Kernel Commit Check

Uh oh!

bmastbergen commented Jul 31, 2025

Uh oh!

bmastbergen commented Jul 31, 2025

🔍 Upstream Linux Kernel Commit Check

Uh oh!

shreeya-patel98 commented Jul 31, 2025

Uh oh!

bmastbergen left a comment

Choose a reason for hiding this comment

Uh oh!

thefossguy-ciq left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

shreeya-patel98 commented Jul 30, 2025 •

edited

Loading

shreeya-patel98 commented Jul 30, 2025 •

edited

Loading