Author: stefanct Date: Sun Jul 13 14:52:15 2014 New Revision: 1824 URL: http://flashrom.org/trac/flashrom/changeset/1824
Log: Fix garbage handling in DMI strings. Previously we tried to replace garbage characters with <space> directly in the read-only memory-mapped SMBIOS area(!). This could never have worked for any DMI strings with garbage and results in a segfault on machines with such strings. Thanks to Brian Rak (Supermicro X10SLE-F) and John Pohlman (HP XW9400) for reporting this issue. With this patch the strings are duplicated within dmi_string() already, just before we sanitize them. Also, the limit variable used everywhere points to the first invalid byte address. Refine respective checks accordingly. Signed-off-by: Stefan Tauner <[email protected]> Acked-by: Carl-Daniel Hailfinger <[email protected]> Modified: trunk/dmi.c Modified: trunk/dmi.c ============================================================================== --- trunk/dmi.c Sun Jul 13 02:05:07 2014 (r1823) +++ trunk/dmi.c Sun Jul 13 14:52:15 2014 (r1824) @@ -23,6 +23,7 @@ #include <strings.h> #include <string.h> +#include <ctype.h> #include <stdio.h> #include <stdlib.h> @@ -105,7 +106,16 @@ return (sum == 0); } -static char *dmi_string(uint8_t *buf, uint8_t string_id, uint8_t *limit) +/** Retrieve a DMI string. + * + * See SMBIOS spec. section 6.1.3 "Text strings". + * The table will be unmapped ASAP, hence return a duplicated & sanitized string that needs to be freed later. + * + * \param buf the buffer to search through (usually appended directly to a DMI structure) + * \param string_id index of the string to look for + * \param limit pointer to the first byte beyond \em buf + */ +static char *dmi_string(const char *buf, uint8_t string_id, const char *limit) { size_t i, len; @@ -113,26 +123,33 @@ return "Not Specified"; while (string_id > 1 && string_id--) { - if (buf > limit) { + if (buf >= limit) { msg_perr("DMI table is broken (string portion out of bounds)!\n"); return "<OUT OF BOUNDS>"; } - buf += strnlen((char *)buf, limit - buf) + 1; + buf += strnlen(buf, limit - buf) + 1; } if (!*buf) /* as long as the current byte we're on isn't null */ return "<BAD INDEX>"; - len = strnlen((char *)buf, limit - buf); - if (len > DMI_MAX_ANSWER_LEN) - len = DMI_MAX_ANSWER_LEN; + len = strnlen(buf, limit - buf); + char *newbuf = malloc(len + 1); + if (newbuf == NULL) { + msg_perr("Out of memory!\n"); + return NULL; + } /* fix junk bytes in the string */ - for (i = 0; i < len; i++) - if (buf[i] < 32 || buf[i] >= 127) - buf[i] = ' '; + for (i = 0; i < len && buf[i] != '\0'; i++) { + if (isprint(buf[i])) + newbuf[i] = buf[i]; + else + newbuf[i] = ' '; + } + newbuf[i] = '\0'; - return (char *)buf; + return newbuf; } static void dmi_chassis_type(uint8_t code) @@ -167,7 +184,7 @@ * - uint8_t length; // data section w/ header w/o strings * - uint16_t handle; */ - while (i < num && data + 4 <= limit) { + while (i < num && data + 4 < limit) { /* - If a short entry is found (less than 4 bytes), not only it * is invalid, but we cannot reliably locate the next entry. * - If the length value indicates that this structure spreads @@ -175,15 +192,16 @@ * Better stop at this point, and let the user know his/her * table is broken. */ - if (data[1] < 4 || data + data[1] > limit) { + if (data[1] < 4 || data + data[1] >= limit) { msg_perr("DMI table is broken (bogus header)!\n"); break; } if(data[0] == 3) { - if (data + 5 <= limit) + if (data + 5 < limit) dmi_chassis_type(data[5]); - /* else the table is broken, but laptop detection is optional, hence continue. */ + else /* the table is broken, but laptop detection is optional, hence continue. */ + msg_pwarn("DMI table is broken (chassis_type out of bounds)!\n"); } else for (j = 0; j < ARRAY_SIZE(dmi_strings); j++) { uint8_t offset = dmi_strings[j].offset; @@ -192,13 +210,13 @@ if (data[0] != type) continue; - if (data[1] <= offset || data+offset > limit) { + if (data[1] <= offset || data + offset >= limit) { msg_perr("DMI table is broken (offset out of bounds)!\n"); goto out; } - /* Table will be unmapped, hence fill the struct with duplicated strings. */ - dmi_strings[j].value = strdup(dmi_string(data + data[1], data[offset], limit)); + dmi_strings[j].value = dmi_string((const char *)(data + data[1]), data[offset], + (const char *)limit); } /* Find next structure by skipping data and string sections */ data += data[1]; _______________________________________________ flashrom mailing list [email protected] http://www.flashrom.org/mailman/listinfo/flashrom
