ipmi: refactor deprecated strncpy

`strncpy` is deprecated for use on NUL-terminated destination strings [1].

In this case, strncpy is being used specifically for its NUL-padding
behavior (and has been commented as such). Moreover, the destination
string is not required to be NUL-terminated [2].

We can use a more robust and less ambiguous interface in
`memcpy_and_pad` which makes the code more readable and even eliminates
the need for that comment.

Let's also use `strnlen` instead of `strlen()` with an upper-bounds
check as this is intrinsically a part of `strnlen`.

Also included in this patch is a simple 1:1 change of `strncpy` to
`strscpy` for ipmi_ssif.c. If NUL-padding is wanted here as well then we
should opt again for `strscpy_pad`.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
Link: https://lore.kernel.org/all/ZQEADYBl0uZ1nX60@mail.minyard.net/
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: Justin Stitt <justinstitt@google.com>
Message-Id: <20230913-strncpy-drivers-char-ipmi-ipmi-v2-1-e3bc0f6e599f@google.com>
Signed-off-by: Corey Minyard <minyard@acm.org>

diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index 186f1fee753403f7b0e0feac76cc7bfd4f20f373..d6f14279684de18b2e894bc26de77407e4a05095 100644 (file)
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -5377,20 +5377,15 @@ static void send_panic_events(struct ipmi_smi *intf, char *str)
 
        j = 0;
        while (*p) {
-               int size = strlen(p);
+               int size = strnlen(p, 11);
 
-               if (size > 11)
-                       size = 11;
                data[0] = 0;
                data[1] = 0;
                data[2] = 0xf0; /* OEM event without timestamp. */
                data[3] = intf->addrinfo[0].address;
                data[4] = j++; /* sequence # */
-               /*
-                * Always give 11 bytes, so strncpy will fill
-                * it with zeroes for me.
-                */
-               strncpy(data+5, p, 11);
+
+               memcpy_and_pad(data+5, 11, p, size, '\0');
                p += size;
 
                ipmi_panic_request_and_wait(intf, &addr, &msg);

diff --git a/drivers/char/ipmi/ipmi_ssif.c b/drivers/char/ipmi/ipmi_ssif.c
index df8dd50b4cbed055fce122627b28435bce7ab2e4..1f7600c361e6857c816998ef6867d6290cc3aeb8 100644 (file)
--- a/drivers/char/ipmi/ipmi_ssif.c
+++ b/drivers/char/ipmi/ipmi_ssif.c
@@ -1945,7 +1945,7 @@ static int new_ssif_client(int addr, char *adapter_name,
                }
        }
 
-       strncpy(addr_info->binfo.type, DEVICE_NAME,
+       strscpy(addr_info->binfo.type, DEVICE_NAME,
                sizeof(addr_info->binfo.type));
        addr_info->binfo.addr = addr;
        addr_info->binfo.platform_data = addr_info;