linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/4] lib: hex2bin error checking
@ 2011-09-20 19:52 Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 1/4] lib: add error checking to hex2bin Mimi Zohar
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Mimi Zohar @ 2011-09-20 19:52 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, linux-kernel, Andrew Morton, Andy Shevchenko,
	Tetsuo Handa, Arnaud Lacombe, James Morris, David Safford

hex2bin converts a hexadecimal string to its binary representation.

This version simply adds error checking to the existing hex2bin().
On success, hex2bin returns 0, on failure -1.

Changelog v1
- refreshed the trusted, encrypted, 'target' patches
- removed unpack_hex_byte()
- changed return code from boolean to int

Mimi Zohar (4):
  lib: add error checking to hex2bin
  trusted-keys: check hex2bin result
  encrypted-keys: check hex2bin result
  target: check hex2bin result

 drivers/target/target_core_fabric_lib.c  |   11 +++++++++--
 include/linux/kernel.h                   |    2 +-
 lib/hexdump.c                            |   15 +++++++++++----
 security/keys/encrypted-keys/encrypted.c |   14 +++++++++++---
 security/keys/trusted.c                  |   19 +++++++++++++++----
 5 files changed, 47 insertions(+), 14 deletions(-)

-- 
1.7.3.4


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

* [PATCH v1 1/4] lib: add error checking to hex2bin
  2011-09-20 19:52 [PATCH v1 0/4] lib: hex2bin error checking Mimi Zohar
@ 2011-09-20 19:52 ` Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 2/4] trusted-keys: check hex2bin result Mimi Zohar
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2011-09-20 19:52 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, linux-kernel, Andrew Morton, Andy Shevchenko,
	Tetsuo Handa, Arnaud Lacombe, James Morris, David Safford

hex2bin converts a hexadecimal string to its binary representation.
The original version of hex2bin did not do any error checking.  This
patch adds error checking and returns the result.

Changelog v1:
- removed unpack_hex_byte()
- changed return code from boolean to int

Changelog:
- use the new unpack_hex_byte()
- add __must_check compiler option (Andy Shevchenko's suggestion)
- change function API to return error checking result
  (based on Tetsuo Handa's initial patch)

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 include/linux/kernel.h |    2 +-
 lib/hexdump.c          |   15 +++++++++++----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/include/linux/kernel.h b/include/linux/kernel.h
index 46ac9a5..8eefcf7 100644
--- a/include/linux/kernel.h
+++ b/include/linux/kernel.h
@@ -382,7 +382,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 int __must_check 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..51d5ae2 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -38,14 +38,21 @@ EXPORT_SYMBOL(hex_to_bin);
  * @dst: binary result
  * @src: ascii hexadecimal string
  * @count: result length
+ *
+ * Return 0 on success, -1 in case of bad input.
  */
-void hex2bin(u8 *dst, const char *src, size_t count)
+int hex2bin(u8 *dst, const char *src, size_t count)
 {
 	while (count--) {
-		*dst = hex_to_bin(*src++) << 4;
-		*dst += hex_to_bin(*src++);
-		dst++;
+		int hi = hex_to_bin(*src++);
+		int lo = hex_to_bin(*src++);
+
+		if ((hi < 0) || (lo < 0))
+			return -1;
+
+		*dst++ = (hi << 4) | lo;
 	}
+	return 0;
 }
 EXPORT_SYMBOL(hex2bin);
 
-- 
1.7.3.4


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

* [PATCH v1 2/4] trusted-keys: check hex2bin result
  2011-09-20 19:52 [PATCH v1 0/4] lib: hex2bin error checking Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 1/4] lib: add error checking to hex2bin Mimi Zohar
@ 2011-09-20 19:52 ` Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 3/4] encrypted-keys: " Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 4/4] target: " Mimi Zohar
  3 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2011-09-20 19:52 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, linux-kernel, Andrew Morton, Andy Shevchenko,
	Tetsuo Handa, Arnaud Lacombe, James Morris, David Safford

For each hex2bin call in trusted keys, check that the ascii hex string is
valid.  On failure, return -EINVAL.

Changelog v1:
- hex2bin now returns an int

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/keys/trusted.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/security/keys/trusted.c b/security/keys/trusted.c
index 0c33e2e..0964fc2 100644
--- a/security/keys/trusted.c
+++ b/security/keys/trusted.c
@@ -779,7 +779,10 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 			opt->pcrinfo_len = strlen(args[0].from) / 2;
 			if (opt->pcrinfo_len > MAX_PCRINFO_SIZE)
 				return -EINVAL;
-			hex2bin(opt->pcrinfo, args[0].from, opt->pcrinfo_len);
+			res = hex2bin(opt->pcrinfo, args[0].from,
+				      opt->pcrinfo_len);
+			if (res < 0)
+				return -EINVAL;
 			break;
 		case Opt_keyhandle:
 			res = strict_strtoul(args[0].from, 16, &handle);
@@ -791,12 +794,18 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
 		case Opt_keyauth:
 			if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
 				return -EINVAL;
-			hex2bin(opt->keyauth, args[0].from, SHA1_DIGEST_SIZE);
+			res = hex2bin(opt->keyauth, args[0].from,
+				      SHA1_DIGEST_SIZE);
+			if (res < 0)
+				return -EINVAL;
 			break;
 		case Opt_blobauth:
 			if (strlen(args[0].from) != 2 * SHA1_DIGEST_SIZE)
 				return -EINVAL;
-			hex2bin(opt->blobauth, args[0].from, SHA1_DIGEST_SIZE);
+			res = hex2bin(opt->blobauth, args[0].from,
+				      SHA1_DIGEST_SIZE);
+			if (res < 0)
+				return -EINVAL;
 			break;
 		case Opt_migratable:
 			if (*args[0].from == '0')
@@ -860,7 +869,9 @@ static int datablob_parse(char *datablob, struct trusted_key_payload *p,
 		p->blob_len = strlen(c) / 2;
 		if (p->blob_len > MAX_BLOB_SIZE)
 			return -EINVAL;
-		hex2bin(p->blob, c, p->blob_len);
+		ret = hex2bin(p->blob, c, p->blob_len);
+		if (ret < 0)
+			return -EINVAL;
 		ret = getoptions(datablob, p, o);
 		if (ret < 0)
 			return ret;
-- 
1.7.3.4


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

* [PATCH v1 3/4] encrypted-keys: check hex2bin result
  2011-09-20 19:52 [PATCH v1 0/4] lib: hex2bin error checking Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 1/4] lib: add error checking to hex2bin Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 2/4] trusted-keys: check hex2bin result Mimi Zohar
@ 2011-09-20 19:52 ` Mimi Zohar
  2011-09-20 19:52 ` [PATCH v1 4/4] target: " Mimi Zohar
  3 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2011-09-20 19:52 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, linux-kernel, Andrew Morton, Andy Shevchenko,
	Tetsuo Handa, Arnaud Lacombe, James Morris, David Safford

For each hex2bin call in encrypted keys, check that the ascii hex string
is valid.  On failure, return -EINVAL.

Changelog v1:
- hex2bin now returns an int

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 security/keys/encrypted-keys/encrypted.c |   14 +++++++++++---
 1 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/security/keys/encrypted-keys/encrypted.c b/security/keys/encrypted-keys/encrypted.c
index 3f57795..f33804c 100644
--- a/security/keys/encrypted-keys/encrypted.c
+++ b/security/keys/encrypted-keys/encrypted.c
@@ -667,11 +667,19 @@ static int encrypted_key_decrypt(struct encrypted_key_payload *epayload,
 		return -EINVAL;
 
 	hex_encoded_data = hex_encoded_iv + (2 * ivsize) + 2;
-	hex2bin(epayload->iv, hex_encoded_iv, ivsize);
-	hex2bin(epayload->encrypted_data, hex_encoded_data, encrypted_datalen);
+	ret = hex2bin(epayload->iv, hex_encoded_iv, ivsize);
+	if (ret < 0)
+		return -EINVAL;
+	ret = hex2bin(epayload->encrypted_data, hex_encoded_data,
+		      encrypted_datalen);
+	if (ret < 0)
+		return -EINVAL;
 
 	hmac = epayload->format + epayload->datablob_len;
-	hex2bin(hmac, hex_encoded_data + (encrypted_datalen * 2), HASH_SIZE);
+	ret = hex2bin(hmac, hex_encoded_data + (encrypted_datalen * 2),
+		      HASH_SIZE);
+	if (ret < 0)
+		return -EINVAL;
 
 	mkey = request_master_key(epayload, &master_key, &master_keylen);
 	if (IS_ERR(mkey))
-- 
1.7.3.4


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

* [PATCH v1 4/4] target: check hex2bin result
  2011-09-20 19:52 [PATCH v1 0/4] lib: hex2bin error checking Mimi Zohar
                   ` (2 preceding siblings ...)
  2011-09-20 19:52 ` [PATCH v1 3/4] encrypted-keys: " Mimi Zohar
@ 2011-09-20 19:52 ` Mimi Zohar
  2011-09-20 23:55   ` Andy Shevchenko
  2011-09-21 12:35   ` [PATCH v1 4/4] target: check hex2bin result (updated) Mimi Zohar
  3 siblings, 2 replies; 9+ messages in thread
From: Mimi Zohar @ 2011-09-20 19:52 UTC (permalink / raw)
  To: linux-security-module
  Cc: Mimi Zohar, linux-kernel, Andrew Morton, Andy Shevchenko,
	Tetsuo Handa, Arnaud Lacombe, James Morris, David Safford

Now that hex2bin does error checking, on error add debugging error msg.

Changelog v1:
- hex2bin now returns an int

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 drivers/target/target_core_fabric_lib.c |   11 +++++++++--
 1 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/target/target_core_fabric_lib.c b/drivers/target/target_core_fabric_lib.c
index c4ea3a9..17658ca 100644
--- a/drivers/target/target_core_fabric_lib.c
+++ b/drivers/target/target_core_fabric_lib.c
@@ -63,6 +63,7 @@ u32 sas_get_pr_transport_id(
 	unsigned char *buf)
 {
 	unsigned char *ptr;
+	bool ret;
 
 	/*
 	 * Set PROTOCOL IDENTIFIER to 6h for SAS
@@ -74,7 +75,9 @@ u32 sas_get_pr_transport_id(
 	 */
 	ptr = &se_nacl->initiatorname[4]; /* Skip over 'naa. prefix */
 
-	hex2bin(&buf[4], ptr, 8);
+	ret = hex2bin(&buf[4], ptr, 8);
+	if (ret < 0)
+		pr_debug("sas transport_id: invalid hex string\n");
 
 	/*
 	 * The SAS Transport ID is a hardcoded 24-byte length
@@ -158,6 +161,8 @@ u32 fc_get_pr_transport_id(
 	unsigned char *ptr;
 	int i;
 	u32 off = 8;
+	bool ret;
+
 	/*
 	 * PROTOCOL IDENTIFIER is 0h for FCP-2
 	 *
@@ -174,7 +179,9 @@ u32 fc_get_pr_transport_id(
 			i++;
 			continue;
 		}
-		hex2bin(&buf[off++], &ptr[i], 1);
+		ret = hex2bin(&buf[off++], &ptr[i], 1);
+		if (ret < 0)
+			pr_debug("fc transport_id: invalid hex string\n");
 		i += 2;
 	}
 	/*
-- 
1.7.3.4


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

* Re: [PATCH v1 4/4] target: check hex2bin result
  2011-09-20 19:52 ` [PATCH v1 4/4] target: " Mimi Zohar
@ 2011-09-20 23:55   ` Andy Shevchenko
  2011-09-21  3:41     ` Mimi Zohar
  2011-09-21 12:35   ` [PATCH v1 4/4] target: check hex2bin result (updated) Mimi Zohar
  1 sibling, 1 reply; 9+ messages in thread
From: Andy Shevchenko @ 2011-09-20 23:55 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-security-module, linux-kernel, Andrew Morton, Tetsuo Handa,
	Arnaud Lacombe, James Morris, David Safford

On Tue, Sep 20, 2011 at 10:52 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Now that hex2bin does error checking, on error add debugging error msg.
>
> Changelog v1:
> - hex2bin now returns an int
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> ---
>  drivers/target/target_core_fabric_lib.c |   11 +++++++++--
>  1 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/target/target_core_fabric_lib.c b/drivers/target/target_core_fabric_lib.c
> index c4ea3a9..17658ca 100644
> --- a/drivers/target/target_core_fabric_lib.c
> +++ b/drivers/target/target_core_fabric_lib.c
> @@ -63,6 +63,7 @@ u32 sas_get_pr_transport_id(
>        unsigned char *buf)
>  {
>        unsigned char *ptr;
> +       bool ret;
I'm sorry for this, but I guess you meant int?
>
>        /*
>         * Set PROTOCOL IDENTIFIER to 6h for SAS

> @@ -158,6 +161,8 @@ u32 fc_get_pr_transport_id(
>        unsigned char *ptr;
>        int i;
>        u32 off = 8;
> +       bool ret;
Ditto.
> +
>        /*
>         * PROTOCOL IDENTIFIER is 0h for FCP-2
>         *
Otherwise have my Acked-by. I think Andrew also will be happy.


-- 
With Best Regards,
Andy Shevchenko

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

* Re: [PATCH v1 4/4] target: check hex2bin result
  2011-09-20 23:55   ` Andy Shevchenko
@ 2011-09-21  3:41     ` Mimi Zohar
  2011-09-21 15:04       ` Mimi Zohar
  0 siblings, 1 reply; 9+ messages in thread
From: Mimi Zohar @ 2011-09-21  3:41 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-security-module, linux-kernel, Andrew Morton, Tetsuo Handa,
	Arnaud Lacombe, James Morris, David Safford

On Wed, 2011-09-21 at 02:55 +0300, Andy Shevchenko wrote:
> On Tue, Sep 20, 2011 at 10:52 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Now that hex2bin does error checking, on error add debugging error msg.
> >
> > Changelog v1:
> > - hex2bin now returns an int
> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > ---
> >  drivers/target/target_core_fabric_lib.c |   11 +++++++++--
> >  1 files changed, 9 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/target/target_core_fabric_lib.c b/drivers/target/target_core_fabric_lib.c
> > index c4ea3a9..17658ca 100644
> > --- a/drivers/target/target_core_fabric_lib.c
> > +++ b/drivers/target/target_core_fabric_lib.c
> > @@ -63,6 +63,7 @@ u32 sas_get_pr_transport_id(
> >        unsigned char *buf)
> >  {
> >        unsigned char *ptr;
> > +       bool ret;
> I'm sorry for this, but I guess you meant int?

Sigh, yes of course.  As an after thought, I decided to include the
'target' patch.  :( 
> >
> >        /*
> >         * Set PROTOCOL IDENTIFIER to 6h for SAS
> 
> > @@ -158,6 +161,8 @@ u32 fc_get_pr_transport_id(
> >        unsigned char *ptr;
> >        int i;
> >        u32 off = 8;
> > +       bool ret;
> Ditto.
> > +
> >        /*
> >         * PROTOCOL IDENTIFIER is 0h for FCP-2
> >         *
> Otherwise have my Acked-by. I think Andrew also will be happy.

Thanks for all the reviews/Acks!

Mimi


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

* Re: [PATCH v1 4/4] target: check hex2bin result (updated)
  2011-09-20 19:52 ` [PATCH v1 4/4] target: " Mimi Zohar
  2011-09-20 23:55   ` Andy Shevchenko
@ 2011-09-21 12:35   ` Mimi Zohar
  1 sibling, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2011-09-21 12:35 UTC (permalink / raw)
  To: linux-security-module
  Cc: linux-kernel, Andrew Morton, Andy Shevchenko, Tetsuo Handa,
	Arnaud Lacombe, James.Mor

Now that hex2bin does error checking, on error add debugging error msg.

Changelog v1 (update):
- fixed definition of 'ret'
- hex2bin now returns an int

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Acked-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/target/target_core_fabric_lib.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/target/target_core_fabric_lib.c b/drivers/target/target_core_fabric_lib.c
index c4ea3a9..39f021b 100644
--- a/drivers/target/target_core_fabric_lib.c
+++ b/drivers/target/target_core_fabric_lib.c
@@ -63,6 +63,7 @@ u32 sas_get_pr_transport_id(
 	unsigned char *buf)
 {
 	unsigned char *ptr;
+	int ret;
 
 	/*
 	 * Set PROTOCOL IDENTIFIER to 6h for SAS
@@ -74,7 +75,9 @@ u32 sas_get_pr_transport_id(
 	 */
 	ptr = &se_nacl->initiatorname[4]; /* Skip over 'naa. prefix */
 
-	hex2bin(&buf[4], ptr, 8);
+	ret = hex2bin(&buf[4], ptr, 8);
+	if (ret < 0)
+		pr_debug("sas transport_id: invalid hex string\n");
 
 	/*
 	 * The SAS Transport ID is a hardcoded 24-byte length
@@ -156,8 +159,9 @@ u32 fc_get_pr_transport_id(
 	unsigned char *buf)
 {
 	unsigned char *ptr;
-	int i;
+	int i, ret;
 	u32 off = 8;
+
 	/*
 	 * PROTOCOL IDENTIFIER is 0h for FCP-2
 	 *
@@ -174,7 +178,9 @@ u32 fc_get_pr_transport_id(
 			i++;
 			continue;
 		}
-		hex2bin(&buf[off++], &ptr[i], 1);
+		ret = hex2bin(&buf[off++], &ptr[i], 1);
+		if (ret < 0)
+			pr_debug("fc transport_id: invalid hex string\n");
 		i += 2;
 	}
 	/*
-- 
1.7.3.4



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

* Re: [PATCH v1 4/4] target: check hex2bin result
  2011-09-21  3:41     ` Mimi Zohar
@ 2011-09-21 15:04       ` Mimi Zohar
  0 siblings, 0 replies; 9+ messages in thread
From: Mimi Zohar @ 2011-09-21 15:04 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-security-module, linux-kernel, Andrew Morton, Tetsuo Handa,
	Arnaud Lacombe, James Morris, David Safford

On Tue, 2011-09-20 at 23:41 -0400, Mimi Zohar wrote:
> On Wed, 2011-09-21 at 02:55 +0300, Andy Shevchenko wrote:

> > Otherwise have my Acked-by. I think Andrew also will be happy.
> 
> Thanks for all the reviews/Acks!
> 
> Mimi

An updated version of "target: check hex2bin result" was posted here -
https://lkml.org/lkml/2011/9/21/195. The patch set is available from
git://github.com/mzohar/linux-evm.git #next-hex2bin.

thanks,

Mimi


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

end of thread, other threads:[~2011-09-21 15:11 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-09-20 19:52 [PATCH v1 0/4] lib: hex2bin error checking Mimi Zohar
2011-09-20 19:52 ` [PATCH v1 1/4] lib: add error checking to hex2bin Mimi Zohar
2011-09-20 19:52 ` [PATCH v1 2/4] trusted-keys: check hex2bin result Mimi Zohar
2011-09-20 19:52 ` [PATCH v1 3/4] encrypted-keys: " Mimi Zohar
2011-09-20 19:52 ` [PATCH v1 4/4] target: " Mimi Zohar
2011-09-20 23:55   ` Andy Shevchenko
2011-09-21  3:41     ` Mimi Zohar
2011-09-21 15:04       ` Mimi Zohar
2011-09-21 12:35   ` [PATCH v1 4/4] target: check hex2bin result (updated) Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).