[PATCH] net: can: remove custom hex_to_bin()

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Wolfgang Grandegger <redacted>
---
 drivers/net/can/slcan.c |   26 +++++---------------------
 1 files changed, 5 insertions(+), 21 deletions(-)

diff --git a/drivers/net/can/slcan.c b/drivers/net/can/slcan.c
index aa8ad73..65e54fd 100644
--- a/drivers/net/can/slcan.c
+++ b/drivers/net/can/slcan.c

@@ -56,6 +56,7 @@
 #include <linux/sched.h>
 #include <linux/delay.h>
 #include <linux/init.h>
+#include <linux/kernel.h>
 #include <linux/can.h>
 
 static __initdata const char banner[] =

@@ -142,21 +143,6 @@ static struct net_device **slcan_devs;
   *			STANDARD SLCAN DECAPSULATION			 *
   ************************************************************************/
 
-static int asc2nibble(char c)
-{
-
-	if ((c >= '0') && (c <= '9'))
-		return c - '0';
-
-	if ((c >= 'A') && (c <= 'F'))
-		return c - 'A' + 10;
-
-	if ((c >= 'a') && (c <= 'f'))
-		return c - 'a' + 10;
-
-	return 16; /* error */
-}
-
 /* Send one completely decapsulated can_frame to the network layer */
 static void slc_bump(struct slcan *sl)
 {

@@ -195,18 +181,16 @@ static void slc_bump(struct slcan *sl)
 	*(u64 *) (&cf.data) = 0; /* clear payload */
 
 	for (i = 0, dlc_pos++; i < cf.can_dlc; i++) {
-
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] = (tmp << 4);
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] |= tmp;
 	}
 
-
 	skb = dev_alloc_skb(sizeof(struct can_frame));
 	if (!skb)
 		return;

-- 
1.7.5.4

Andy Shevchenko wrote:

 	for (i = 0, dlc_pos++; i < cf.can_dlc; i++) {
-
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] = (tmp << 4);
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] |= tmp;
 	}

What about changing

  void hex2bin(u8 *dst, const char *src, size_t count)

to

  bool hex2bin(u8 *dst, const char *src, size_t count)

in order to do error checks like

bool hex2bin_with_validation(u8 *dst, const char *src, size_t count)
{
	while (count--) {
		int c = hex_to_bin(*src++);
		int d;
		if (c < 0)
			return false;
		d = hex_to_bin(*src++)
		if (d < 0)
			return false;
		*dst++ = (c << 4) | d;
	}
	return true;
}

and use hex2bin() rather than hex_to_bin()?

On Mon, 2011-07-18 at 20:41 +0900, Tetsuo Handa wrote: 

Andy Shevchenko wrote:

quoted

 	for (i = 0, dlc_pos++; i < cf.can_dlc; i++) {
-
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] = (tmp << 4);
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] |= tmp;
 	}

What about changing

  void hex2bin(u8 *dst, const char *src, size_t count)

to

  bool hex2bin(u8 *dst, const char *src, size_t count)

in order to do error checks like

bool hex2bin_with_validation(u8 *dst, const char *src, size_t count)
{
	while (count--) {
		int c = hex_to_bin(*src++);
		int d;
		if (c < 0)
			return false;
		d = hex_to_bin(*src++)
		if (d < 0)
			return false;
		*dst++ = (c << 4) | d;
	}
	return true;
}

and use hex2bin() rather than hex_to_bin()?

Perhaps, good idea. Could you submit a patch?

-- 
Andy Shevchenko [off-list ref]
Intel Finland Oy

Currently, security/keys/ is the only user of hex2bin().
Should I keep hex2bin() unmodified in case of bad input?
If so, I'd like to make it as hex2bin_safe().
----------------------------------------
[PATCH] Add error check to hex2bin().

Since converting 2 hexadecimal letters into a byte with error checks is
commonly used, we can replace multiple hex_to_bin() calls with single hex2bin()
call by changing hex2bin() to do error checks.

Signed-off-by: Tetsuo Handa <penguin-kernel@I=love.SAKURA.ne.jp>
---

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 953352a..186e9fc 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h

@@ -374,7 +374,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 }
 
 extern int hex_to_bin(char ch);
-extern void hex2bin(u8 *dst, const char *src, size_t count);
+extern bool hex2bin(u8 *dst, const char *src, size_t count);
 
 /*
  * General tracing related utility functions - trace_printk(),

diff --git a/lib/hexdump.c b/lib/hexdump.c
index f5fe6ba..1524002 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c

@@ -38,14 +38,22 @@ EXPORT_SYMBOL(hex_to_bin);
  * @dst: binary result
  * @src: ascii hexadecimal string
  * @count: result length
+ *
+ * Returns true on success, false in case of bad input.
  */
-void hex2bin(u8 *dst, const char *src, size_t count)
+bool hex2bin(u8 *dst, const char *src, size_t count)
 {
 	while (count--) {
-		*dst = hex_to_bin(*src++) << 4;
-		*dst += hex_to_bin(*src++);
-		dst++;
+		int c = hex_to_bin(*src++);
+		int d;
+		if (c < 0)
+			return false;
+		d = hex_to_bin(*src++);
+		if (d < 0)
+			return false;
+		*dst++ = (c << 4) | d;
 	}
+	return true;
 }
 EXPORT_SYMBOL(hex2bin);
 

In message "Re: [PATCH] net: can: remove custom hex_to_bin()",
Andy Shevchenko wrote:

On Mon, 2011-07-18 at 20:41 +0900, Tetsuo Handa wrote: 

quoted

Andy Shevchenko wrote:

quoted

 	for (i = 0, dlc_pos++; i < cf.can_dlc; i++) {
-
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] = (tmp << 4);
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] |= tmp;
 	}

What about changing

  void hex2bin(u8 *dst, const char *src, size_t count)

to

  bool hex2bin(u8 *dst, const char *src, size_t count)

in order to do error checks like

bool hex2bin_with_validation(u8 *dst, const char *src, size_t count)
{
	while (count--) {
		int c = hex_to_bin(*src++);
		int d;
		if (c < 0)
			return false;
		d = hex_to_bin(*src++)
		if (d < 0)
			return false;
		*dst++ = (c << 4) | d;
	}
	return true;
}

and use hex2bin() rather than hex_to_bin()?

Perhaps, good idea. Could you submit a patch?

-- 
Andy Shevchenko [off-list ref]
Intel Finland Oy

On 18.07.2011 10:26, Andy Shevchenko wrote:

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Wolfgang Grandegger <redacted>

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

On Mon, 2011-07-18 at 21:48 +0900, Tetsuo Handa wrote:

Currently, security/keys/ is the only user of hex2bin().
Should I keep hex2bin() unmodified in case of bad input?
If so, I'd like to make it as hex2bin_safe().

Missing blank line here.

+		if (c < 0)
+			return false;
+		d = hex_to_bin(*src++);
+		if (d < 0)
+			return false;
+		*dst++ = (c << 4) | d;
 	}
+	return true;
 }
 EXPORT_SYMBOL(hex2bin);

We probably don't need to define a separate 'safe' function.

Instead of changing the existing code to short circuit out and return a
value, does only adding the return value work?  Something like:

        bool ret = true;
        int c, d;

        while (count--) {
                c = hex_to_bin(*src++);
                d = hex_to_bin(*src++);
                *dst++ = (c << 4) | d;

                if (c < 0 || d < 0)
                        ret = false;
        }
        return ret;

thanks,

Mimi

In message "Re: [PATCH] net: can: remove custom hex_to_bin()",
Andy Shevchenko wrote:

quoted

On Mon, 2011-07-18 at 20:41 +0900, Tetsuo Handa wrote: 

quoted

Andy Shevchenko wrote:

quoted

 	for (i = 0, dlc_pos++; i < cf.can_dlc; i++) {
-
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] = (tmp << 4);
-		tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-		if (tmp > 0x0F)
+		tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+		if (tmp < 0)
 			return;
 		cf.data[i] |= tmp;
 	}

What about changing

  void hex2bin(u8 *dst, const char *src, size_t count)

to

  bool hex2bin(u8 *dst, const char *src, size_t count)

in order to do error checks like

bool hex2bin_with_validation(u8 *dst, const char *src, size_t count)
{
	while (count--) {
		int c = hex_to_bin(*src++);
		int d;
		if (c < 0)
			return false;
		d = hex_to_bin(*src++)
		if (d < 0)
			return false;
		*dst++ = (c << 4) | d;
	}
	return true;
}

and use hex2bin() rather than hex_to_bin()?

Perhaps, good idea. Could you submit a patch?

-- 
Andy Shevchenko [off-list ref]
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Oliver Hartkopp <socketcan@hartkopp.net>
Date: Mon, 18 Jul 2011 16:02:49 +0200

On 18.07.2011 10:26, Andy Shevchenko wrote:

quoted

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Wolfgang Grandegger <redacted>

Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>

Applied.

On Mon, Jul 18, 2011 at 9:03 PM, Mimi Zohar [off-list ref] wrote:

On Mon, 2011-07-18 at 21:48 +0900, Tetsuo Handa wrote:

quoted

Currently, security/keys/ is the only user of hex2bin().
Should I keep hex2bin() unmodified in case of bad input?
If so, I'd like to make it as hex2bin_safe().

quoted

----------------------------------------
[PATCH] Add error check to hex2bin().

Since converting 2 hexadecimal letters into a byte with error checks is
commonly used, we can replace multiple hex_to_bin() calls with single hex2bin()
call by changing hex2bin() to do error checks.

Signed-off-by: Tetsuo Handa <penguin-kernel@I=love.SAKURA.ne.jp>
---

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 953352a..186e9fc 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h

@@ -374,7 +374,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)

 }

 extern int hex_to_bin(char ch);
-extern void hex2bin(u8 *dst, const char *src, size_t count);
+extern bool hex2bin(u8 *dst, const char *src, size_t count);

 /*
  * General tracing related utility functions - trace_printk(),

diff --git a/lib/hexdump.c b/lib/hexdump.c
index f5fe6ba..1524002 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c

@@ -38,14 +38,22 @@ EXPORT_SYMBOL(hex_to_bin);

  * @dst: binary result
  * @src: ascii hexadecimal string
  * @count: result length
+ *
+ * Returns true on success, false in case of bad input.
  */
-void hex2bin(u8 *dst, const char *src, size_t count)
+bool hex2bin(u8 *dst, const char *src, size_t count)
 {
      while (count--) {
-             *dst = hex_to_bin(*src++) << 4;
-             *dst += hex_to_bin(*src++);
-             dst++;
+             int c = hex_to_bin(*src++);
+             int d;

Missing blank line here.

quoted

+             if (c < 0)
+                     return false;
+             d = hex_to_bin(*src++);
+             if (d < 0)
+                     return false;
+             *dst++ = (c << 4) | d;
      }
+     return true;
 }
 EXPORT_SYMBOL(hex2bin);

We probably don't need to define a separate 'safe' function.

There is an opponent on any  approach. Although, small and fast error
route could be good.

Instead of changing the existing code to short circuit out and return a
value, does only adding the return value work?  Something like:

       bool ret = true;
       int c, d;

       while (count--) {
               c = hex_to_bin(*src++);
               d = hex_to_bin(*src++);

Here is a performance issue, yeah. The user prefers to know about an
error as soon as possible.

               *dst++ = (c << 4) | d;

               if (c < 0 || d < 0)
                       ret = false;

The ret value is redundant, and here you continue to fill the result
array by something arbitrary (might be wrong data).

       }
       return ret;

thanks,

Mimi

quoted

In message "Re: [PATCH] net: can: remove custom hex_to_bin()",
Andy Shevchenko wrote:

quoted

On Mon, 2011-07-18 at 20:41 +0900, Tetsuo Handa wrote:

quoted

Andy Shevchenko wrote:

quoted

        for (i = 0, dlc_pos++; i < cf.can_dlc; i++) {
-
-               tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-               if (tmp > 0x0F)
+               tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+               if (tmp < 0)
                        return;
                cf.data[i] = (tmp << 4);
-               tmp = asc2nibble(sl->rbuff[dlc_pos++]);
-               if (tmp > 0x0F)
+               tmp = hex_to_bin(sl->rbuff[dlc_pos++]);
+               if (tmp < 0)
                        return;
                cf.data[i] |= tmp;
        }

What about changing

  void hex2bin(u8 *dst, const char *src, size_t count)

to

  bool hex2bin(u8 *dst, const char *src, size_t count)

in order to do error checks like

bool hex2bin_with_validation(u8 *dst, const char *src, size_t count)
{
  while (count--) {
          int c = hex_to_bin(*src++);
          int d;
          if (c < 0)
                  return false;
          d = hex_to_bin(*src++)
          if (d < 0)
                  return false;
          *dst++ = (c << 4) | d;
  }
  return true;
}

and use hex2bin() rather than hex_to_bin()?

Perhaps, good idea. Could you submit a patch?

--
Andy Shevchenko [off-list ref]
Intel Finland Oy

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

-- 
With Best Regards,
Andy Shevchenko

On Mon, 2011-07-18 at 21:57 +0300, Andy Shevchenko wrote:

On Mon, Jul 18, 2011 at 9:03 PM, Mimi Zohar [off-list ref] wrote:

quoted

On Mon, 2011-07-18 at 21:48 +0900, Tetsuo Handa wrote:

quoted

Currently, security/keys/ is the only user of hex2bin().
Should I keep hex2bin() unmodified in case of bad input?
If so, I'd like to make it as hex2bin_safe().

quoted

----------------------------------------
[PATCH] Add error check to hex2bin().

Since converting 2 hexadecimal letters into a byte with error checks is
commonly used, we can replace multiple hex_to_bin() calls with single hex2bin()
call by changing hex2bin() to do error checks.

Signed-off-by: Tetsuo Handa <penguin-kernel@I=love.SAKURA.ne.jp>
---

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 953352a..186e9fc 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h

@@ -374,7 +374,7 @@ static inline char *pack_hex_byte(char *buf, u8 byte)
 }

 extern int hex_to_bin(char ch);
-extern void hex2bin(u8 *dst, const char *src, size_t count);
+extern bool hex2bin(u8 *dst, const char *src, size_t count);

 /*
  * General tracing related utility functions - trace_printk(),

diff --git a/lib/hexdump.c b/lib/hexdump.c
index f5fe6ba..1524002 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c

@@ -38,14 +38,22 @@ EXPORT_SYMBOL(hex_to_bin);
  * @dst: binary result
  * @src: ascii hexadecimal string
  * @count: result length
+ *
+ * Returns true on success, false in case of bad input.
  */
-void hex2bin(u8 *dst, const char *src, size_t count)
+bool hex2bin(u8 *dst, const char *src, size_t count)
 {
      while (count--) {
-             *dst = hex_to_bin(*src++) << 4;
-             *dst += hex_to_bin(*src++);
-             dst++;
+             int c = hex_to_bin(*src++);
+             int d;

Missing blank line here.

quoted

+             if (c < 0)
+                     return false;
+             d = hex_to_bin(*src++);
+             if (d < 0)
+                     return false;
+             *dst++ = (c << 4) | d;
      }
+     return true;
 }
 EXPORT_SYMBOL(hex2bin);

We probably don't need to define a separate 'safe' function.

There is an opponent on any  approach. Although, small and fast error
route could be good.

As nothing but trusted/encrypted keys is using hex2bin, it shouldn't be
a problem.  :-)  I'll update trusted/encrypted keys to check the return
code.

thanks,

Mimi

quoted

Instead of changing the existing code to short circuit out and return a
value, does only adding the return value work?  Something like:

       bool ret = true;
       int c, d;

       while (count--) {
               c = hex_to_bin(*src++);
               d = hex_to_bin(*src++);

Here is a performance issue, yeah. The user prefers to know about an
error as soon as possible.

ok

quoted

               *dst++ = (c << 4) | d;

               if (c < 0 || d < 0)
                       ret = false;

The ret value is redundant, and here you continue to fill the result
array by something arbitrary (might be wrong data).

quoted

       }
       return ret;

On Mon, Jul 18, 2011 at 10:20 PM, Mimi Zohar [off-list ref] wrote:

quoted

quoted

We probably don't need to define a separate 'safe' function.

There is an opponent on any  approach. Although, small and fast error
route could be good.

As nothing but trusted/encrypted keys is using hex2bin, it shouldn't be
a problem.  :-)

The key word "until now". But people will start to use anything which
has public API, won't they?

 I'll update trusted/encrypted keys to check the return
code.

Actually another question shall we add __must_check to the prototype or not?

-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Jul 18, 2011 at 14:48, Tetsuo Handa
[off-list ref] wrote:

What about making it return the number of unprocessed bytes left instead?
Then the caller knows where the problem lies. And zero would mean success.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Jul 18, 2011 at 11:49 PM, Geert Uytterhoeven
[off-list ref] wrote:

What about making it return the number of unprocessed bytes left instead?
Then the caller knows where the problem lies. And zero would mean success.

If I remember correctly it used to be src as return value in some
version of that patch. I don't know the details of that interim
solution. My current opinion is to return boolean and make an
additional parameter to return src value. However, it could make this
simple function fat.
P.S. Take into account that the user of it is only one so far, I would
like to hear a Mimi's opinion.

-- 
With Best Regards,
Andy Shevchenko

On Mon, 2011-07-18 at 22:44 +0300, Andy Shevchenko wrote:

On Mon, Jul 18, 2011 at 10:20 PM, Mimi Zohar [off-list ref] wrote:

quoted

quoted

quoted

We probably don't need to define a separate 'safe' function.

There is an opponent on any  approach. Although, small and fast error
route could be good.

quoted

As nothing but trusted/encrypted keys is using hex2bin, it shouldn't be
a problem.  :-)

The key word "until now". But people will start to use anything which
has public API, won't they?

Someone with more experience than me needs to responds.

quoted

 I'll update trusted/encrypted keys to check the return
code.

Actually another question shall we add __must_check to the prototype or not?

Probably a good idea.

thanks,

Mimi

(sorry for re-posting, but this doesn't seem to have made it to the
lists.)

On Tue, 2011-07-19 at 00:18 +0300, Andy Shevchenko wrote:

On Mon, Jul 18, 2011 at 11:49 PM, Geert Uytterhoeven
[off-list ref] wrote:

quoted

What about making it return the number of unprocessed bytes left instead?
Then the caller knows where the problem lies. And zero would mean success.

If I remember correctly it used to be src as return value in some
version of that patch. I don't know the details of that interim
solution. My current opinion is to return boolean and make an
additional parameter to return src value. However, it could make this
simple function fat.
P.S. Take into account that the user of it is only one so far, I would
like to hear a Mimi's opinion.

Trusted/encrypted keys are not in a critical code path. They're used for
loading/storing key blobs from userspace.  From a trusted/encrypted key
perspective, it doesn't make much of a difference.

thanks,

Mimi

On Tue, 2011-07-19 at 00:18 +0300, Andy Shevchenko wrote:

On Mon, Jul 18, 2011 at 11:49 PM, Geert Uytterhoeven
[off-list ref] wrote:

quoted

What about making it return the number of unprocessed bytes left instead?
Then the caller knows where the problem lies. And zero would mean success.

If I remember correctly it used to be src as return value in some
version of that patch. I don't know the details of that interim
solution. My current opinion is to return boolean and make an
additional parameter to return src value. However, it could make this
simple function fat.
P.S. Take into account that the user of it is only one so far, I would
like to hear a Mimi's opinion.

Trusted/encrypted keys are not in a critical code path. They're used for
loading/storing key blobs from userspace. Once you change the API, short
circuiting out and adding an error return, from a trusted/encrypted key
perspective, it doesn't make a difference.

thanks,

Mimi

(sorry for re-posting, but this doesn't seem to have been sent.)

On Tue, 2011-07-19 at 00:18 +0300, Andy Shevchenko wrote:

On Mon, Jul 18, 2011 at 11:49 PM, Geert Uytterhoeven
[off-list ref] wrote:

quoted

What about making it return the number of unprocessed bytes left instead?
Then the caller knows where the problem lies. And zero would mean success.

If I remember correctly it used to be src as return value in some
version of that patch. I don't know the details of that interim
solution. My current opinion is to return boolean and make an
additional parameter to return src value. However, it could make this
simple function fat.
P.S. Take into account that the user of it is only one so far, I would
like to hear a Mimi's opinion.

Trusted/encrypted keys are not in a critical code path. They're used for
loading/storing key blobs from userspace. Once you change the API, short
circuiting out and adding an error return, from a trusted/encrypted key
perspective, it doesn't make a difference.

thanks,

Mimi