* [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
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
* 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