printk: cleanup deprecated uses of strncpy/strcpy
authorJustin Stitt <justinstitt@google.com>
Mon, 29 Apr 2024 23:06:54 +0000 (23:06 +0000)
committerPetr Mladek <pmladek@suse.com>
Tue, 7 May 2024 08:41:51 +0000 (10:41 +0200)
Cleanup some deprecated uses of strncpy() and strcpy() [1].

There doesn't seem to be any bugs with the current code but the
readability of this code could benefit from a quick makeover while
removing some deprecated stuff as a benefit.

The most interesting replacement made in this patch involves
concatenating "ttyS" with a digit-led user-supplied string. Instead of
doing two distinct string copies with carefully managed offsets and
lengths, let's use the more robust and self-explanatory scnprintf().
scnprintf will 1) respect the bounds of @buf, 2) null-terminate @buf, 3)
do the concatenation. This allows us to drop the manual NUL-byte assignment.

Also, since isdigit() is used about a dozen lines after the open-coded
version we'll replace it for uniformity's sake.

All the strcpy() --> strscpy() replacements are trivial as the source
strings are literals and much smaller than the destination size. No
behavioral change here.

Use the new 2-argument version of strscpy() introduced in Commit
e6584c3964f2f ("string: Allow 2-argument strscpy()"). However, to make
this work fully (since the size must be known at compile time), also
update the extern-qualified declaration to have the proper size
information.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
Link: https://github.com/KSPP/linux/issues/90
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Link: https://lore.kernel.org/r/20240429-strncpy-kernel-printk-printk-c-v1-1-4da7926d7b69@google.com
[pmladek@suse.com: Removed obsolete brackets and added empty lines.]
Signed-off-by: Petr Mladek <pmladek@suse.com>
include/linux/printk.h
kernel/printk/printk.c

index 2fde40cc96778ec6b943224b16fd75a6171c90c6..dbbd202b1cb314d4bfae029bd44db6c4154b5068 100644 (file)
@@ -71,7 +71,7 @@ extern void console_verbose(void);
 
 /* strlen("ratelimit") + 1 */
 #define DEVKMSG_STR_MAX_SIZE 10
-extern char devkmsg_log_str[];
+extern char devkmsg_log_str[DEVKMSG_STR_MAX_SIZE];
 struct ctl_table;
 
 extern int suppress_printk;
index b06f63e276c1f6680975ba26790f8044704d9543..360182dfe4cfeb933d6e421db4e7c8777e4df319 100644 (file)
@@ -178,9 +178,9 @@ static int __init control_devkmsg(char *str)
         * Set sysctl string accordingly:
         */
        if (devkmsg_log == DEVKMSG_LOG_MASK_ON)
-               strcpy(devkmsg_log_str, "on");
+               strscpy(devkmsg_log_str, "on");
        else if (devkmsg_log == DEVKMSG_LOG_MASK_OFF)
-               strcpy(devkmsg_log_str, "off");
+               strscpy(devkmsg_log_str, "off");
        /* else "ratelimit" which is set by default. */
 
        /*
@@ -209,7 +209,7 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
                        return -EINVAL;
 
                old = devkmsg_log;
-               strncpy(old_str, devkmsg_log_str, DEVKMSG_STR_MAX_SIZE);
+               strscpy(old_str, devkmsg_log_str);
        }
 
        err = proc_dostring(table, write, buffer, lenp, ppos);
@@ -227,7 +227,7 @@ int devkmsg_sysctl_set_loglvl(struct ctl_table *table, int write,
 
                        /* ... and restore old setting. */
                        devkmsg_log = old;
-                       strncpy(devkmsg_log_str, old_str, DEVKMSG_STR_MAX_SIZE);
+                       strscpy(devkmsg_log_str, old_str);
 
                        return -EINVAL;
                }
@@ -2500,22 +2500,22 @@ static int __init console_setup(char *str)
        /*
         * Decode str into name, index, options.
         */
-       if (str[0] >= '0' && str[0] <= '9') {
-               strcpy(buf, "ttyS");
-               strncpy(buf + 4, str, sizeof(buf) - 5);
-       } else {
-               strncpy(buf, str, sizeof(buf) - 1);
-       }
-       buf[sizeof(buf) - 1] = 0;
+       if (isdigit(str[0]))
+               scnprintf(buf, sizeof(buf), "ttyS%s", str);
+       else
+               strscpy(buf, str);
+
        options = strchr(str, ',');
        if (options)
                *(options++) = 0;
+
 #ifdef __sparc__
        if (!strcmp(str, "ttya"))
-               strcpy(buf, "ttyS0");
+               strscpy(buf, "ttyS0");
        if (!strcmp(str, "ttyb"))
-               strcpy(buf, "ttyS1");
+               strscpy(buf, "ttyS1");
 #endif
+
        for (s = buf; *s; s++)
                if (isdigit(*s) || *s == ',')
                        break;