Use the number of characters written by scnprintf() to zero-pad the
remaining bytes, instead of clearing the buffer first and then writing
the offset.

Fix a typo in the kernel-doc and remove the TODO from 2006 while at it.

Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev>
---
 fs/ecryptfs/crypto.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
index 3b59346d68c5..7fac3ec1a8cd 100644
--- a/fs/ecryptfs/crypto.c
+++ b/fs/ecryptfs/crypto.c
@@ -72,7 +72,7 @@ static int ecryptfs_crypto_api_algify_cipher_name(char **algified_name,
 
 /**
  * ecryptfs_derive_iv
- * @iv: destination for the derived iv vale
+ * @iv: destination for the derived iv value
  * @crypt_stat: Pointer to crypt_stat struct for the current inode
  * @offset: Offset of the extent whose IV we are to derive
  *
@@ -84,18 +84,15 @@ void ecryptfs_derive_iv(char *iv, struct ecryptfs_crypt_stat *crypt_stat,
 {
 	char dst[MD5_DIGEST_SIZE];
 	char src[ECRYPTFS_MAX_IV_BYTES + 16];
+	size_t len;
 
 	if (unlikely(ecryptfs_verbosity > 0)) {
 		ecryptfs_printk(KERN_DEBUG, "root iv:\n");
 		ecryptfs_dump_hex(crypt_stat->root_iv, crypt_stat->iv_bytes);
 	}
-	/* TODO: It is probably secure to just cast the least
-	 * significant bits of the root IV into an unsigned long and
-	 * add the offset to that rather than go through all this
-	 * hashing business. -Halcrow */
 	memcpy(src, crypt_stat->root_iv, crypt_stat->iv_bytes);
-	memset((src + crypt_stat->iv_bytes), 0, 16);
-	snprintf((src + crypt_stat->iv_bytes), 16, "%lld", offset);
+	len = scnprintf(src + crypt_stat->iv_bytes, 16, "%lld", offset) + 1;
+	memset(src + crypt_stat->iv_bytes + len, 0, 16 - len);
 	if (unlikely(ecryptfs_verbosity > 0)) {
 		ecryptfs_printk(KERN_DEBUG, "source:\n");
 		ecryptfs_dump_hex(src, (crypt_stat->iv_bytes + 16));

On Sun, Mar 29, 2026 at 11:23:25PM +0200, Thorsten Blum wrote:
> Use the number of characters written by scnprintf() to zero-pad the
> remaining bytes, instead of clearing the buffer first and then writing
> the offset.
[...]
> +	size_t len;
[...]
>  	memcpy(src, crypt_stat->root_iv, crypt_stat->iv_bytes);
> -	memset((src + crypt_stat->iv_bytes), 0, 16);
> -	snprintf((src + crypt_stat->iv_bytes), 16, "%lld", offset);
> +	len = scnprintf(src + crypt_stat->iv_bytes, 16, "%lld", offset) + 1;
> +	memset(src + crypt_stat->iv_bytes + len, 0, 16 - len);

This isn't exactly "streamlining" the code.  memset(p, 0, 16) tends to
get compiled into just two instructions.  In contrast, a variable-length
memset tends to be several instructions to set up, plus a call
instruction, and the instructions inside memset() itself.  scnprintf()
is also a few more instructions than snprintf().

So I'd say the old version is more "streamlined", actually.  Granted,
the difference is probably only a few cycles, but it sounds like the
motivation for this patch is that you assumed the new version is faster?

- Eric

On Sun, Mar 29, 2026 at 03:05:05PM -0700, Eric Biggers wrote:
> On Sun, Mar 29, 2026 at 11:23:25PM +0200, Thorsten Blum wrote:
> > Use the number of characters written by scnprintf() to zero-pad the
> > remaining bytes, instead of clearing the buffer first and then writing
> > the offset.
> [...]
> > +	size_t len;
> [...]
> >  	memcpy(src, crypt_stat->root_iv, crypt_stat->iv_bytes);
> > -	memset((src + crypt_stat->iv_bytes), 0, 16);
> > -	snprintf((src + crypt_stat->iv_bytes), 16, "%lld", offset);
> > +	len = scnprintf(src + crypt_stat->iv_bytes, 16, "%lld", offset) + 1;
> > +	memset(src + crypt_stat->iv_bytes + len, 0, 16 - len);
> 
> This isn't exactly "streamlining" the code.  memset(p, 0, 16) tends to
> get compiled into just two instructions.  In contrast, a variable-length
> memset tends to be several instructions to set up, plus a call
> instruction, and the instructions inside memset() itself.  scnprintf()
> is also a few more instructions than snprintf().
> 
> So I'd say the old version is more "streamlined", actually.  Granted,
> the difference is probably only a few cycles, but it sounds like the
> motivation for this patch is that you assumed the new version is faster?

I meant "streamline" as in write bytes once.

Maybe we should just zero-initialize the 32 bytes in 'src' instead and
keep snprintf(). That removes the need to keep track of 'len' and also
gets rid of the explicit memset(0).

On Mon, Mar 30, 2026 at 03:00:04AM +0200, Thorsten Blum wrote:
> On Sun, Mar 29, 2026 at 03:05:05PM -0700, Eric Biggers wrote:
> > This isn't exactly "streamlining" the code.  memset(p, 0, 16) tends to
> > get compiled into just two instructions.  In contrast, a variable-length
> > memset tends to be several instructions to set up, plus a call
> > instruction, and the instructions inside memset() itself.  scnprintf()
> > is also a few more instructions than snprintf().
> > 
> > So I'd say the old version is more "streamlined", actually.  Granted,
> > the difference is probably only a few cycles, but it sounds like the
> > motivation for this patch is that you assumed the new version is faster?
> 
> I meant "streamline" as in write bytes once.
> 
> Maybe we should just zero-initialize the 32 bytes in 'src' instead and
> keep snprintf(). That removes the need to keep track of 'len' and also
> gets rid of the explicit memset(0).

Let's drop this patch and I'll send a new one to fix the typo and remove
the TODO. Keeping the explicit memset(0) is probably best here. Thanks!