On Thu, 13 Nov 2025 22:55:45 +0100 Thorsten Blum <[email protected]> wrote:
> strcpy() is deprecated; use the safer strscpy() and use its return > value, the number of bytes copied, instead of calling strlen() on the > destination buffer again. String truncation can be ignored since > 'derived_buf' is guaranteed to be large enough. > > Link: https://github.com/KSPP/linux/issues/88 > Signed-off-by: Thorsten Blum <[email protected]> > --- > Changes in v2: > - Revert some changes to include the trailing '\0' even if key_type == 0 > since this would change the bytes passed to sha256() (Eric Biggers) > - Use strscpy() and its return value instead > - Link to v1: > https://lore.kernel.org/lkml/[email protected]/ > --- > security/keys/encrypted-keys/encrypted.c | 21 ++++++++------------- > 1 file changed, 8 insertions(+), 13 deletions(-) > > diff --git a/security/keys/encrypted-keys/encrypted.c > b/security/keys/encrypted-keys/encrypted.c > index 15841466b5d4..94408909f1dd 100644 > --- a/security/keys/encrypted-keys/encrypted.c > +++ b/security/keys/encrypted-keys/encrypted.c > @@ -12,6 +12,7 @@ > */ > > #include <linux/uaccess.h> > +#include <linux/minmax.h> > #include <linux/module.h> > #include <linux/init.h> > #include <linux/slab.h> > @@ -330,23 +331,17 @@ static int get_derived_key(u8 *derived_key, enum > derived_key_type key_type, > const u8 *master_key, size_t master_keylen) > { > u8 *derived_buf; > - unsigned int derived_buf_len; > - > - derived_buf_len = strlen("AUTH_KEY") + 1 + master_keylen; > - if (derived_buf_len < HASH_SIZE) > - derived_buf_len = HASH_SIZE; > + size_t derived_buf_len; > + const char *key_name; > + ssize_t len; > > + derived_buf_len = max(strlen("AUTH_KEY") + 1 + master_keylen, > HASH_SIZE); > derived_buf = kzalloc(derived_buf_len, GFP_KERNEL); > if (!derived_buf) > return -ENOMEM; > - > - if (key_type) > - strcpy(derived_buf, "AUTH_KEY"); > - else > - strcpy(derived_buf, "ENC_KEY"); > - > - memcpy(derived_buf + strlen(derived_buf) + 1, master_key, > - master_keylen); > + key_name = key_type ? "AUTH_KEY" : "ENC_KEY"; > + len = strscpy(derived_buf, key_name, derived_buf_len); > + memcpy(derived_buf + len + 1, master_key, master_keylen); > sha256(derived_buf, derived_buf_len, derived_key); I'm not sure this is an improvement, but has this code ever been correct? The buffer passed to sha256 is either: "AUTH_KEY"'\0'master_key or "ENC_KEY"'\0'master_key For short master_key the buffer is HASH_SIZE bytes and padded with zeros (ok). However for long master_key the length is calculated using "AUTH_KEY" so there is an additional trailing '\0' in the "ENC_KEY" case. David > kfree_sensitive(derived_buf); > return 0;
