From: Sabrina Dubroca on gitlab.com Merge Request: https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619
This MR introduces a new kABI macro, RH_KABI_EXCLUDE_WITH_SIZE. The implementation is identical to RH_KABI_EXTEND_WITH_SIZE, but the meaning is more in line with RH_KABI_EXCLUDE. The size check and reserved space allows us to exclude an element embedded inside a kABI-protected structure while reserving space for it to grow in the future.
The size checks that are part of multiple kABI macros are currently not enabled because the config option to turn them on is missing. Those size checks only make sense on regular builds, so they're disabled on debug kernels.
Signed-off-by: Sabrina Dubroca sdubroca@redhat.com
--- include/linux/rh_kabi.h | 26 ++++++++++ redhat/configs/ark/debug/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS | 1 + redhat/configs/ark/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS | 1 + redhat/configs/ark/kgcov/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS | 1 + redhat/configs/common/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS | 1 + Kconfig.redhat | 9 +++ 6 files changed, 39 insertions(+), 0 deletions(-)
From: Sabrina Dubroca sdubroca@redhat.com
redhat: rh_kabi: introduce RH_KABI_EXCLUDE_WITH_SIZE
This macro is similar to RH_KABI_EXTEND_WITH_SIZE, but with a different intention. While RH_KABI_EXTEND_WITH_SIZE is meant to add new elements that may grow in the future to kABI-protected structures, RH_KABI_EXCLUDE_WITH_SIZE should be used to hide existing elements from kABI checksum computation, while allowing them to be extended in the future without silently breaking binary compatibility of any field that follows it.
The implementation differs from RH_KABI_EXTEND_WITH_SIZE so that _size is included in the kABI checksum. That way, we can't accidentally grow the reserved space, as that would change the offsets of all the following fields.
The size check guarantees that binary compatibility is preserved. Until now, __RH_KABI_CHECK_SIZE was only used in the !__GENKSYMS__ versions of macros, so it wasn't defined in the __GENKSYMS__ case. We need to define it to nothing so that _new is ignored by checksum computation.
Signed-off-by: Sabrina Dubroca sdubroca@redhat.com
diff --git a/include/linux/rh_kabi.h b/include/linux/rh_kabi.h index blahblah..blahblah 100644 --- a/include/linux/rh_kabi.h +++ b/include/linux/rh_kabi.h @@ -306,6 +306,23 @@ * of the size is not allowed and would constitute a silent kABI breakage. * Beware that the RH_KABI_EXCLUDE macro does not do any size checks. * + * RH_KABI_EXCLUDE_WITH_SIZE + * Like RH_KABI_EXCLUDE, this macro excludes the element from + * checksum generation. The same warnings as for RH_KABI_EXCLUDE + * apply: use RH_KABI_FORCE_CHANGE. + * + * This macro is intended to be used for elements embedded inside + * kABI-protected structures (struct, array). In contrast with + * RH_KABI_EXCLUDE, this macro reserves extra space, so that the + * embedded element can grow without changing the offsets of the + * fields that follow. The provided 'size' is the total space to be + * added in longs (i.e. it's 8 * 'size' bytes), including the size + * of the added element. It is automatically checked that the new + * element does not overflow the reserved space, now nor in the + * future. The size is also included in the checksum via the + * reserved space, to ensure that we don't accidentally change it, + * which would change the offsets of the fields that follow. + * * RH_KABI_BROKEN_INSERT * RH_KABI_BROKEN_REMOVE * Insert a field to the middle of a struct / delete a field from a struct. @@ -377,6 +394,8 @@ # define _RH_KABI_REPLACE(_orig, _new) _orig # define _RH_KABI_EXCLUDE(_elem)
+# define __RH_KABI_CHECK_SIZE(_item, _size) + #else
# define RH_KABI_ALIGN_WARNING ". Disable CONFIG_RH_KABI_SIZE_ALIGN_CHECKS if debugging." @@ -477,6 +496,13 @@
#define RH_KABI_EXCLUDE(_elem) _RH_KABI_EXCLUDE(_elem);
+#define RH_KABI_EXCLUDE_WITH_SIZE(_new, _size) \ + union { \ + RH_KABI_EXCLUDE(_new) \ + unsigned long RH_KABI_UNIQUE_ID[_size]; \ + __RH_KABI_CHECK_SIZE(_new, 8 * (_size)); \ + }; + #define RH_KABI_EXTEND_WITH_SIZE(_new, _size) \ RH_KABI_EXTEND(union { \ _new; \
-- https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619
From: Jiri Benc on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619#note_8466797...
This leaves the semicolon here in the GENKSYMS pass. I don't think we want that; the kABI checksum would be generated from:
``` union { unsigned long xyz[_size]; ; } ```
The fix is redefining __RH_KABI_CHECK_SIZE to include the semicolon (and call it without the semicolon everywhere).
From: Sabrina Dubroca on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619#note_1630068...
Do we care? As long as wer'e consistent and always compute the checksum with that extra semicolon, I don't see how it makes a difference.
(I'll re-open this MR pretty soon since it turns out we're actually going to need this macro now)
From: Sabrina Dubroca sdubroca@redhat.com
redhat: kABI: add missing RH_KABI_SIZE_ALIGN_CHECKS Kconfig option
When the kABI macros were copied from the RHEL kernel to ARK, the RH_KABI_SIZE_ALIGN_CHECKS config option was dropped. Without it, the size checks included in some of the kABI macros can't be enabled.
Add the KABI_SIZE_ALIGN_CHECKS config option, enable it on regular ARK configs, and disable it for debug and kgcov kernels.
Signed-off-by: Sabrina Dubroca sdubroca@redhat.com
diff --git a/Kconfig.redhat b/Kconfig.redhat index blahblah..blahblah 100644 --- a/Kconfig.redhat +++ b/Kconfig.redhat @@ -14,4 +14,13 @@ config RHEL_DIFFERENCES
Unless you want a restricted kernel, say N here.
+config RH_KABI_SIZE_ALIGN_CHECKS + bool "Enables more stringent kabi checks in the macros" + depends on RHEL_DIFFERENCES + default y + help + This option enables more stringent kabi checks. Those must + be disabled in case of a debug build, because debug builds + allow to change struct sizes. + endmenu diff --git a/redhat/configs/ark/debug/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS b/redhat/configs/ark/debug/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS new file mode 100644 index blahblah..blahblah 100644 --- /dev/null +++ b/redhat/configs/ark/debug/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS @@ -0,0 +1 @@ +# CONFIG_RH_KABI_SIZE_ALIGN_CHECKS is not set diff --git a/redhat/configs/ark/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS b/redhat/configs/ark/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS new file mode 100644 index blahblah..blahblah 100644 --- /dev/null +++ b/redhat/configs/ark/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS @@ -0,0 +1 @@ +CONFIG_RH_KABI_SIZE_ALIGN_CHECKS=y diff --git a/redhat/configs/ark/kgcov/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS b/redhat/configs/ark/kgcov/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS new file mode 100644 index blahblah..blahblah 100644 --- /dev/null +++ b/redhat/configs/ark/kgcov/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS @@ -0,0 +1 @@ +# CONFIG_RH_KABI_SIZE_ALIGN_CHECKS is not set diff --git a/redhat/configs/common/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS b/redhat/configs/common/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS new file mode 100644 index blahblah..blahblah 100644 --- /dev/null +++ b/redhat/configs/common/generic/CONFIG_RH_KABI_SIZE_ALIGN_CHECKS @@ -0,0 +1 @@ +# CONFIG_RH_KABI_SIZE_ALIGN_CHECKS is not set
-- https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619
From: Patrick Talbert on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619#note_8664217...
@sdubroca The `os-build` branch is rebased whenever there is an official release of the upstream kernel. This is expected to next happen this coming Monday, the 14th. So just be aware if this MR is not ready for merge by then that it will need to be rebased.
From: Justin M. Forbes on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619#note_1651905...
This file should not be required. We set the config appropriately in redhat/configs/rhel and there is nothing common between the RHEL and Fedora configs for it.
From: Justin M. Forbes on gitlab.com https://gitlab.com/cki-project/kernel-ark/-/merge_requests/1619#note_1651907...
This should also not be required. Fedora does not need an entry for this one anymore because the Kconfig is now dependent on RHEL_DIFFERENCES which is not enabled for Fedora.
kernel@lists.fedoraproject.org