<p>Patrick Rudolph <strong>merged</strong> this change.</p><p><a href="https://review.coreboot.org/29099">View Change</a></p><div style="white-space:pre-wrap">Approvals:
build bot (Jenkins): Verified
Patrick Georgi: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">util/intelvbttool: Cleanup and fixes<br><br>* Clear remalloced memory<br>* Fix check for invalid VBT offset in header<br>* Fix VBIOS checksum generation<br>* Fix VBIOS size field<br>* Align VBIOS size to multiple of 512<br>* Reassign pointers after use of remalloc<br>* Don't leak on error path<br><br>Current version is enough to allow the proprietary Windows Intel GMA<br>driver to find the VBT in the legacy VBIOS area and it doesn't BSOD<br>any more.<br><br>The LVDS screen remains black, due to an unknown issue with the<br>proprietary driver, while the VGA works.<br><br>Tested with libgfxinit and native graphics init.<br><br>Change-Id: If07b1bb51d8fb3499d13102f70fedb36c020fb72<br>Signed-off-by: Patrick Rudolph <siro@das-labor.org><br>Reviewed-on: https://review.coreboot.org/29099<br>Tested-by: build bot (Jenkins) <no-reply@coreboot.org><br>Reviewed-by: Patrick Georgi <pgeorgi@google.com><br>---<br>M util/intelvbttool/intelvbttool.c<br>1 file changed, 51 insertions(+), 52 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/util/intelvbttool/intelvbttool.c b/util/intelvbttool/intelvbttool.c</span><br><span>index c8b973e..1444129 100644</span><br><span>--- a/util/intelvbttool/intelvbttool.c</span><br><span>+++ b/util/intelvbttool/intelvbttool.c</span><br><span>@@ -341,10 +341,12 @@</span><br><span> return NULL;</span><br><span> </span><br><span> fo->data = realloc(fo->data, size);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> if (!fo->data)</span><br><span> return NULL;</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ if (fo->size < size)</span><br><span style="color: hsl(120, 100%, 40%);">+ memset(&fo->data[fo->size], 0, size - fo->size);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> fo->size = size;</span><br><span> </span><br><span> return fo;</span><br><span>@@ -882,7 +884,6 @@</span><br><span> struct fileobject **vbt)</span><br><span> {</span><br><span> const optionrom_header_t *oh = (const optionrom_header_t *)fo->data;</span><br><span style="color: hsl(0, 100%, 40%);">- const struct vbt_header *head;</span><br><span> size_t i;</span><br><span> </span><br><span> *vbt = NULL;</span><br><span>@@ -897,24 +898,23 @@</span><br><span> return;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- head = (const struct vbt_header *)fo->data;</span><br><span style="color: hsl(0, 100%, 40%);">- if (memcmp(head->signature, "$VBT", 4) == 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- parse_vbt(fo, vbt);</span><br><span style="color: hsl(120, 100%, 40%);">+ struct fileobject *fo_vbt = malloc_fo_sub(fo, oh->vbt_offset);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fo_vbt) {</span><br><span style="color: hsl(120, 100%, 40%);">+ parse_vbt(fo_vbt, vbt);</span><br><span style="color: hsl(120, 100%, 40%);">+ free_fo(fo_vbt);</span><br><span> if (*vbt)</span><br><span> return;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- printwarn("VBT wasn't found at specified offset\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ printwarn("VBT wasn't found at specified offset of %04x\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ oh->vbt_offset);</span><br><span> </span><br><span> for (i = sizeof(optionrom_header_t);</span><br><span> i <= fo->size - sizeof(struct vbt_header); i++) {</span><br><span> struct fileobject *fo_vbt = malloc_fo_sub(fo, i);</span><br><span> if (!fo_vbt)</span><br><span style="color: hsl(0, 100%, 40%);">- continue;</span><br><span style="color: hsl(120, 100%, 40%);">+ break;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- head = (const struct vbt_header *)fo_vbt->data;</span><br><span style="color: hsl(0, 100%, 40%);">- if (memcmp(head->signature, "$VBT", 4) == 0)</span><br><span style="color: hsl(0, 100%, 40%);">- parse_vbt(fo_vbt, vbt);</span><br><span style="color: hsl(120, 100%, 40%);">+ parse_vbt(fo_vbt, vbt);</span><br><span> </span><br><span> free_fo(fo_vbt);</span><br><span> </span><br><span>@@ -980,32 +980,28 @@</span><br><span> };</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-/* Open a VBIOS file, calculate the checksum and fix it */</span><br><span style="color: hsl(0, 100%, 40%);">-static int fix_vbios_checksum(const char *filename)</span><br><span style="color: hsl(120, 100%, 40%);">+/* Fix VBIOS header and PCIR */</span><br><span style="color: hsl(120, 100%, 40%);">+static int fix_vbios_header(struct fileobject *fo)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- struct fileobject *fo = read_file(filename);</span><br><span style="color: hsl(0, 100%, 40%);">- if (!fo) {</span><br><span style="color: hsl(0, 100%, 40%);">- printerr("%s open failed\n", filename);</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (fo->size < sizeof(optionrom_header_t))</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!fo || fo->size < sizeof(optionrom_header_t))</span><br><span> return 1;</span><br><span> </span><br><span> optionrom_header_t *oh = (optionrom_header_t *)fo->data;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- if (oh->size * 512 > fo->size)</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- /* fix checksum */</span><br><span style="color: hsl(0, 100%, 40%);">- oh->checksum = -(checksum_vbios(oh) - oh->checksum);</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span style="color: hsl(0, 100%, 40%);">- if (write_file(filename, fo)) {</span><br><span style="color: hsl(0, 100%, 40%);">- printerr("%s write failed\n", filename);</span><br><span style="color: hsl(0, 100%, 40%);">- free_fo(fo);</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Fix size alignment */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fo->size % 512) {</span><br><span style="color: hsl(120, 100%, 40%);">+ print("Aligning size to 512\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ fo = remalloc_fo(fo, (fo->size + 511) / 512 * 512);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!fo)</span><br><span style="color: hsl(120, 100%, 40%);">+ return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ oh = (optionrom_header_t *)fo->data;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- free_fo(fo);</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Fix size field */</span><br><span style="color: hsl(120, 100%, 40%);">+ oh->size = fo->size / 512;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span style="color: hsl(120, 100%, 40%);">+ /* Fix checksum field */</span><br><span style="color: hsl(120, 100%, 40%);">+ oh->checksum = -(checksum_vbios(oh) - oh->checksum);</span><br><span> </span><br><span> return 0;</span><br><span> }</span><br><span>@@ -1042,8 +1038,10 @@</span><br><span> fo = remalloc_fo(fo, fo->size - vbt_size(old_vbt));</span><br><span> if (!fo) {</span><br><span> printerr("Failed to allocate memory\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ free_fo(old_vbt);</span><br><span> return 1;</span><br><span> }</span><br><span style="color: hsl(120, 100%, 40%);">+ oh = (optionrom_header_t *)fo->data;</span><br><span> oh->vbt_offset = 0;</span><br><span> } else if (vbt_size(old_vbt) < vbt_size(fo_vbt)) {</span><br><span> /* In the middle of the file - Remove old VBT */</span><br><span>@@ -1061,7 +1059,7 @@</span><br><span> </span><br><span> if (!oh->vbt_offset) {</span><br><span> print("increasing VBIOS to append VBT\n");</span><br><span style="color: hsl(0, 100%, 40%);">- if ((fo->size + vbt_size(fo_vbt)) >= 64 * KiB) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((fo->size + vbt_size(fo_vbt)) >= 2 * 64 * KiB) {</span><br><span> printerr("VBT won't fit\n");</span><br><span> return 1;</span><br><span> }</span><br><span>@@ -1072,7 +1070,7 @@</span><br><span> printerr("Failed to allocate memory\n");</span><br><span> return 1;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- oh->size = (fo->size + 512 - 1) / 512;</span><br><span style="color: hsl(120, 100%, 40%);">+ oh = (optionrom_header_t *)fo->data;</span><br><span> }</span><br><span> </span><br><span> head = (struct vbt_header *)((u8 *)oh + oh->vbt_offset);</span><br><span>@@ -1083,7 +1081,7 @@</span><br><span> </span><br><span> int main(int argc, char **argv)</span><br><span> {</span><br><span style="color: hsl(0, 100%, 40%);">- int opt, option_index = 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ int opt, ret, option_index = 0;</span><br><span> </span><br><span> size_t has_input = 0, has_output = 0;</span><br><span> size_t dump = 0, in_legacy = 0;</span><br><span>@@ -1180,40 +1178,41 @@</span><br><span> if (!dump)</span><br><span> print_vbt(vbt);</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 0;</span><br><span> if (dump) {</span><br><span> dump_vbt(vbt);</span><br><span> } else if (out_vbt) {</span><br><span style="color: hsl(0, 100%, 40%);">- if (write_file(out_vbt, vbt))</span><br><span style="color: hsl(120, 100%, 40%);">+ if (write_file(out_vbt, vbt)) {</span><br><span> printerr("Failed to write VBT\n");</span><br><span style="color: hsl(0, 100%, 40%);">- else</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ } else {</span><br><span> print("VBT written to %s\n", out_vbt);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> } else if (patch_oprom) {</span><br><span> fo = read_file(patch_oprom);</span><br><span> if (!fo) {</span><br><span> printerr("Failed to read input file\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 1;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (!is_valid_vbios(fo)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret != 1 && !is_valid_vbios(fo)) {</span><br><span> printerr("Invalid input file\n");</span><br><span style="color: hsl(0, 100%, 40%);">- free_fo(fo);</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 1;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (patch_vbios(fo, vbt)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret != 1 && patch_vbios(fo, vbt)) {</span><br><span> printerr("Failed to patch VBIOS\n");</span><br><span style="color: hsl(0, 100%, 40%);">- free_fo(fo);</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 1;</span><br><span> }</span><br><span style="color: hsl(0, 100%, 40%);">- if (write_file(patch_oprom, fo)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret != 1 && fix_vbios_header(fo)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ printerr("Failed to fix VBIOS header\n");</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret != 1 && write_file(patch_oprom, fo)) {</span><br><span> printerr("Failed to write VBIOS\n");</span><br><span style="color: hsl(0, 100%, 40%);">- free_fo(fo);</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(120, 100%, 40%);">+ ret = 1;</span><br><span> }</span><br><span> free_fo(fo);</span><br><span style="color: hsl(0, 100%, 40%);">- if (fix_vbios_checksum(patch_oprom)) {</span><br><span style="color: hsl(0, 100%, 40%);">- printerr("Failed to fix checksum\n");</span><br><span style="color: hsl(0, 100%, 40%);">- return 1;</span><br><span style="color: hsl(0, 100%, 40%);">- }</span><br><span style="color: hsl(0, 100%, 40%);">- print("VBIOS %s successfully patched\n", patch_oprom);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ret != 1)</span><br><span style="color: hsl(120, 100%, 40%);">+ print("VBIOS %s successfully patched\n", patch_oprom);</span><br><span> }</span><br><span> </span><br><span> /* cleanup */</span><br><span>@@ -1228,5 +1227,5 @@</span><br><span> </span><br><span> free_fo(vbt);</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- return 0;</span><br><span style="color: hsl(120, 100%, 40%);">+ return ret;</span><br><span> }</span><br><span></span><br></pre><p>To view, visit <a href="https://review.coreboot.org/29099">change 29099</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://review.coreboot.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://review.coreboot.org/29099"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: coreboot </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: merged </div>
<div style="display:none"> Gerrit-Change-Id: If07b1bb51d8fb3499d13102f70fedb36c020fb72 </div>
<div style="display:none"> Gerrit-Change-Number: 29099 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: Patrick Rudolph <siro@das-labor.org> </div>
<div style="display:none"> Gerrit-Reviewer: Patrick Georgi <pgeorgi@google.com> </div>
<div style="display:none"> Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org> </div>
<div style="display:none"> Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org> </div>