Changeset View
Standalone View
sys/x86/x86/ucode.c
| Show First 20 Lines • Show All 198 Lines • ▼ Show 20 Lines | |||||
| ucode_intel_match(const uint8_t *data, size_t *len) | ucode_intel_match(const uint8_t *data, size_t *len) | ||||
| { | { | ||||
| const struct ucode_intel_header *hdr; | const struct ucode_intel_header *hdr; | ||||
| const struct ucode_intel_extsig_table *table; | const struct ucode_intel_extsig_table *table; | ||||
| const struct ucode_intel_extsig *entry; | const struct ucode_intel_extsig *entry; | ||||
| uint64_t platformid; | uint64_t platformid; | ||||
| size_t resid; | size_t resid; | ||||
| uint32_t data_size, flags, regs[4], sig, total_size; | uint32_t data_size, flags, regs[4], sig, total_size; | ||||
| int i; | |||||
| do_cpuid(1, regs); | do_cpuid(1, regs); | ||||
| sig = regs[0]; | sig = regs[0]; | ||||
| platformid = rdmsr(MSR_IA32_PLATFORM_ID); | platformid = rdmsr(MSR_IA32_PLATFORM_ID); | ||||
| flags = 1 << ((platformid >> 50) & 0x7); | flags = 1 << ((platformid >> 50) & 0x7); | ||||
| for (resid = *len; resid > 0; data += total_size, resid -= total_size) { | for (resid = *len; resid > 0; data += total_size, resid -= total_size) { | ||||
| hdr = (const struct ucode_intel_header *)data; | hdr = (const struct ucode_intel_header *)data; | ||||
| if (ucode_intel_verify(hdr, resid) != 0) { | if (ucode_intel_verify(hdr, resid) != 0) { | ||||
| ucode_error = VERIFICATION_FAILED; | ucode_error = VERIFICATION_FAILED; | ||||
| break; | break; | ||||
| } | } | ||||
| data_size = hdr->data_size; | data_size = hdr->data_size; | ||||
| total_size = hdr->total_size; | total_size = hdr->total_size; | ||||
| if (data_size == 0) | if (data_size == 0) | ||||
| data_size = UCODE_INTEL_DEFAULT_DATA_SIZE; | data_size = UCODE_INTEL_DEFAULT_DATA_SIZE; | ||||
| if (total_size == 0) | if (total_size == 0) | ||||
| total_size = UCODE_INTEL_DEFAULT_DATA_SIZE + | total_size = UCODE_INTEL_DEFAULT_DATA_SIZE + | ||||
| sizeof(struct ucode_intel_header); | sizeof(struct ucode_intel_header); | ||||
| if (data_size > total_size + sizeof(struct ucode_intel_header)) | |||||
| if (total_size > data_size + sizeof(struct ucode_intel_header)) | |||||
| table = (const struct ucode_intel_extsig_table *) | table = (const struct ucode_intel_extsig_table *) | ||||
| ((const uint8_t *)(hdr + 1) + data_size); | ((const uint8_t *)(hdr + 1) + data_size); | ||||
| else | else | ||||
| table = NULL; | table = NULL; | ||||
| if (hdr->processor_signature == sig) { | if (hdr->processor_signature == sig && | ||||
| if ((hdr->processor_flags & flags) != 0) { | (hdr->processor_flags & flags) != 0) { | ||||
| *len = data_size; | *len = data_size; | ||||
| return (hdr + 1); | return (hdr + 1); | ||||
| } | } | ||||
| } else if (table != NULL) { | if (table != NULL) { | ||||
| for (i = 0; i < table->signature_count; i++) { | size_t extsize; | ||||
| extsize = total_size - | |||||
| (data_size + sizeof(struct ucode_intel_header)); | |||||
| if (extsize < sizeof(struct ucode_intel_extsig_table)) { | |||||
| ucode_error = VERIFICATION_FAILED; | |||||
| break; | |||||
| } | |||||
| extsize -= sizeof(struct ucode_intel_extsig_table); | |||||
| for (uint32_t i = 0; i < table->signature_count; i++) { | |||||
| if (extsize < sizeof(struct ucode_intel_extsig)) { | |||||
| ucode_error = VERIFICATION_FAILED; | |||||
| goto out; | |||||
kib: Why this check is needed? Wouldn't the next check first addend give the same verification? | |||||
Done Inline ActionsI wanted to first check that it's "safe" to access table->signature_count. Overflow of the multiplication you mean? I believe it can't happen on 64-bit systems, but yes it could be written more carefully. In general I think we should at least make sure we're accessing fields within the bounds of the loaded microcode file. markj: I wanted to first check that it's "safe" to access table->signature_count.
Overflow of the… | |||||
Not Done Inline Actions
Please add one-line comment there.
I wonder if some kind of 'blob access API' which internally checks the bounds is due. It could be utilized in more places. But this is out of scope of this change, of course. kib: > I wanted to first check that it's "safe" to access table->signature_count.
Please add one… | |||||
Done Inline Actions
That would be useful. Something which lets you take a variety of "memory descriptors" (e.g., plain buffer, SG list of vaddrs or paddrs, mbufs, etc.) and maintains a current seek offset, then lets you copy in or copy out (more expensive, safer) or obtain a raw pointer to a sub-range of a specific size (cheaper, unsafe). The opencrypto code has something like this, see crypto_cursor_*, and I'm sure there are others. markj: > I wonder if some kind of 'blob access API' which internally checks the bounds is due. It… | |||||
| } | |||||
| extsize -= sizeof(struct ucode_intel_extsig); | |||||
| entry = &table->entries[i]; | entry = &table->entries[i]; | ||||
| if (entry->processor_signature == sig && | if (entry->processor_signature == sig && | ||||
| (entry->processor_flags & flags) != 0) { | (entry->processor_flags & flags) != 0) { | ||||
| *len = data_size; | *len = data_size; | ||||
| return (hdr + 1); | return (hdr + 1); | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| } | } | ||||
| out: | |||||
| return (NULL); | return (NULL); | ||||
| } | } | ||||
| int | int | ||||
| ucode_amd_load(const void *data, ucode_load_how how, uint64_t *nrevp, | ucode_amd_load(const void *data, ucode_load_how how, uint64_t *nrevp, | ||||
| uint64_t *orevp) | uint64_t *orevp) | ||||
| { | { | ||||
| uint64_t nrev, orev; | uint64_t nrev, orev; | ||||
| ▲ Show 20 Lines • Show All 227 Lines • Show Last 20 Lines | |||||
Why this check is needed? Wouldn't the next check first addend give the same verification?
BTW, how much do we want to check the blobs? eg are overflows believed to never occur?