When an attribute group is created with sysfs_create_group(), the ->sysfs_ops() callback is set to kobj_sysfs_ops, which sets the ->show() and ->store() callbacks to kobj_attr_show() and kobj_attr_store() respectively. These functions use container_of() to get the respective callback from the passed attribute, meaning that these callbacks need to be the same type as the callbacks in 'struct kobj_attribute'.
However, the platform_profile sysfs functions have the type of the ->show() and ->store() callbacks in 'struct device_attribute', which results a CFI violation when accessing platform_profile or platform_profile_choices under /sys/firmware/acpi because the types do not match:
CFI failure at kobj_attr_show+0x19/0x30 (target: platform_profile_choices_show+0x0/0x140; expected type: 0x7a69590c)
There is no functional issue from the type mismatch because the layout of 'struct kobj_attribute' and 'struct device_attribute' are the same, so the container_of() cast does not break anything aside from CFI.
Change the type of platform_profile_choices_show() and platform_profile_{show,store}() to match the callbacks in 'struct kobj_attribute' and update the attribute variables to match, which resolves the CFI violation.
Cc: stable@vger.kernel.org Fixes: a2ff95e018f1 ("ACPI: platform: Add platform profile support") Reported-by: John Rowley lkml@johnrowley.me Closes: https://github.com/ClangBuiltLinux/linux/issues/2047 Tested-by: John Rowley lkml@johnrowley.me Reviewed-by: Sami Tolvanen samitolvanen@google.com Signed-off-by: Nathan Chancellor nathan@kernel.org --- John reminded me that this is still an issue in 6.12:
https://github.com/ClangBuiltLinux/linux/issues/2047#issuecomment-2482635093
I know Greg said this driver should be using a 'struct device' but I am not familiar with that conversion and there was never any follow up to me asking how it should be done. I think it would be best to pick this change up now and have that conversion be done properly by someone who is more familiar with this driver later.
Changes in v2: - Rebase on linux-pm/acpi - Pick up Sami's reviewed-by tag - Adjust wording around why there is no functional issue from the mismatched types - Link to v1: https://lore.kernel.org/r/20240819-acpi-platform_profile-fix-cfi-violation-v... --- drivers/acpi/platform_profile.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/acpi/platform_profile.c b/drivers/acpi/platform_profile.c index d2f7fd7743a13df1dac3100c885208f4418218a5..11278f785526d48243e16659cd78d0b717846a7f 100644 --- a/drivers/acpi/platform_profile.c +++ b/drivers/acpi/platform_profile.c @@ -22,8 +22,8 @@ static const char * const profile_names[] = { }; static_assert(ARRAY_SIZE(profile_names) == PLATFORM_PROFILE_LAST);
-static ssize_t platform_profile_choices_show(struct device *dev, - struct device_attribute *attr, +static ssize_t platform_profile_choices_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) { int len = 0; @@ -49,8 +49,8 @@ static ssize_t platform_profile_choices_show(struct device *dev, return len; }
-static ssize_t platform_profile_show(struct device *dev, - struct device_attribute *attr, +static ssize_t platform_profile_show(struct kobject *kobj, + struct kobj_attribute *attr, char *buf) { enum platform_profile_option profile = PLATFORM_PROFILE_BALANCED; @@ -77,8 +77,8 @@ static ssize_t platform_profile_show(struct device *dev, return sysfs_emit(buf, "%s\n", profile_names[profile]); }
-static ssize_t platform_profile_store(struct device *dev, - struct device_attribute *attr, +static ssize_t platform_profile_store(struct kobject *kobj, + struct kobj_attribute *attr, const char *buf, size_t count) { int err, i; @@ -115,12 +115,12 @@ static ssize_t platform_profile_store(struct device *dev, return count; }
-static DEVICE_ATTR_RO(platform_profile_choices); -static DEVICE_ATTR_RW(platform_profile); +static struct kobj_attribute attr_platform_profile_choices = __ATTR_RO(platform_profile_choices); +static struct kobj_attribute attr_platform_profile = __ATTR_RW(platform_profile);
static struct attribute *platform_profile_attrs[] = { - &dev_attr_platform_profile_choices.attr, - &dev_attr_platform_profile.attr, + &attr_platform_profile_choices.attr, + &attr_platform_profile.attr, NULL };
--- base-commit: d47a60e487fbb65bbbca3d99e59009f0a4acf34d change-id: 20240819-acpi-platform_profile-fix-cfi-violation-de278753bd5f
Best regards,