Skip to content

Commit f700b71

Browse files
JustinStittkees
authored andcommitted
fs: ecryptfs: 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. A good alternative is strscpy() as it guarantees NUL-termination on the destination buffer. In crypto.c: We expect cipher_name to be NUL-terminated based on its use with the C-string format specifier %s and with other string apis like strlen(): | printk(KERN_ERR "Error attempting to initialize key TFM " | "cipher with name = [%s]; rc = [%d]\n", | tmp_tfm->cipher_name, rc); and | int cipher_name_len = strlen(cipher_name); In main.c: We can remove the manual NUL-byte assignments as well as the pointers to destinations (which I assume only existed to trim down on line length?) in favor of directly using the destination buffer which allows the compiler to get size information -- enabling the usage of the new 2-argument strscpy(). Note that this patch relies on the _new_ 2-argument versions of strscpy() and strscpy_pad() introduced in Commit e6584c3 ("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: KSPP#90 Cc: <[email protected]> Signed-off-by: Justin Stitt <[email protected]> Reviewed-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/20240321-strncpy-fs-ecryptfs-crypto-c-v1-1-d78b74c214ac@google.com Signed-off-by: Kees Cook <[email protected]>
1 parent 7dcbf17 commit f700b71

File tree

2 files changed

+7
-23
lines changed

2 files changed

+7
-23
lines changed

fs/ecryptfs/crypto.c

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1606,9 +1606,7 @@ ecryptfs_add_new_key_tfm(struct ecryptfs_key_tfm **key_tfm, char *cipher_name,
16061606
goto out;
16071607
}
16081608
mutex_init(&tmp_tfm->key_tfm_mutex);
1609-
strncpy(tmp_tfm->cipher_name, cipher_name,
1610-
ECRYPTFS_MAX_CIPHER_NAME_SIZE);
1611-
tmp_tfm->cipher_name[ECRYPTFS_MAX_CIPHER_NAME_SIZE] = '\0';
1609+
strscpy(tmp_tfm->cipher_name, cipher_name);
16121610
tmp_tfm->key_size = key_size;
16131611
rc = ecryptfs_process_key_cipher(&tmp_tfm->key_tfm,
16141612
tmp_tfm->cipher_name,

fs/ecryptfs/main.c

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,8 @@ static int ecryptfs_parse_options(struct ecryptfs_sb_info *sbi, char *options,
256256
substring_t args[MAX_OPT_ARGS];
257257
int token;
258258
char *sig_src;
259-
char *cipher_name_dst;
260259
char *cipher_name_src;
261-
char *fn_cipher_name_dst;
262260
char *fn_cipher_name_src;
263-
char *fnek_dst;
264261
char *fnek_src;
265262
char *cipher_key_bytes_src;
266263
char *fn_cipher_key_bytes_src;
@@ -293,12 +290,8 @@ static int ecryptfs_parse_options(struct ecryptfs_sb_info *sbi, char *options,
293290
case ecryptfs_opt_cipher:
294291
case ecryptfs_opt_ecryptfs_cipher:
295292
cipher_name_src = args[0].from;
296-
cipher_name_dst =
297-
mount_crypt_stat->
298-
global_default_cipher_name;
299-
strncpy(cipher_name_dst, cipher_name_src,
300-
ECRYPTFS_MAX_CIPHER_NAME_SIZE);
301-
cipher_name_dst[ECRYPTFS_MAX_CIPHER_NAME_SIZE] = '\0';
293+
strscpy(mount_crypt_stat->global_default_cipher_name,
294+
cipher_name_src);
302295
cipher_name_set = 1;
303296
break;
304297
case ecryptfs_opt_ecryptfs_key_bytes:
@@ -326,11 +319,8 @@ static int ecryptfs_parse_options(struct ecryptfs_sb_info *sbi, char *options,
326319
break;
327320
case ecryptfs_opt_fnek_sig:
328321
fnek_src = args[0].from;
329-
fnek_dst =
330-
mount_crypt_stat->global_default_fnek_sig;
331-
strncpy(fnek_dst, fnek_src, ECRYPTFS_SIG_SIZE_HEX);
332-
mount_crypt_stat->global_default_fnek_sig[
333-
ECRYPTFS_SIG_SIZE_HEX] = '\0';
322+
strscpy(mount_crypt_stat->global_default_fnek_sig,
323+
fnek_src);
334324
rc = ecryptfs_add_global_auth_tok(
335325
mount_crypt_stat,
336326
mount_crypt_stat->global_default_fnek_sig,
@@ -348,12 +338,8 @@ static int ecryptfs_parse_options(struct ecryptfs_sb_info *sbi, char *options,
348338
break;
349339
case ecryptfs_opt_fn_cipher:
350340
fn_cipher_name_src = args[0].from;
351-
fn_cipher_name_dst =
352-
mount_crypt_stat->global_default_fn_cipher_name;
353-
strncpy(fn_cipher_name_dst, fn_cipher_name_src,
354-
ECRYPTFS_MAX_CIPHER_NAME_SIZE);
355-
mount_crypt_stat->global_default_fn_cipher_name[
356-
ECRYPTFS_MAX_CIPHER_NAME_SIZE] = '\0';
341+
strscpy(mount_crypt_stat->global_default_fn_cipher_name,
342+
fn_cipher_name_src);
357343
fn_cipher_name_set = 1;
358344
break;
359345
case ecryptfs_opt_fn_cipher_key_bytes:

0 commit comments

Comments
 (0)