Edward O'Callaghan has uploaded this change for review. ( https://review.coreboot.org/c/flashrom/+/37239 )
Change subject: cbtable.c: Factor out lb_table_validation logic ......................................................................
cbtable.c: Factor out lb_table_validation logic
Write a pure function for the table validation logic, it is easier to unit-test.
Change-Id: I07b0f95ec0443fa6a8f54eb93f4a7ea1875cccad Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M cbtable.c 1 file changed, 18 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/37239/1
diff --git a/cbtable.c b/cbtable.c index cd0cf15..14e9a85 100644 --- a/cbtable.c +++ b/cbtable.c @@ -150,6 +150,23 @@ return 1; }
+static int lb_table_valid(struct lb_header *head, struct lb_record *recs) +{ + if (compute_checksum(recs, head->table_bytes) + != head->table_checksum) { + msg_perr("Bad table checksum: %04x.\n", + head->table_checksum); + return 0; + } + if (count_lb_records(head) != head->table_entries) { + msg_perr("Bad record count: %d.\n", + head->table_entries); + 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)) && \ @@ -183,17 +200,8 @@ (struct lb_record *)(((char *)base) + addr + sizeof(*head)); if (!lb_header_valid(head, addr)) continue; - if (count_lb_records(head) != head->table_entries) { - msg_perr("Bad record count: %d.\n", - head->table_entries); + if (!lb_table_valid(head, recs)) continue; - } - if (compute_checksum(recs, head->table_bytes) - != head->table_checksum) { - msg_perr("Bad table checksum: %04x.\n", - head->table_checksum); - continue; - } msg_pdbg("Found coreboot table at 0x%08lx.\n", addr); return head;
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/+/37239
to look at the new patch set (#2).
Change subject: cbtable.c: Factor out lb_table_validation logic ......................................................................
cbtable.c: Factor out lb_table_validation logic
Write a pure function for the table validation logic, it is easier to unit-test.
Change-Id: I07b0f95ec0443fa6a8f54eb93f4a7ea1875cccad Signed-off-by: Edward O'Callaghan quasisec@chromium.org --- M cbtable.c 1 file changed, 18 insertions(+), 10 deletions(-)
git pull ssh://review.coreboot.org:29418/flashrom refs/changes/39/37239/2
Stefan Reinauer has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/37239 )
Change subject: cbtable.c: Factor out lb_table_validation logic ......................................................................
Patch Set 2: Code-Review+2
Edward O'Callaghan has submitted this change. ( https://review.coreboot.org/c/flashrom/+/37239 )
Change subject: cbtable.c: Factor out lb_table_validation logic ......................................................................
cbtable.c: Factor out lb_table_validation logic
Write a pure function for the table validation logic, it is easier to unit-test.
Change-Id: I07b0f95ec0443fa6a8f54eb93f4a7ea1875cccad Signed-off-by: Edward O'Callaghan quasisec@chromium.org Reviewed-on: https://review.coreboot.org/c/flashrom/+/37239 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Stefan Reinauer stefan.reinauer@coreboot.org --- M cbtable.c 1 file changed, 18 insertions(+), 10 deletions(-)
Approvals: build bot (Jenkins): Verified Stefan Reinauer: Looks good to me, approved
diff --git a/cbtable.c b/cbtable.c index 669a461..e566840 100644 --- a/cbtable.c +++ b/cbtable.c @@ -170,6 +170,23 @@ return 1; }
+static int lb_table_valid(struct lb_header *head, struct lb_record *recs) +{ + if (compute_checksum(recs, head->table_bytes) + != head->table_checksum) { + msg_perr("Bad table checksum: %04x.\n", + head->table_checksum); + return 0; + } + if (count_lb_records(head) != head->table_entries) { + msg_perr("Bad record count: %d.\n", + head->table_entries); + return 0; + } + + return 1; +} + static struct lb_header *find_lb_table(void *base, unsigned long start, unsigned long end) { @@ -183,17 +200,8 @@ (struct lb_record *)(((char *)base) + addr + sizeof(*head)); if (!lb_header_valid(head, addr)) continue; - if (count_lb_records(head) != head->table_entries) { - msg_perr("Bad record count: %d.\n", - head->table_entries); + if (!lb_table_valid(head, recs)) continue; - } - if (compute_checksum(recs, head->table_bytes) - != head->table_checksum) { - msg_perr("Bad table checksum: %04x.\n", - head->table_checksum); - continue; - } msg_pdbg("Found coreboot table at 0x%08lx.\n", addr); return head;
-
Edward O'Callaghan (Code Review) -
Stefan Reinauer (Code Review)