summaryrefslogtreecommitdiff
path: root/fs/smb/client/smb2ops.c
diff options
context:
space:
mode:
authorJustin Stitt <justinstitt@google.com>2024-03-28 21:44:48 +0000
committerSteve French <stfrench@microsoft.com>2024-03-31 17:35:14 -0500
commitebd9779683aaf089ad0173862553cdd3288ad9b4 (patch)
treeb52c6d48feb81b7c316d78ed7bb96be2723af7f3 /fs/smb/client/smb2ops.c
parent39cd87c4eb2b893354f3b850f916353f2658ae6f (diff)
smb: client: replace deprecated strncpy with strscpy
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 cifssmb.c: Using strncpy with a length argument equal to strlen(src) is generally dangerous because it can cause string buffers to not be NUL-terminated. In this case, however, there was extra effort made to ensure the buffer was NUL-terminated via a manual NUL-byte assignment. In an effort to rid the kernel of strncpy() use, let's swap over to using strscpy() which guarantees NUL-termination on the destination buffer. To handle the case where ea_name is NULL, let's use the ?: operator to substitute in an empty string, thereby allowing strscpy to still NUL-terminate the destintation string. Interesting note: this flex array buffer may go on to also have some value encoded after the NUL-termination: | if (ea_value_len) | memcpy(parm_data->list.name + name_len + 1, | ea_value, ea_value_len); Now for smb2ops.c and smb2transport.c: Both of these cases are simple, strncpy() is used to copy string literals which have a length less than the destination buffer's size. We can simply swap in the new 2-argument version of strscpy() introduced in Commit e6584c3964f2f ("string: Allow 2-argument strscpy()"). 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: 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> Signed-off-by: Steve French <stfrench@microsoft.com>
Diffstat (limited to 'fs/smb/client/smb2ops.c')
-rw-r--r--fs/smb/client/smb2ops.c2
1 files changed, 1 insertions, 1 deletions
diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c
index 2ed456948f34..35bf7eb315cd 100644
--- a/fs/smb/client/smb2ops.c
+++ b/fs/smb/client/smb2ops.c
@@ -3913,7 +3913,7 @@ smb21_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock,
strcat(message, "W");
}
if (!new_oplock)
- strncpy(message, "None", sizeof(message));
+ strscpy(message, "None");
cinode->oplock = new_oplock;
cifs_dbg(FYI, "%s Lease granted on inode %p\n", message,