Patch Set 1:

(1 comment)

I'm somewhat confused how multiplying two u16 into an s32 could cause overflow or sign issues? This seems unnecessary. (Also, if you do want to do this, note that size_t is dependent on the host architecture which is not guaranteed to be x86_64. You should use an explicit-width type like Elf64_Off or uint64_t.)

It is possible for the two u16 to overflow an s32, which will lead to a negative number (which is already UB), and then converting to a u64 will sign-extend it to an extremely large integer. Eg.

    int32_t x = -1;
uint64_t y = x; // x is sign-extended to UINT64_MAX

For the size_t issue I suppose 64 bits itself doesn't matter as long as it is at least a u32, which can always hold a u16 * u16. I think this should be true on all platforms we support. I'll clarify this in the commit message.

View Change

To view, visit change 33986. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I5391221d46d620d0e5bd629e2f9680be7a53342e
Gerrit-Change-Number: 33986
Gerrit-PatchSet: 1
Gerrit-Owner: Jacob Garber <jgarber1@ualberta.ca>
Gerrit-Reviewer: Jacob Garber <jgarber1@ualberta.ca>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-CC: Julius Werner <jwerner@chromium.org>
Gerrit-Comment-Date: Tue, 02 Jul 2019 21:35:15 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment