summaryrefslogtreecommitdiffstats
path: root/drivers/platform/x86/thinkpad_acpi.c
diff options
context:
space:
mode:
authorJustin Stitt <justinstitt@google.com>2023-10-20 17:52:43 +0000
committerIlpo Järvinen <ilpo.jarvinen@linux.intel.com>2023-10-23 16:58:21 +0300
commit942a4a61b64e182d756a1a5776aa500d3b3d862f (patch)
tree547d6dcabd90fd7efff5eda42699dd2de9b58a01 /drivers/platform/x86/thinkpad_acpi.c
parente485c7a1939d9b1e6ed2a728e15009b44074f131 (diff)
downloadlinux-stable-942a4a61b64e182d756a1a5776aa500d3b3d862f.tar.gz
linux-stable-942a4a61b64e182d756a1a5776aa500d3b3d862f.tar.bz2
linux-stable-942a4a61b64e182d756a1a5776aa500d3b3d862f.zip
platform/x86: thinkpad_acpi: replace deprecated strncpy with memcpy
strncpy() is deprecated for use on NUL-terminated destination strings [1] and as such we should prefer more robust and less ambiguous interfaces. We expect ec_fw_string to be NUL-terminated based on its use with format strings in thinkpad_acpi.c: 11241 | pr_notice("ThinkPad firmware release %s doesn't match the known patterns\n", 11242 | ec_fw_string); Moreover, NUL-padding is not required since ec_fw_string is explicitly zero-initialized: 11185 | char ec_fw_string[18] = {0}; When carefully copying bytes from one buffer to another in pre-determined blocks (like what's happening here with dmi_data): | static void find_new_ec_fwstr(const struct dmi_header *dm, void *private) | { | char *ec_fw_string = (char *) private; | const char *dmi_data = (const char *)dm; | /* | * ThinkPad Embedded Controller Program Table on newer models | * | * Offset | Name | Width | Description | * ---------------------------------------------------- | * 0x00 | Type | BYTE | 0x8C | * 0x01 | Length | BYTE | | * 0x02 | Handle | WORD | Varies | * 0x04 | Signature | BYTEx6 | ASCII for "LENOVO" | * 0x0A | OEM struct offset | BYTE | 0x0B | * 0x0B | OEM struct number | BYTE | 0x07, for this structure | * 0x0C | OEM struct revision | BYTE | 0x01, for this format | * 0x0D | ECP version ID | STR ID | | * 0x0E | ECP release date | STR ID | | */ | | /* Return if data structure not match */ | if (dm->type != 140 || dm->length < 0x0F || | memcmp(dmi_data + 4, "LENOVO", 6) != 0 || | dmi_data[0x0A] != 0x0B || dmi_data[0x0B] != 0x07 || | dmi_data[0x0C] != 0x01) | return; | | /* fwstr is the first 8byte string */ | strncpy(ec_fw_string, dmi_data + 0x0F, 8); ... we shouldn't be using a C string api. Let's instead use memcpy() as this more properly relays the intended behavior. Do note that ec_fw_string will still end up being NUL-terminated since we are memcpy'ing only 8 bytes into a buffer full of 18 zeroes. There's still some trailing NUL-bytes there. To ensure this behavior, let's add a BUILD_BUG_ON checking the length leaves space for at least one trailing NUL-byte. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1] Link: https://github.com/KSPP/linux/issues/90 Cc: Kees Cook <keescook@chromium.org> Signed-off-by: Justin Stitt <justinstitt@google.com> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca> Reviewed-by: Kees Cook <keescook@chromium.org> Link: https://lore.kernel.org/r/20231020-strncpy-drivers-platform-x86-thinkpad_acpi-c-v1-1-312f2e33034f@google.com Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
Diffstat (limited to 'drivers/platform/x86/thinkpad_acpi.c')
-rw-r--r--drivers/platform/x86/thinkpad_acpi.c7
1 files changed, 5 insertions, 2 deletions
diff --git a/drivers/platform/x86/thinkpad_acpi.c b/drivers/platform/x86/thinkpad_acpi.c
index 9c19624a7454..c3499bf60582 100644
--- a/drivers/platform/x86/thinkpad_acpi.c
+++ b/drivers/platform/x86/thinkpad_acpi.c
@@ -11226,6 +11226,8 @@ invalid:
return '\0';
}
+#define EC_FW_STRING_LEN 18
+
static void find_new_ec_fwstr(const struct dmi_header *dm, void *private)
{
char *ec_fw_string = (char *) private;
@@ -11254,7 +11256,8 @@ static void find_new_ec_fwstr(const struct dmi_header *dm, void *private)
return;
/* fwstr is the first 8byte string */
- strncpy(ec_fw_string, dmi_data + 0x0F, 8);
+ BUILD_BUG_ON(EC_FW_STRING_LEN <= 8);
+ memcpy(ec_fw_string, dmi_data + 0x0F, 8);
}
/* returns 0 - probe ok, or < 0 - probe error.
@@ -11264,7 +11267,7 @@ static int __must_check __init get_thinkpad_model_data(
struct thinkpad_id_data *tp)
{
const struct dmi_device *dev = NULL;
- char ec_fw_string[18] = {0};
+ char ec_fw_string[EC_FW_STRING_LEN] = {0};
char const *s;
char t;