ext4: replace deprecated strncpy with alternatives

strncpy() is deprecated for use on NUL-terminated destination strings
[1] and as such we should prefer more robust and less ambiguous string
interfaces.

in file.c:
s_last_mounted is marked as __nonstring meaning it does not need to be
NUL-terminated. Let's instead use strtomem_pad() to copy bytes from the
string source to the byte array destination -- while also ensuring to
pad with zeroes.

in ioctl.c:
We can drop the memset and size argument in favor of using the new
2-argument version of strscpy_pad() -- which was introduced with Commit
e6584c3964f2f ("string: Allow 2-argument strscpy()"). This guarantees
NUL-termination and NUL-padding on the destination buffer -- which seems
to be a requirement judging from this comment:

|	static int ext4_ioctl_getlabel(struct ext4_sb_info *sbi, char __user *user_label)
|	{
|		char label[EXT4_LABEL_MAX + 1];
|
|		/*
|		 * EXT4_LABEL_MAX must always be smaller than FSLABEL_MAX because
|		 * FSLABEL_MAX must include terminating null byte, while s_volume_name
|		 * does not have to.
|		 */

in super.c:
s_first_error_func is marked as __nonstring meaning we can take the same
approach as in file.c; just use strtomem_pad()

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings
Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Signed-off-by: Justin Stitt <justinstitt@google.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Link: https://lore.kernel.org/r/20240321-strncpy-fs-ext4-file-c-v1-1-36a6a09fef0c@google.com
Signed-off-by: Theodore Ts'o <tytso@mit.edu>

diff --git a/fs/ext4/file.c b/fs/ext4/file.c
index 965febab1d0484d453a0d5cb525f12626a3d11d9..77529c655f95862bf77ae80d0c5a84c1f0be94a8 100644 (file)
--- a/fs/ext4/file.c
+++ b/fs/ext4/file.c
@@ -844,8 +844,7 @@ static int ext4_sample_last_mounted(struct super_block *sb,
        if (err)
                goto out_journal;
        lock_buffer(sbi->s_sbh);
-       strncpy(sbi->s_es->s_last_mounted, cp,
-               sizeof(sbi->s_es->s_last_mounted));
+       strtomem_pad(sbi->s_es->s_last_mounted, cp, 0);
        ext4_superblock_csum_set(sb);
        unlock_buffer(sbi->s_sbh);
        ext4_handle_dirty_metadata(handle, NULL, sbi->s_sbh);

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 7160a71044c88a8fe409111ec51cd597408f98f4..dab7acd497092369ef8b3a7e677a300de55269e0 100644 (file)
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -1150,9 +1150,8 @@ static int ext4_ioctl_getlabel(struct ext4_sb_info *sbi, char __user *user_label
         */
        BUILD_BUG_ON(EXT4_LABEL_MAX >= FSLABEL_MAX);
 
-       memset(label, 0, sizeof(label));
        lock_buffer(sbi->s_sbh);
-       strncpy(label, sbi->s_es->s_volume_name, EXT4_LABEL_MAX);
+       strscpy_pad(label, sbi->s_es->s_volume_name);
        unlock_buffer(sbi->s_sbh);
 
        if (copy_to_user(user_label, label, sizeof(label)))

diff --git a/fs/ext4/super.c b/fs/ext4/super.c
index 9bde300b155135644ba6d08a27542a27de47d047..f9b2b18374f3d0f3d9890e0b2090819fa31a620d 100644 (file)
--- a/fs/ext4/super.c
+++ b/fs/ext4/super.c
@@ -6129,8 +6129,8 @@ static void ext4_update_super(struct super_block *sb)
                        __ext4_update_tstamp(&es->s_first_error_time,
                                             &es->s_first_error_time_hi,
                                             sbi->s_first_error_time);
-                       strncpy(es->s_first_error_func, sbi->s_first_error_func,
-                               sizeof(es->s_first_error_func));
+                       strtomem_pad(es->s_first_error_func,
+                                    sbi->s_first_error_func, 0);
                        es->s_first_error_line =
                                cpu_to_le32(sbi->s_first_error_line);
                        es->s_first_error_ino =
@@ -6143,8 +6143,7 @@ static void ext4_update_super(struct super_block *sb)
                __ext4_update_tstamp(&es->s_last_error_time,
                                     &es->s_last_error_time_hi,
                                     sbi->s_last_error_time);
-               strncpy(es->s_last_error_func, sbi->s_last_error_func,
-                       sizeof(es->s_last_error_func));
+               strtomem_pad(es->s_last_error_func, sbi->s_last_error_func, 0);
                es->s_last_error_line = cpu_to_le32(sbi->s_last_error_line);
                es->s_last_error_ino = cpu_to_le32(sbi->s_last_error_ino);
                es->s_last_error_block = cpu_to_le64(sbi->s_last_error_block);