linux-kernel - [PATCH v2] ACPI: platform-profile: Fix CFI violation when accessing sysfs files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20241118-acpi-platform_profile-fix-cfi-violation-v2-1-62ff952804de@kernel.org>
Date: Mon, 18 Nov 2024 07:39:32 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: "Rafael J. Wysocki" <rafael@...nel.org>, Len Brown <lenb@...nel.org>
Cc: Hans de Goede <hdegoede@...hat.com>, 
 Mark Pearson <markpearson@...ovo.com>, Kees Cook <kees@...nel.org>, 
 Sami Tolvanen <samitolvanen@...gle.com>, linux-acpi@...r.kernel.org, 
 linux-kernel@...r.kernel.org, llvm@...ts.linux.dev, patches@...ts.linux.dev, 
 stable@...r.kernel.org, John Rowley <lkml@...nrowley.me>, 
 Nathan Chancellor <nathan@...nel.org>
Subject: [PATCH v2] ACPI: platform-profile: Fix CFI violation when
 accessing sysfs files

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@...r.kernel.org
Fixes: a2ff95e018f1 ("ACPI: platform: Add platform profile support")
Reported-by: John Rowley <lkml@...nrowley.me>
Closes: https://github.com/ClangBuiltLinux/linux/issues/2047
Tested-by: John Rowley <lkml@...nrowley.me>
Reviewed-by: Sami Tolvanen <samitolvanen@...gle.com>
Signed-off-by: Nathan Chancellor <nathan@...nel.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-v1-1-479365d848f6@kernel.org
---
 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,
-- 
Nathan Chancellor <nathan@...nel.org>