[PATCH] ecryptfs: streamline offset formatting in ecryptfs_derive_iv

[PATCH] ecryptfs: streamline offset formatting in ecryptfs_derive_iv

[Thorsten Blum]

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));

Re: [PATCH] ecryptfs: streamline offset formatting in ecryptfs_derive_iv

[Eric Biggers]

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

Re: [PATCH] ecryptfs: streamline offset formatting in ecryptfs_derive_iv

[Thorsten Blum]

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).

Re: [PATCH] ecryptfs: streamline offset formatting in ecryptfs_derive_iv

[Thorsten Blum]

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!