linux-kernel - [PATCH] dell-smbios: fix double free in dell_smbios_init() and fixes in dell_smbios

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>] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 16 Dec 2022 02:17:41 +0300
From:   Rustam Subkhankulov <subkhankulov@...ras.ru>
To:     Hans de Goede <hdegoede@...hat.com>
Cc:     Rustam Subkhankulov <subkhankulov@...ras.ru>,
        Mark Gross <markgross@...nel.org>,
        ye xingchen <ye.xingchen@....com.cn>,
        "Darren Hart (VMware)" <dvhart@...radead.org>,
        Mario Limonciello <mario.limonciello@...l.com>,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org,
        Alexey Khoroshilov <khoroshilov@...ras.ru>
Subject: [PATCH] dell-smbios: fix double free in dell_smbios_init() and fixes in dell_smbios_exit()

If an error occurs in function build_tokens_sysfs(), then all the memory
that has been allocated is correctly freed at certain labels at the end
of this function.

build_tokens_sysfs() returns a non-zero value on error, function
free_group() is called, resulting in a double-free. Removing
free_group() function call will fix this problem.

Also, it seems that instead of free_group() call, there should be
exit_dell_smbios_smm() and exit_dell_smbios_wmi() calls, since there is
initialization, but there is no release of resources in case of an error.

Since calling 'exit' functions for 'smm' and 'wmi' is unsafe if
initialization failed, in dell_smbios_exit() and dell_smbios_init()
we need to call 'exit' only if initialization before was successful.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Rustam Subkhankulov <subkhankulov@...ras.ru>
Fixes: 25d47027e100 ("platform/x86: dell-smbios: Link all dell-smbios-* modules together")
---
 drivers/platform/x86/dell/dell-smbios-base.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c
index fc086b66f70b..cfef8cdd1215 100644
--- a/drivers/platform/x86/dell/dell-smbios-base.c
+++ b/drivers/platform/x86/dell/dell-smbios-base.c
@@ -29,6 +29,8 @@ static struct device_attribute *token_location_attrs;
 static struct device_attribute *token_value_attrs;
 static struct attribute **token_attrs;
 static DEFINE_MUTEX(smbios_mutex);
+static bool wmi_initialized;
+static bool smm_initialized;
 
 struct smbios_device {
 	struct list_head list;
@@ -607,6 +609,9 @@ static int __init dell_smbios_init(void)
 			goto fail_sysfs;
 	}
 
+	wmi_initialized = !(wmi);
+	smm_initialized = !(smm);
+
 	return 0;
 
 fail_sysfs:
@@ -628,8 +633,16 @@ static int __init dell_smbios_init(void)
 
 static void __exit dell_smbios_exit(void)
 {
-	exit_dell_smbios_wmi();
-	exit_dell_smbios_smm();
+	if (wmi_initialized) {
+		exit_dell_smbios_wmi();
+		wmi_initialized = 0;
+	}
+
+	if (smm_initialized) {
+		exit_dell_smbios_smm();
+		smm_initialized = 0;
+	}
+
 	mutex_lock(&smbios_mutex);
 	if (platform_device) {
 		if (da_tokens)
-- 
2.34.1