mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] ecryptfs: replace ecryptfs_from_hex() with hex2bin()
@ 2026-08-29 19:45 Drif Abdelmalek Mohamed Said
  2026-09-10 11:17 ` DRIF ABDELMALEK MOHAMED SAID
  2026-10-13  5:45 ` Tyler Hicks
  0 siblings, 2 replies; 3+ messages in thread
From: Drif Abdelmalek Mohamed Said @ 2026-08-29 19:45 UTC (permalink / raw)
  To: Tyler Hicks
  Cc: Thorsten Blum, Eric Biggers, Christian Brauner, ecryptfs,
	linux-kernel, Drif Abdelmalek Mohamed Said

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.

Update call sites with the (u8 *) casts required by hex2bin()'s
signature.

All call sites guarantee src is composed only of valid hex characters
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);
-	}
-}
 
 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);
 
 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


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ecryptfs: replace ecryptfs_from_hex() with hex2bin()
  2026-08-29 19:45 [PATCH] ecryptfs: replace ecryptfs_from_hex() with hex2bin() Drif Abdelmalek Mohamed Said
@ 2026-09-10 11:17 ` DRIF ABDELMALEK MOHAMED SAID
  2026-10-13  5:45 ` Tyler Hicks
  1 sibling, 0 replies; 3+ messages in thread
From: DRIF ABDELMALEK MOHAMED SAID @ 2026-09-10 11:17 UTC (permalink / raw)
  To: Tyler Hicks
  Cc: Thorsten Blum, Eric Biggers, Christian Brauner, ecryptfs, linux-kernel

Gentle ping on this one — seems to have gotten buried during the merge window.

Thanks,

On Sat, Aug 29, 2026 at 8:45 PM Drif Abdelmalek Mohamed Said
<drifabdelmalekmohamedsaid@gmail.com> 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.
>
> Update call sites with the (u8 *) casts required by hex2bin()'s
> signature.
>
> All call sites guarantee src is composed only of valid hex characters
> 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);
> -       }
> -}
>
>  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);
>
>  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
>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] ecryptfs: replace ecryptfs_from_hex() with hex2bin()
  2026-08-29 19:45 [PATCH] ecryptfs: replace ecryptfs_from_hex() with hex2bin() Drif Abdelmalek Mohamed Said
  2026-09-10 11:17 ` DRIF ABDELMALEK MOHAMED SAID
@ 2026-10-13  5:45 ` Tyler Hicks
  1 sibling, 0 replies; 3+ messages in thread
From: Tyler Hicks @ 2026-10-13  5:45 UTC (permalink / raw)
  To: Drif Abdelmalek Mohamed Said
  Cc: Thorsten Blum, Eric Biggers, Christian Brauner, ecryptfs, linux-kernel

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
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-10-13  5:45 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-29 19:45 [PATCH] ecryptfs: replace ecryptfs_from_hex() with hex2bin() Drif Abdelmalek Mohamed Said
2026-09-10 11:17 ` DRIF ABDELMALEK MOHAMED SAID
2026-10-13  5:45 ` Tyler Hicks

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®