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 <[email protected]> --- 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
