Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37238 )
Change subject: cbtable.c: Factor out lb_header_validation logic ......................................................................
cbtable.c: Factor out lb_header_validation logic
Write a pure function for the header validation logic, it is easier to unit-test.
Change-Id: Ia288bcbc5c371329952a6efba30ccf0e18965a3d Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M cbtable.c 1 file changed, 20 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/37238/1
diff --git a/cbtable.c b/cbtable.c index bdf53ce..cd0cf15 100644 --- a/cbtable.c +++ b/cbtable.c @@ -131,6 +131,25 @@ return (~chksum.word) & 0xFFFF; }
+static int lb_header_valid(struct lb_header *head, unsigned long addr) +{ + if (memcmp(head->signature, "LBIO", 4) != 0) + return 0; + msg_pdbg("Found candidate at: %08lx-%08lx\n", + addr, addr + sizeof(*head) + head->table_bytes); + if (head->header_bytes != sizeof(*head)) { + msg_perr("Header bytes of %d are incorrect.\n", + head->header_bytes); + return 0; + } + if (compute_checksum((uint8_t *) head, sizeof(*head)) != 0) { + msg_perr("Bad header checksum.\n"); + return 0; + } + + return 1; +} + #define for_each_lbrec(head, rec) \ for(rec = (struct lb_record *)(((char *)head) + sizeof(*head)); \ (((char *)rec) < (((char *)head) + sizeof(*head) + head->table_bytes)) && \ @@ -162,24 +181,13 @@ (struct lb_header *)(((char *)base) + addr); struct lb_record *recs = (struct lb_record *)(((char *)base) + addr + sizeof(*head)); - if (memcmp(head->signature, "LBIO", 4) != 0) + if (!lb_header_valid(head, addr)) continue; - msg_pdbg("Found candidate at: %08lx-%08lx\n", - addr, addr + head->table_bytes); - if (head->header_bytes != sizeof(*head)) { - msg_perr("Header bytes of %d are incorrect.\n", - head->header_bytes); - continue; - } if (count_lb_records(head) != head->table_entries) { msg_perr("Bad record count: %d.\n", head->table_entries); continue; } - if (compute_checksum((uint8_t *) head, sizeof(*head)) != 0) { - msg_perr("Bad header checksum.\n"); - continue; - } if (compute_checksum(recs, head->table_bytes) != head->table_checksum) { msg_perr("Bad table checksum: %04x.\n",
Hello Angel Pons, build bot (Jenkins), Nico Huber, Patrick Georgi,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/flashrom/+/37238
to look at the new patch set (#2).
Change subject: cbtable.c: Factor out lb_header_validation logic ......................................................................
cbtable.c: Factor out lb_header_validation logic
Write a pure function for the header validation logic, it is easier to unit-test.
Change-Id: Ia288bcbc5c371329952a6efba30ccf0e18965a3d Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M cbtable.c 1 file changed, 20 insertions(+), 12 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/38/37238/2
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37238 )
Change subject: cbtable.c: Factor out lb_header_validation logic ......................................................................
Patch Set 2: Code-Review+2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37238 )
Change subject: cbtable.c: Factor out lb_header_validation logic ......................................................................
Patch Set 2: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/37238 )
Change subject: cbtable.c: Factor out lb_header_validation logic ......................................................................
cbtable.c: Factor out lb_header_validation logic
Write a pure function for the header validation logic, it is easier to unit-test.
Change-Id: Ia288bcbc5c371329952a6efba30ccf0e18965a3d Signed-off-by: Edward O'Callaghan quasisec@chromium.org Reviewed-on: https://review.coreboot.org/c/flashrom/+/37238 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M cbtable.c 1 file changed, 20 insertions(+), 12 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved Arthur Heymans: Looks good to me, approved
diff --git a/cbtable.c b/cbtable.c index bdf53ce..669a461 100644 --- a/cbtable.c +++ b/cbtable.c @@ -151,6 +151,25 @@ return count; }
+static int lb_header_valid(struct lb_header *head, unsigned long addr) +{ + if (memcmp(head->signature, "LBIO", 4) != 0) + return 0; + msg_pdbg("Found candidate at: %08lx-%08lx\n", + addr, addr + sizeof(*head) + head->table_bytes); + if (head->header_bytes != sizeof(*head)) { + msg_perr("Header bytes of %d are incorrect.\n", + head->header_bytes); + return 0; + } + if (compute_checksum((uint8_t *) head, sizeof(*head)) != 0) { + msg_perr("Bad header checksum.\n"); + return 0; + } + + return 1; +} + static struct lb_header *find_lb_table(void *base, unsigned long start, unsigned long end) { @@ -162,24 +181,13 @@ (struct lb_header *)(((char *)base) + addr); struct lb_record *recs = (struct lb_record *)(((char *)base) + addr + sizeof(*head)); - if (memcmp(head->signature, "LBIO", 4) != 0) + if (!lb_header_valid(head, addr)) continue; - msg_pdbg("Found candidate at: %08lx-%08lx\n", - addr, addr + head->table_bytes); - if (head->header_bytes != sizeof(*head)) { - msg_perr("Header bytes of %d are incorrect.\n", - head->header_bytes); - continue; - } if (count_lb_records(head) != head->table_entries) { msg_perr("Bad record count: %d.\n", head->table_entries); continue; } - if (compute_checksum((uint8_t *) head, sizeof(*head)) != 0) { - msg_perr("Bad header checksum.\n"); - continue; - } if (compute_checksum(recs, head->table_bytes) != head->table_checksum) { msg_perr("Bad table checksum: %04x.\n",
-
Arthur Heymans (Code Review) -
Edward O'Callaghan (Code Review)
-
Stefan Reinauer (Code Review)