[fips-9-compliant] perf: Disallow mis-matched inherited group reads

[shreeya-patel98]

• 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-8891
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]>

Note :-
Above upstream patch has a NULL pointer deref issue, hence we added https://github.com/ctrliq/kernel-src-tree/commit/28a6c6e2cece9b3490c1ee8612559118cea2fddb ("perf/core: Fix potential NULL deref") to avoid any regression.

-------------------------------------------------------------------------------------------------------
perf/core: Fix potential NULL deref

jira VULN-8891
cve-bf 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-9-compliant/kernel-src-tree
Running make mrproper...
[TIMER]{MRPROPER}: 5s
x86_64 architecture detected, copying config
'configs/kernel-x86_64-rhel.config' -> '.config'
Setting Local Version for build
CONFIG_LOCALVERSION="-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb"
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]
  HOSTCC  scripts/kconfig/lexer.lex.o
  HOSTCC  scripts/kconfig/menu.o
  HOSTCC  scripts/kconfig/parser.tab.o
  HOSTCC  scripts/kconfig/preprocess.o
  HOSTCC  scripts/kconfig/symbol.o
  HOSTCC  scripts/kconfig/util.o
  HOSTLD  scripts/kconfig/conf
#
# configuration written to .config
#
Starting Build
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_32.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_64.h
  SYSHDR  arch/x86/include/generated/uapi/asm/unistd_x32.h
  SYSTBL  arch/x86/include/generated/asm/syscalls_32.h
  SYSHDR  arch/x86/include/generated/asm/unistd_32_ia32.h
  SYSHDR  arch/x86/include/generated/asm/unistd_64_x32.h
  SYSTBL  arch/x86/include/generated/asm/syscalls_64.h
  <--snip-->
  STRIP   /lib/modules/5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+/kernel/sound/virtio/virtio_snd.ko
  SIGN    /lib/modules/5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+/kernel/sound/usb/usx2y/snd-usb-us122l.ko
  SIGN    /lib/modules/5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+/kernel/sound/virtio/virtio_snd.ko
  DEPMOD  /lib/modules/5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+
[TIMER]{MODULES}: 11s
Making Install
sh ./arch/x86/boot/install.sh \
	5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+ arch/x86/boot/bzImage \
	System.map "/boot"
[TIMER]{INSTALL}: 20s
Checking kABI
kABI check passed
Setting Default Kernel to /boot/vmlinuz-5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+ and Index to 2
The default is /boot/loader/entries/809410938d1447fc931cf787fb714082-5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+.conf with index 2 and kernel /boot/vmlinuz-5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+
The default is /boot/loader/entries/809410938d1447fc931cf787fb714082-5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+.conf with index 2 and kernel /boot/vmlinuz-5.14.0-shreeya_fips-9-compliant_5.14.0-284.30.1-020b04cbbbeb+
Generating grub configuration file ...
Adding boot menu entry for UEFI Firmware Settings ...
done
Hopefully Grub2.0 took everything ... rebooting after time metrices
[TIMER]{MRPROPER}: 5s
[TIMER]{BUILD}: 1326s
[TIMER]{MODULES}: 11s
[TIMER]{INSTALL}: 20s
[TIMER]{TOTAL} 1367s
Rebooting in 10 seconds

kernel-build.log

Kselftests

shreeya@spatel-dev-bom:~/ciq/workspace/fips-9-compliant$ grep '^ok ' kselftest-before.log | wc -l && grep '^ok ' kselftest-after.log | wc -l
290
291
shreeya@spatel-dev-bom:~/ciq/workspace/fips-9-compliant$ grep '^not ok ' kselftest-before.log | wc -l && grep '^not ok ' kselftest-after.log | wc -l
68
67

kselftest-after.log
kselftest-before.log

[thefossguy-ciq]

upstream-diff [...]
[...]
Also, add an upstream patch 28a6c6e ("perf/core: Fix potential NULL deref")
which fixes a NULL pointer deref issue in the existing CVE fix.

I don't know what's the consensus on the above addition to the upstream-diff. I'm indifferent but want to know @PlaidCat's thoughts.

The rest looks good though!

[PlaidCat]

upstream-diff [...]
[...]
Also, add an upstream patch 28a6c6e ("perf/core: Fix potential NULL deref")
which fixes a NULL pointer deref issue in the existing CVE fix.

I don't know what's the consensus on the above addition to the upstream-diff. I'm indifferent but want to know @PlaidCat's thoughts.

The rest looks good though!

OK, since we're not explicit including this in the original CVE fix commit and its a cve-bf commit later this is more contextual references to the PR rather than the specific commit.

What would happen is we might bisect the git history to this commit, read the commit message and assume that 28a6c6e ("perf/core: Fix potential NULL deref") is incorporated into that specific commit and may become confused.

Lets strip that out, and move it to the PR statement, the instinct is correct though.
Thank You

[shreeya-patel98]

Changes are done as requested

[thefossguy-ciq]

🚤

[bmastbergen]

🥌