Hello Furquan Shaikh, Patrick Georgi,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/48277
to review the following change.
Change subject: cbfs: mcache: Fix end-of-cache check ......................................................................
cbfs: mcache: Fix end-of-cache check
After the mcache is copied into CBMEM, it has *just* the right size to fit the final tag with no room to spare. That means the test to check if we walked over the end must be `end - current >= sizeof(tag)`, not `end - current > sizeof(tag)`.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I25a0d774fb3294bb4d15f31f432940bfccc84af0 --- M src/commonlib/bsd/cbfs_mcache.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/48277/1
diff --git a/src/commonlib/bsd/cbfs_mcache.c b/src/commonlib/bsd/cbfs_mcache.c index d2f969d..0d5d8e0 100644 --- a/src/commonlib/bsd/cbfs_mcache.c +++ b/src/commonlib/bsd/cbfs_mcache.c @@ -95,7 +95,7 @@ const void *end = mcache + mcache_size; const void *current = mcache;
- while (current + sizeof(uint32_t) < end) { + while (current + sizeof(uint32_t) <= end) { const union mcache_entry *entry = current;
if (entry->magic == MCACHE_MAGIC_END)
Hello Furquan Shaikh, Patrick Georgi, Patrick Rudolph, Arthur Heymans,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/48277
to look at the new patch set (#2).
Change subject: cbfs: mcache: Fix end-of-cache check ......................................................................
cbfs: mcache: Fix end-of-cache check
After the mcache is copied into CBMEM, it has *just* the right size to fit the final tag with no room to spare. That means the test to check if we walked over the end must be `current + sizeof(tag) <= end`, not `current + sizeof(tag) < end`.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I25a0d774fb3294bb4d15f31f432940bfccc84af0 --- M src/commonlib/bsd/cbfs_mcache.c 1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/77/48277/2
Arthur Heymans has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/48277 )
Change subject: cbfs: mcache: Fix end-of-cache check ......................................................................
Patch Set 2: Code-Review+2
Julius Werner has submitted this change. ( https://review.coreboot.org/c/coreboot/+/48277 )
Change subject: cbfs: mcache: Fix end-of-cache check ......................................................................
cbfs: mcache: Fix end-of-cache check
After the mcache is copied into CBMEM, it has *just* the right size to fit the final tag with no room to spare. That means the test to check if we walked over the end must be `current + sizeof(tag) <= end`, not `current + sizeof(tag) < end`.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I25a0d774fb3294bb4d15f31f432940bfccc84af0 Reviewed-on: https://review.coreboot.org/c/coreboot/+/48277 Tested-by: build bot (Jenkins) no-reply@coreboot.org Reviewed-by: Arthur Heymans arthur@aheymans.xyz --- M src/commonlib/bsd/cbfs_mcache.c 1 file changed, 1 insertion(+), 1 deletion(-)
Approvals: build bot (Jenkins): Verified Arthur Heymans: Looks good to me, approved
diff --git a/src/commonlib/bsd/cbfs_mcache.c b/src/commonlib/bsd/cbfs_mcache.c index d2f969d..0d5d8e0 100644 --- a/src/commonlib/bsd/cbfs_mcache.c +++ b/src/commonlib/bsd/cbfs_mcache.c @@ -95,7 +95,7 @@ const void *end = mcache + mcache_size; const void *current = mcache;
- while (current + sizeof(uint32_t) < end) { + while (current + sizeof(uint32_t) <= end) { const union mcache_entry *entry = current;
if (entry->magic == MCACHE_MAGIC_END)
-
Arthur Heymans (Code Review) -
Julius Werner (Code Review)