From patchwork Tue May 28 20:49:02 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Armin Wolf X-Patchwork-Id: 13677444 X-Patchwork-Delegate: hdegoede@redhat.com Received: from mout.gmx.net (mout.gmx.net [212.227.15.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8130D45024; Tue, 28 May 2024 20:49:15 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.15.18 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716929358; cv=none; b=VjFHb3RwjXK7CfdlYaeg8oFiYsn86pN6jwggVcsdmIFSs7ygMcpNUEBmdPzNFx38TdzSnSpoH300yn8H18/ziHKg5xvD8PpWbCIUYxYVeuoS2l577UvfbSWBoasitYpBWyAxDtU3z7LPbkOa/7jscKWNIdpb3Vw8NFmTl0R8eBI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716929358; c=relaxed/simple; bh=lgj/spm7RG5xxoefr3gCdjm12/1dHbjezBtdM193Rgc=; h=From:To:Cc:Subject:Date:Message-Id:MIME-Version; b=nFSsI6XzPZE7pwE32u2uab84mvuaBD49wjoLNbiUFKutAyZNuHnrH/5CzS8IGaWMHQFRjG8YLKCdBX2sw+1qC1QjmwtQmww3Yfdb46AkZeOv96Oc+KPf2SBOROq4wbFxJ8Z8yEmICo1XYQp5w6yjVH2NMa16rxa7A0Jcu4aGBu0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gmx.de; spf=pass smtp.mailfrom=gmx.de; dkim=pass (2048-bit key) header.d=gmx.de header.i=w_armin@gmx.de header.b=qU9LfOXH; arc=none smtp.client-ip=212.227.15.18 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gmx.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmx.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmx.de header.i=w_armin@gmx.de header.b="qU9LfOXH" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmx.de; s=s31663417; t=1716929348; x=1717534148; i=w_armin@gmx.de; bh=/MoR4zs991DYMvTz8rBH16o4vXvcYUjLg0tOjYGFX7U=; h=X-UI-Sender-Class:From:To:Cc:Subject:Date:Message-Id: MIME-Version:Content-Transfer-Encoding:cc: content-transfer-encoding:content-type:date:from:message-id: mime-version:reply-to:subject:to; b=qU9LfOXH3kttrp82hwxTW+Wjqfa5WMs9IHG7lL0VH30GV+yssiXnajDr8/nhQToN bNqtLUPwrMMuqX3rfgkGV3n7ZkAFvVKwRQgHiPTl8NODO8DMLtBRhYmyZpAkfbbtd +GRr8rOLXusBSuPc1GGXXZkK5kJ8rrN13MpkufIoVvISq2ZHZTKHl7J+aq44FclTe qo69FAP0DAmgK5gSIvgbvgGJh1gM8uLRbSsKG+I3/0KKTejOk1kpncvw/Q3RENmzB w+pRowDc6hotPixXgDQvVrEil0WpQZvldqknR/jOjHoYDoQv/Tl5EWTdomvs0WQAg T1JIgpQbM7MEi6Vj4A== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from mx-amd-b650.users.agdsn.de ([141.30.226.129]) by mail.gmx.net (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1M8QWA-1s7g2d3lQW-004PmO; Tue, 28 May 2024 22:49:07 +0200 From: Armin Wolf To: hdegoede@redhat.com, ilpo.jarvinen@linux.intel.com Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 1/2] platform/x86: dell-smbios: Fix wrong token data in sysfs Date: Tue, 28 May 2024 22:49:02 +0200 Message-Id: <20240528204903.445546-1-W_Armin@gmx.de> X-Mailer: git-send-email 2.39.2 Precedence: bulk X-Mailing-List: platform-driver-x86@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Provags-ID: V03:K1:RIu+x3Tr2dOcYnpYqC2S6AQeTLiKMoAzO3Re2OU/aMuaaANNRnp a28RwAPVpGcRM8NFyCMXvXWvPOCWv9ZtPd5XijjsMWPsDv+/N2J4oAjIgNt9zzLlcaKAItr z2bP2tfznnWEY41scMfyRzKVf5gpcOsJBuNyE5I3QnVlaVhwTaq3lUQqDEHehuhakSzqERk NkdmKX7sWX9fFGmeBZHMg== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:yRxcls1m9yE=;fk5kj66kEpRoN/4AgoTh6KnkR55 0mdzv/TRnjOr3uAoJS3y1CRg4+/mUxUxh53Mw6Nrpml3lCLVKKJRB6Me/xFPmpw9xehW9sALX 1+RKkyoqJiCC2joVmgBUfKQBBH71keIfXFeDlVG2dhdgB8DhbCmXb2N3Byz3Dm0HiRQ+lDXs3 uhCqGnY7xkfFHlaJskq6jXlNi1gYUVxmFICJ7MNGfjVF2iagpsrxF+mro8UbgWnrgt7x8NVfB JHyuVAn3BrmglhC0bUMxe3uih8w2KqH+myK6t6ucLfaAGfc068YtVn+cQ8orK2Yao3XrZYzq7 YdD+bu9HDNPS8/uDaSqV4ou7kKXbBnGz7xdELDJy3Dd/O4T9V7d3vbot2Vyj86ceaCn4SWp9E DcozyhRNevWQ/Z/aouXRIOqlLmdTZUZp1UiUMLMZ8IV4S2k6OBUbyhjwvNMfImVIKIsaQeQW1 PxRog3HOaQoZ0mkxhu3LAZmQjWy7OIVhjcz6Ch4SHFwjUa/KWYOGHCnYegN+DTa07La2ZUiCA 8eo0i9ZwRQFp1uS/J+xr8uQb5sL9E+D0yTaTsW8w5TNKrSPw41Cfw+jPKgdD7A1WuScyvlPeI U057XNz/e7G0TYIGC69xN8N9+FDp3QeI4xPPiJOrrHnmKfWGyqa+ARcUfMuAuZNVCL60Pd2tZ d2QW+H/Hr/If8miZaV56ijXs3fvGfQAZ0Yb0SNjJrfpQBz334xMauIxBIBb98aUVw3SxX7gmT 9tJxD5XmxMiAcw0ZsiaUR1iYGAVrTxuqCRtdeM/80qcBCGjuPlm4FY0dvCBCo72LZn6cjvAKa alW5y+APj2bodhsMbBiYvpNF/AdHXn0cC+hqdOxU8ryoQ= When reading token data from sysfs on my Inspiron 3505, the token locations and values are wrong. This happens because match_attribute() blindly assumes that all entries in da_tokens have an associated entry in token_attrs. This however is not true as soon as da_tokens[] contains zeroed token entries. Those entries are being skipped when initialising token_attrs, breaking the core assumption of match_attribute(). Fix this by defining an extra struct for each pair of token attributes and use container_of() to retrieve token information. Tested on a Dell Inspiron 3050. Fixes: 33b9ca1e53b4 ("platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens") Signed-off-by: Armin Wolf Reviewed-by: Ilpo Järvinen --- Changes since v1: - remove style changes - improve sizeof() usage with pointer targets --- drivers/platform/x86/dell/dell-smbios-base.c | 92 ++++++++------------ 1 file changed, 36 insertions(+), 56 deletions(-) -- 2.39.2 diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c index e61bfaf8b5c4..86b95206cb1b 100644 --- a/drivers/platform/x86/dell/dell-smbios-base.c +++ b/drivers/platform/x86/dell/dell-smbios-base.c @@ -11,6 +11,7 @@ */ #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt +#include #include #include #include @@ -25,11 +26,16 @@ static u32 da_supported_commands; static int da_num_tokens; static struct platform_device *platform_device; static struct calling_interface_token *da_tokens; -static struct device_attribute *token_location_attrs; -static struct device_attribute *token_value_attrs; +static struct token_sysfs_data *token_entries; static struct attribute **token_attrs; static DEFINE_MUTEX(smbios_mutex); +struct token_sysfs_data { + struct device_attribute location_attr; + struct device_attribute value_attr; + struct calling_interface_token *token; +}; + struct smbios_device { struct list_head list; struct device *device; @@ -416,47 +422,26 @@ static void __init find_tokens(const struct dmi_header *dm, void *dummy) } } -static int match_attribute(struct device *dev, - struct device_attribute *attr) -{ - int i; - - for (i = 0; i < da_num_tokens * 2; i++) { - if (!token_attrs[i]) - continue; - if (strcmp(token_attrs[i]->name, attr->attr.name) == 0) - return i/2; - } - dev_dbg(dev, "couldn't match: %s\n", attr->attr.name); - return -EINVAL; -} - static ssize_t location_show(struct device *dev, struct device_attribute *attr, char *buf) { - int i; + struct token_sysfs_data *data = container_of(attr, struct token_sysfs_data, location_attr); if (!capable(CAP_SYS_ADMIN)) return -EPERM; - i = match_attribute(dev, attr); - if (i > 0) - return sysfs_emit(buf, "%08x", da_tokens[i].location); - return 0; + return sysfs_emit(buf, "%08x", data->token->location); } static ssize_t value_show(struct device *dev, struct device_attribute *attr, char *buf) { - int i; + struct token_sysfs_data *data = container_of(attr, struct token_sysfs_data, value_attr); if (!capable(CAP_SYS_ADMIN)) return -EPERM; - i = match_attribute(dev, attr); - if (i > 0) - return sysfs_emit(buf, "%08x", da_tokens[i].value); - return 0; + return sysfs_emit(buf, "%08x", data->token->value); } static struct attribute_group smbios_attribute_group = { @@ -473,22 +458,15 @@ static int build_tokens_sysfs(struct platform_device *dev) { char *location_name; char *value_name; - size_t size; int ret; int i, j; - /* (number of tokens + 1 for null terminated */ - size = sizeof(struct device_attribute) * (da_num_tokens + 1); - token_location_attrs = kzalloc(size, GFP_KERNEL); - if (!token_location_attrs) + token_entries = kcalloc(da_num_tokens, sizeof(*token_entries), GFP_KERNEL); + if (!token_entries) return -ENOMEM; - token_value_attrs = kzalloc(size, GFP_KERNEL); - if (!token_value_attrs) - goto out_allocate_value; /* need to store both location and value + terminator*/ - size = sizeof(struct attribute *) * ((2 * da_num_tokens) + 1); - token_attrs = kzalloc(size, GFP_KERNEL); + token_attrs = kcalloc((2 * da_num_tokens) + 1, sizeof(*token_attrs), GFP_KERNEL); if (!token_attrs) goto out_allocate_attrs; @@ -496,27 +474,32 @@ static int build_tokens_sysfs(struct platform_device *dev) /* skip empty */ if (da_tokens[i].tokenID == 0) continue; + + token_entries[i].token = &da_tokens[i]; + /* add location */ location_name = kasprintf(GFP_KERNEL, "%04x_location", da_tokens[i].tokenID); if (location_name == NULL) goto out_unwind_strings; - sysfs_attr_init(&token_location_attrs[i].attr); - token_location_attrs[i].attr.name = location_name; - token_location_attrs[i].attr.mode = 0444; - token_location_attrs[i].show = location_show; - token_attrs[j++] = &token_location_attrs[i].attr; + + sysfs_attr_init(&token_entries[i].location_attr.attr); + token_entries[i].location_attr.attr.name = location_name; + token_entries[i].location_attr.attr.mode = 0444; + token_entries[i].location_attr.show = location_show; + token_attrs[j++] = &token_entries[i].location_attr.attr; /* add value */ value_name = kasprintf(GFP_KERNEL, "%04x_value", da_tokens[i].tokenID); if (value_name == NULL) goto loop_fail_create_value; - sysfs_attr_init(&token_value_attrs[i].attr); - token_value_attrs[i].attr.name = value_name; - token_value_attrs[i].attr.mode = 0444; - token_value_attrs[i].show = value_show; - token_attrs[j++] = &token_value_attrs[i].attr; + + sysfs_attr_init(&token_entries[i].value_attr.attr); + token_entries[i].value_attr.attr.name = value_name; + token_entries[i].value_attr.attr.mode = 0444; + token_entries[i].value_attr.show = value_show; + token_attrs[j++] = &token_entries[i].value_attr.attr; continue; loop_fail_create_value: @@ -532,14 +515,12 @@ static int build_tokens_sysfs(struct platform_device *dev) out_unwind_strings: while (i--) { - kfree(token_location_attrs[i].attr.name); - kfree(token_value_attrs[i].attr.name); + kfree(token_entries[i].location_attr.attr.name); + kfree(token_entries[i].value_attr.attr.name); } kfree(token_attrs); out_allocate_attrs: - kfree(token_value_attrs); -out_allocate_value: - kfree(token_location_attrs); + kfree(token_entries); return -ENOMEM; } @@ -551,12 +532,11 @@ static void free_group(struct platform_device *pdev) sysfs_remove_group(&pdev->dev.kobj, &smbios_attribute_group); for (i = 0; i < da_num_tokens; i++) { - kfree(token_location_attrs[i].attr.name); - kfree(token_value_attrs[i].attr.name); + kfree(token_entries[i].location_attr.attr.name); + kfree(token_entries[i].value_attr.attr.name); } kfree(token_attrs); - kfree(token_value_attrs); - kfree(token_location_attrs); + kfree(token_entries); } static int __init dell_smbios_init(void) From patchwork Tue May 28 20:49:03 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Armin Wolf X-Patchwork-Id: 13677445 X-Patchwork-Delegate: hdegoede@redhat.com Received: from mout.gmx.net (mout.gmx.net [212.227.17.20]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1E6B671753; Tue, 28 May 2024 20:49:20 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=212.227.17.20 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716929363; cv=none; b=SHHoXR6YkyMrQnZFaujOMHCc3Zw3ZqEWQrn7cgNbRX/2BVE2fuI5rIHeD2CIjNjOb61xLLDuMvFInH2zb1ZbD93w5ea0W2WzycBx+lPNd4TAjz3X2JPujjL53+FTom9Uz1SiRia7u0FpGk/yE/dTPFN4SsBca64EbyLGhFij3QI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1716929363; c=relaxed/simple; bh=cd/SbaQnWmAq08MyAtXZ8t1qjAc7VyezJTSklhCEWbI=; h=From:To:Cc:Subject:Date:Message-Id:In-Reply-To:References: MIME-Version; b=BFa1HAacjk033FiJXH94pIg+8SxfIMlWAQ/oUW3112wF8FVFvC03Wi/tojskG5AErvpTVtC3Ndv6sFAI6n4MLlhbQOa2FCEtnCKfb62ZLlOO73IcLNATQgwABEwKwpTxAiOEfTbmOSJqS4ZCc716Z94W1AztNMaSscuCEbILGDs= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gmx.de; spf=pass smtp.mailfrom=gmx.de; dkim=pass (2048-bit key) header.d=gmx.de header.i=w_armin@gmx.de header.b=SZC3uMbY; arc=none smtp.client-ip=212.227.17.20 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=gmx.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmx.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmx.de header.i=w_armin@gmx.de header.b="SZC3uMbY" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmx.de; s=s31663417; t=1716929350; x=1717534150; i=w_armin@gmx.de; bh=Hpk5WF3fBlOFgzw+1dpUyIelI8tfyqNS40822E06XGM=; h=X-UI-Sender-Class:From:To:Cc:Subject:Date:Message-Id:In-Reply-To: References:MIME-Version:Content-Transfer-Encoding:cc: content-transfer-encoding:content-type:date:from:message-id: mime-version:reply-to:subject:to; b=SZC3uMbYBs6x5oiXHLHfM0iwLrzsRr8ZG8oMDaFhWIb5fiwokvvJeZ1w7auYSGDW BaexouWpjObywthVDMN9FExkOcJBFePrO+0JoNhNZQcr0VjBSeRH0pK5tL76kQnQS xZhAbroe69b2ja7m3rdo4qTofwCiyAQXfGLVXcv+J1xHig7vagH/keOdPamq86qJI S6Oiuuvm6B+FYpI+LhFqxyK8RPqFi9s6XF1c8bj6bPxk4iw1kJQEW8eujZGR1/Hxu EN5ilQwh3yUGcvVgjEC5n/5FDLlq9v8ySI3VKeB0y45BRYqpPAXyYSwb9Bbw15bsi gAlD6jsBKl7xq6xpcw== X-UI-Sender-Class: 724b4f7f-cbec-4199-ad4e-598c01a50d3a Received: from mx-amd-b650.users.agdsn.de ([141.30.226.129]) by mail.gmx.net (mrgmx104 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MTiTt-1s4c1r39tF-00QT5R; Tue, 28 May 2024 22:49:10 +0200 From: Armin Wolf To: hdegoede@redhat.com, ilpo.jarvinen@linux.intel.com Cc: platform-driver-x86@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 2/2] platform/x86: dell-smbios: Simplify error handling Date: Tue, 28 May 2024 22:49:03 +0200 Message-Id: <20240528204903.445546-2-W_Armin@gmx.de> X-Mailer: git-send-email 2.39.2 In-Reply-To: <20240528204903.445546-1-W_Armin@gmx.de> References: <20240528204903.445546-1-W_Armin@gmx.de> Precedence: bulk X-Mailing-List: platform-driver-x86@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Provags-ID: V03:K1:+UIIXA77L1iidDRteJr97oG5jSONXoEfxxwv8Sc7Sqyg2sOT6bK +CDe82mMUFq1flfWdR6Z9xrDhji4iQjLCvqsWVTIZ6HeLnlwnt9dZeZg7u91M8gJcznlcTS FTFDpSKNf9ZGho/2FoWVF1KI94FWINvg+yyNqR460N2rOo7TbvZsSYlez7Vhed/cMRZUjqC WvMTzTc2gXyyRtKlL0SYQ== X-Spam-Flag: NO UI-OutboundReport: notjunk:1;M01:P0:FogjPxr2wa8=;hd8yRzrswIPSCQ+KJkOZU38R25L X0r3J75YxvTyvuzR7DBPYpf4umYmpAUhOjQW9i1FP8n5ROJ15+dlwgNbmrQT1qmqIvQrzVmGb LbBGj+P8coJdKZPLzZ0Xjyga3QoWBuVWcftz91pihotC3W9H0hfsolfjgJF4bGDFgc2JbbINS G4dUjziG1VHUQyBvU1BV4vbAOnlWMDTJuWNX44qzwZ1FWG8kftVQbSX0s6/hsSCyr0V6ll0uB zTzAerIaQF2j6qP+0VZz0ELzDRHlIdrGGuKBZkPIwLqtQK6BCPu5hBMnQwhyOCuaePQceiclP DWPb9PzWEVejZGJgql5TNivqGzRV0D7FtVM2enw23fML8NzBNNCrnoKEpOhaE824rjRijqBVy wq8sRinYJdjusXfRkdE8mpN+1fPc+K3jhVS4vwmAjpzVSKcqS/lYK1c7OGinToNQRheTvwTJ/ ZA0AtusQ5HQy1P+6+p3HOZemZ9hwtb0CWsn0uJ5mKE7ObNIRDijv4qOWHLQ1y3z6l52j516RT JR4tiAYdk5HMyM/bnrkm5DBLaGFiJ6fPGLBy1vMFAuW7YyZiajaa95xfjhSlzT+7piXe8sUsl uJV0LTgXNkW9Xxpoa8L7hITpRiHfXDEvo2Ur6hjC56PjczD0pcDeqfRkJw/0SusMFUBIAoIz0 X8gig856l0EqPWXDVEo9YjYuNWcdF0LOsS9OGogTu2P6ia/OFVDuBzvoxpQMyio746K37VH0i otjJn4yHQMzGtIAuVQAvPuf6jiUXeXqoq5oK9tEgduirzqF4bZvvPlf7M5uTzEfafwnBue4eb KkeQ2RwNmhv8aIWeZY9HAo12gwd7vQ4yV7uHsO4jjWjNA= When the allocation of value_name fails, the error handling code uses two gotos for error handling, which is not necessary. Simplify the error handling in this case by only using a single goto. Tested on a Dell Inspiron 3505. Signed-off-by: Armin Wolf Reviewed-by: Ilpo Järvinen --- Changes since v1: - add patch --- drivers/platform/x86/dell/dell-smbios-base.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) -- 2.39.2 diff --git a/drivers/platform/x86/dell/dell-smbios-base.c b/drivers/platform/x86/dell/dell-smbios-base.c index 86b95206cb1b..b562ed99ec4e 100644 --- a/drivers/platform/x86/dell/dell-smbios-base.c +++ b/drivers/platform/x86/dell/dell-smbios-base.c @@ -492,19 +492,16 @@ static int build_tokens_sysfs(struct platform_device *dev) /* add value */ value_name = kasprintf(GFP_KERNEL, "%04x_value", da_tokens[i].tokenID); - if (value_name == NULL) - goto loop_fail_create_value; + if (!value_name) { + kfree(location_name); + goto out_unwind_strings; + } sysfs_attr_init(&token_entries[i].value_attr.attr); token_entries[i].value_attr.attr.name = value_name; token_entries[i].value_attr.attr.mode = 0444; token_entries[i].value_attr.show = value_show; token_attrs[j++] = &token_entries[i].value_attr.attr; - continue; - -loop_fail_create_value: - kfree(location_name); - goto out_unwind_strings; } smbios_attribute_group.attrs = token_attrs;