Re: [PATCH] ecryptfs: replace ecryptfs_from_hex() with hex2bin()

[Tyler Hicks <code@tyhicks.com>]

Hello - Thanks for the patch!

On 2026-08-29 20:45:28, Drif Abdelmalek Mohamed Said wrote:
> ecryptfs_from_hex() relied on the deprecated simple_strtoul(). Rather
> than patching it to use kstrtoul(), its intended replacement, remove it
> entirely and switch all call sites to the generic hex2bin() helper
> from <linux/hex.h>, which already provides the same functionality.

This breaks the build, as none of the callers check the return code yet
hex2bin() is marked as __must_check in include/linux/hex.h:

  CC [M]  keystore.o
keystore.c: In function ‘ecryptfs_write_tag_70_packet’:
keystore.c:717:9: error: ignoring return value of ‘hex2bin’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
  717 |         hex2bin((u8 *)&dest[s->i],
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~
  718 |                           mount_crypt_stat->global_default_fnek_sig,
      |                           ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  719 |                           ECRYPTFS_SIG_SIZE);
      |                           ~~~~~~~~~~~~~~~~~~
keystore.c: In function ‘write_tag_3_packet’:
keystore.c:2183:9: error: ignoring return value of ‘hex2bin’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
 2183 |         hex2bin((u8 *)key_rec->sig, auth_tok->token.password.signature,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2184 |                           ECRYPTFS_SIG_SIZE);
      |                           ~~~~~~~~~~~~~~~~~~
keystore.c: In function ‘write_tag_1_packet’:
keystore.c:2019:9: error: ignoring return value of ‘hex2bin’ declared with attribute ‘warn_unused_result’ [-Werror=unused-result]
 2019 |         hex2bin((u8 *)key_rec->sig, auth_tok->token.private_key.signature,
      |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 2020 |                           ECRYPTFS_SIG_SIZE);
      |                           ~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors

> 
> Update call sites with the (u8 *) casts required by hex2bin()'s
> signature.
> 
> All call sites guarantee src is composed only of valid hex characters

I don't think that's true. The first call site I looked at,
ecryptfs_write_tag_70_packet(), is using a src of global_default_fnek_sig
that's initialized from a mount option that ecryptfs_parse_param() only
verifies is a string. There's no hex character check that I could see.

I didn't look at the other call sites.

> and that src is at least 2*dst_size bytes, so hex2bin()'s stricter
> error handling is never triggered here.
> 
> Signed-off-by: Drif Abdelmalek Mohamed Said <drifabdelmalekmohamedsaid@gmail.com>
> ---
>  fs/ecryptfs/crypto.c          | 18 ------------------
>  fs/ecryptfs/ecryptfs_kernel.h |  1 -
>  fs/ecryptfs/keystore.c        |  7 ++++---
>  3 files changed, 4 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/ecryptfs/crypto.c b/fs/ecryptfs/crypto.c
> index e67119b6029c..7e802fd631a6 100644
> --- a/fs/ecryptfs/crypto.c
> +++ b/fs/ecryptfs/crypto.c
> @@ -29,24 +29,6 @@
>  #define DECRYPT		0
>  #define ENCRYPT		1
>  
> -/**
> - * ecryptfs_from_hex
> - * @dst: Buffer to take the bytes from src hex; must be at least of
> - *       size (src_size / 2)
> - * @src: Buffer to be converted from a hex string representation to raw value
> - * @dst_size: size of dst buffer, or number of hex characters pairs to convert
> - */
> -void ecryptfs_from_hex(char *dst, char *src, int dst_size)
> -{
> -	int x;
> -	char tmp[3] = { 0, };
> -
> -	for (x = 0; x < dst_size; x++) {
> -		tmp[0] = src[x * 2];
> -		tmp[1] = src[x * 2 + 1];
> -		dst[x] = (unsigned char)simple_strtol(tmp, NULL, 16);
> -	}
> -}
>  

Please don't leave two empty lines next to each other.

>  static int ecryptfs_crypto_api_algify_cipher_name(char **algified_name,
>  						  const char *cipher_name,
> diff --git a/fs/ecryptfs/ecryptfs_kernel.h b/fs/ecryptfs/ecryptfs_kernel.h
> index 58165928ed1e..be91f7c3fc58 100644
> --- a/fs/ecryptfs/ecryptfs_kernel.h
> +++ b/fs/ecryptfs/ecryptfs_kernel.h
> @@ -47,7 +47,6 @@ ecryptfs_to_hex(char *dst, char *src, size_t src_size)
>  	*end = '\0';
>  }
>  
> -extern void ecryptfs_from_hex(char *dst, char *src, int dst_size);
>  

Same here.

Tyler

>  struct ecryptfs_key_record {
>  	unsigned char type;
> diff --git a/fs/ecryptfs/keystore.c b/fs/ecryptfs/keystore.c
> index 51651314b7a6..dcfb2e5fa88e 100644
> --- a/fs/ecryptfs/keystore.c
> +++ b/fs/ecryptfs/keystore.c
> @@ -18,6 +18,7 @@
>  #include <linux/random.h>
>  #include <linux/scatterlist.h>
>  #include <linux/slab.h>
> +#include <linux/hex.h>
>  #include "ecryptfs_kernel.h"
>  
>  /*
> @@ -713,7 +714,7 @@ ecryptfs_write_tag_70_packet(char *dest, size_t *remaining_bytes,
>  		goto out_free_unlock;
>  	}
>  	s->i += s->packet_size_len;
> -	ecryptfs_from_hex(&dest[s->i],
> +	hex2bin((u8 *)&dest[s->i],
>  			  mount_crypt_stat->global_default_fnek_sig,
>  			  ECRYPTFS_SIG_SIZE);
>  	s->i += ECRYPTFS_SIG_SIZE;
> @@ -2015,7 +2016,7 @@ write_tag_1_packet(char *dest, size_t *remaining_bytes,
>  	int rc = 0;
>  
>  	(*packet_size) = 0;
> -	ecryptfs_from_hex(key_rec->sig, auth_tok->token.private_key.signature,
> +	hex2bin((u8 *)key_rec->sig, auth_tok->token.private_key.signature,
>  			  ECRYPTFS_SIG_SIZE);
>  	encrypted_session_key_valid = 0;
>  	for (i = 0; i < crypt_stat->key_size; i++)
> @@ -2179,7 +2180,7 @@ write_tag_3_packet(char *dest, size_t *remaining_bytes,
>  	int rc = 0;
>  
>  	(*packet_size) = 0;
> -	ecryptfs_from_hex(key_rec->sig, auth_tok->token.password.signature,
> +	hex2bin((u8 *)key_rec->sig, auth_tok->token.password.signature,
>  			  ECRYPTFS_SIG_SIZE);
>  	rc = ecryptfs_get_tfm_and_mutex_for_cipher_name(&tfm, &tfm_mutex,
>  							crypt_stat->cipher);
> -- 
> 2.43.0
>