Skip to content

Commit 744a563

Browse files
JustinStitttytso
authored andcommitted
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 e6584c3 ("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 [1] Link: https://manpages.debian.org/testing/linux-manual-4.8/strscpy.9.en.html [2] Link: KSPP#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>
1 parent e19089d commit 744a563

3 files changed

Lines changed: 5 additions & 8 deletions

File tree

fs/ext4/file.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -844,8 +844,7 @@ static int ext4_sample_last_mounted(struct super_block *sb,
844844
if (err)
845845
goto out_journal;
846846
lock_buffer(sbi->s_sbh);
847-
strncpy(sbi->s_es->s_last_mounted, cp,
848-
sizeof(sbi->s_es->s_last_mounted));
847+
strtomem_pad(sbi->s_es->s_last_mounted, cp, 0);
849848
ext4_superblock_csum_set(sb);
850849
unlock_buffer(sbi->s_sbh);
851850
ext4_handle_dirty_metadata(handle, NULL, sbi->s_sbh);

fs/ext4/ioctl.c

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1150,9 +1150,8 @@ static int ext4_ioctl_getlabel(struct ext4_sb_info *sbi, char __user *user_label
11501150
*/
11511151
BUILD_BUG_ON(EXT4_LABEL_MAX >= FSLABEL_MAX);
11521152

1153-
memset(label, 0, sizeof(label));
11541153
lock_buffer(sbi->s_sbh);
1155-
strncpy(label, sbi->s_es->s_volume_name, EXT4_LABEL_MAX);
1154+
strscpy_pad(label, sbi->s_es->s_volume_name);
11561155
unlock_buffer(sbi->s_sbh);
11571156

11581157
if (copy_to_user(user_label, label, sizeof(label)))

fs/ext4/super.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6129,8 +6129,8 @@ static void ext4_update_super(struct super_block *sb)
61296129
__ext4_update_tstamp(&es->s_first_error_time,
61306130
&es->s_first_error_time_hi,
61316131
sbi->s_first_error_time);
6132-
strncpy(es->s_first_error_func, sbi->s_first_error_func,
6133-
sizeof(es->s_first_error_func));
6132+
strtomem_pad(es->s_first_error_func,
6133+
sbi->s_first_error_func, 0);
61346134
es->s_first_error_line =
61356135
cpu_to_le32(sbi->s_first_error_line);
61366136
es->s_first_error_ino =
@@ -6143,8 +6143,7 @@ static void ext4_update_super(struct super_block *sb)
61436143
__ext4_update_tstamp(&es->s_last_error_time,
61446144
&es->s_last_error_time_hi,
61456145
sbi->s_last_error_time);
6146-
strncpy(es->s_last_error_func, sbi->s_last_error_func,
6147-
sizeof(es->s_last_error_func));
6146+
strtomem_pad(es->s_last_error_func, sbi->s_last_error_func, 0);
61486147
es->s_last_error_line = cpu_to_le32(sbi->s_last_error_line);
61496148
es->s_last_error_ino = cpu_to_le32(sbi->s_last_error_ino);
61506149
es->s_last_error_block = cpu_to_le64(sbi->s_last_error_block);

0 commit comments

Comments
 (0)