[PATCH] firmware: google: update vpd_decode from upstream

From: Hung-Te Lin
Date: Fri Aug 02 2019 - 04:20:58 EST

The VPD implementation from Chromium Vital Product Data project has been
updated so vpd_decode be easily shared by kernel, firmware and the user
space utility programs. Also improved value range checks to prevent
kernel crash due to bad VPD data.

Signed-off-by: Hung-Te Lin <hungte@xxxxxxxxxxxx>
---
drivers/firmware/google/vpd.c | 38 +++++++++------
drivers/firmware/google/vpd_decode.c | 69 +++++++++++++++-------------
drivers/firmware/google/vpd_decode.h | 17 ++++---
3 files changed, 71 insertions(+), 53 deletions(-)

diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c
index 0739f3b70347..ecf217a7db39 100644
--- a/drivers/firmware/google/vpd.c
+++ b/drivers/firmware/google/vpd.c
@@ -73,7 +73,7 @@ static ssize_t vpd_attrib_read(struct file *filp, struct kobject *kobp,
* exporting them as sysfs attributes. These keys present in old firmwares are
* ignored.
*
- * Returns VPD_OK for a valid key name, VPD_FAIL otherwise.
+ * Returns VPD_DECODE_OK for a valid key name, VPD_DECODE_FAIL otherwise.
*
* @key: The key name to check
* @key_len: key name length
@@ -86,14 +86,14 @@ static int vpd_section_check_key_name(const u8 *key, s32 key_len)
c = *key++;

if (!isalnum(c) && c != '_')
- return VPD_FAIL;
+ return VPD_DECODE_FAIL;
}

- return VPD_OK;
+ return VPD_DECODE_OK;
}

-static int vpd_section_attrib_add(const u8 *key, s32 key_len,
- const u8 *value, s32 value_len,
+static int vpd_section_attrib_add(const u8 *key, u32 key_len,
+ const u8 *value, u32 value_len,
void *arg)
{
int ret;
@@ -101,11 +101,11 @@ static int vpd_section_attrib_add(const u8 *key, s32 key_len,
struct vpd_attrib_info *info;

/*
- * Return VPD_OK immediately to decode next entry if the current key
- * name contains invalid characters.
+ * Return VPD_DECODE_OK immediately to decode next entry if the current
+ * key name contains invalid characters.
*/
- if (vpd_section_check_key_name(key, key_len) != VPD_OK)
- return VPD_OK;
+ if (vpd_section_check_key_name(key, key_len) != VPD_DECODE_OK)
+ return VPD_DECODE_OK;

info = kzalloc(sizeof(*info), GFP_KERNEL);
if (!info)
@@ -174,7 +174,7 @@ static int vpd_section_create_attribs(struct vpd_section *sec)
do {
ret = vpd_decode_string(sec->bin_attr.size, sec->baseaddr,
&consumed, vpd_section_attrib_add, sec);
- } while (ret == VPD_OK);
+ } while (ret == VPD_DECODE_OK);

return 0;
}
@@ -246,7 +246,7 @@ static int vpd_section_destroy(struct vpd_section *sec)

static int vpd_sections_init(phys_addr_t physaddr)
{
- struct vpd_cbmem *temp;
+ struct vpd_cbmem __iomem *temp;
struct vpd_cbmem header;
int ret = 0;

@@ -254,7 +254,7 @@ static int vpd_sections_init(phys_addr_t physaddr)
if (!temp)
return -ENOMEM;

- memcpy(&header, temp, sizeof(struct vpd_cbmem));
+ memcpy_fromio(&header, temp, sizeof(struct vpd_cbmem));
memunmap(temp);

if (header.magic != VPD_CBMEM_MAGIC)
@@ -316,7 +316,19 @@ static struct coreboot_driver vpd_driver = {
},
.tag = CB_TAG_VPD,
};
-module_coreboot_driver(vpd_driver);
+
+static int __init coreboot_vpd_init(void)
+{
+ return coreboot_driver_register(&vpd_driver);
+}
+
+static void __exit coreboot_vpd_exit(void)
+{
+ coreboot_driver_unregister(&vpd_driver);
+}
+
+module_init(coreboot_vpd_init);
+module_exit(coreboot_vpd_exit);

MODULE_AUTHOR("Google, Inc.");
MODULE_LICENSE("GPL");
diff --git a/drivers/firmware/google/vpd_decode.c b/drivers/firmware/google/vpd_decode.c
index 92e3258552fc..5531770e3d58 100644
--- a/drivers/firmware/google/vpd_decode.c
+++ b/drivers/firmware/google/vpd_decode.c
@@ -9,19 +9,19 @@

#include "vpd_decode.h"

-static int vpd_decode_len(const s32 max_len, const u8 *in,
- s32 *length, s32 *decoded_len)
+static int vpd_decode_len(const u32 max_len, const u8 *in, u32 *length,
+ u32 *decoded_len)
{
u8 more;
int i = 0;

if (!length || !decoded_len)
- return VPD_FAIL;
+ return VPD_DECODE_FAIL;

*length = 0;
do {
if (i >= max_len)
- return VPD_FAIL;
+ return VPD_DECODE_FAIL;

more = in[i] & 0x80;
*length <<= 7;
@@ -30,24 +30,43 @@ static int vpd_decode_len(const s32 max_len, const u8 *in,
} while (more);

*decoded_len = i;
+ return VPD_DECODE_OK;
+}
+
+static int vpd_decode_entry(const u32 max_len, const u8 *input_buf,
+ u32 *consumed, const u8 **entry, u32 *entry_len)
+{
+ u32 decoded_len;
+
+ if (vpd_decode_len(max_len - *consumed, &input_buf[*consumed],
+ entry_len, &decoded_len) != VPD_DECODE_OK)
+ return VPD_DECODE_FAIL;
+ if (max_len - *consumed < decoded_len)
+ return VPD_DECODE_FAIL;

- return VPD_OK;
+ *consumed += decoded_len;
+ *entry = input_buf + *consumed;
+
+ /* entry_len is untrusted data and must be checked again. */
+ if (max_len - *consumed < *entry_len)
+ return VPD_DECODE_FAIL;
+
+ *consumed += *entry_len;
+ return VPD_DECODE_OK;
}

-int vpd_decode_string(const s32 max_len, const u8 *input_buf, s32 *consumed,
+int vpd_decode_string(const u32 max_len, const u8 *input_buf, u32 *consumed,
vpd_decode_callback callback, void *callback_arg)
{
int type;
- int res;
- s32 key_len;
- s32 value_len;
- s32 decoded_len;
+ u32 key_len;
+ u32 value_len;
const u8 *key;
const u8 *value;

/* type */
if (*consumed >= max_len)
- return VPD_FAIL;
+ return VPD_DECODE_FAIL;

type = input_buf[*consumed];

@@ -56,25 +75,13 @@ int vpd_decode_string(const s32 max_len, const u8 *input_buf, s32 *consumed,
case VPD_TYPE_STRING:
(*consumed)++;

- /* key */
- res = vpd_decode_len(max_len - *consumed, &input_buf[*consumed],
- &key_len, &decoded_len);
- if (res != VPD_OK || *consumed + decoded_len >= max_len)
- return VPD_FAIL;
-
- *consumed += decoded_len;
- key = &input_buf[*consumed];
- *consumed += key_len;
-
- /* value */
- res = vpd_decode_len(max_len - *consumed, &input_buf[*consumed],
- &value_len, &decoded_len);
- if (res != VPD_OK || *consumed + decoded_len > max_len)
- return VPD_FAIL;
+ if (vpd_decode_entry(max_len, input_buf, consumed, &key,
+ &key_len) != VPD_DECODE_OK)
+ return VPD_DECODE_FAIL;

- *consumed += decoded_len;
- value = &input_buf[*consumed];
- *consumed += value_len;
+ if (vpd_decode_entry(max_len, input_buf, consumed, &value,
+ &value_len) != VPD_DECODE_OK)
+ return VPD_DECODE_FAIL;

if (type == VPD_TYPE_STRING)
return callback(key, key_len, value, value_len,
@@ -82,8 +89,8 @@ int vpd_decode_string(const s32 max_len, const u8 *input_buf, s32 *consumed,
break;

default:
- return VPD_FAIL;
+ return VPD_DECODE_FAIL;
}

- return VPD_OK;
+ return VPD_DECODE_OK;
}
diff --git a/drivers/firmware/google/vpd_decode.h b/drivers/firmware/google/vpd_decode.h
index cf8c2ace155a..4113ac2f4a70 100644
--- a/drivers/firmware/google/vpd_decode.h
+++ b/drivers/firmware/google/vpd_decode.h
@@ -13,28 +13,27 @@
#include <linux/types.h>

enum {
- VPD_OK = 0,
- VPD_FAIL,
+ VPD_DECODE_OK = 0,
+ VPD_DECODE_FAIL = 1,
};

enum {
VPD_TYPE_TERMINATOR = 0,
VPD_TYPE_STRING,
- VPD_TYPE_INFO = 0xfe,
+ VPD_TYPE_INFO = 0xfe,
VPD_TYPE_IMPLICIT_TERMINATOR = 0xff,
};

/* Callback for vpd_decode_string to invoke. */
-typedef int vpd_decode_callback(const u8 *key, s32 key_len,
- const u8 *value, s32 value_len,
- void *arg);
+typedef int vpd_decode_callback(const u8 *key, u32 key_len, const u8 *value,
+ u32 value_len, void *arg);

/*
* vpd_decode_string
*
* Given the encoded string, this function invokes callback with extracted
- * (key, value). The *consumed will be plused the number of bytes consumed in
- * this function.
+ * (key, value). The *consumed will be incremented by the number of bytes
+ * consumed in this function.
*
* The input_buf points to the first byte of the input buffer.
*
@@ -44,7 +43,7 @@ typedef int vpd_decode_callback(const u8 *key, s32 key_len,
* If one entry is successfully decoded, sends it to callback and returns the
* result.
*/
-int vpd_decode_string(const s32 max_len, const u8 *input_buf, s32 *consumed,
+int vpd_decode_string(const u32 max_len, const u8 *input_buf, u32 *consumed,
vpd_decode_callback callback, void *callback_arg);

#endif /* __VPD_DECODE_H */
--
2.22.0.770.g0f2c4a37fd-goog