Attention is currently required from: Angel Pons, Light, Nikolai Artemiev. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/62733 )
Change subject: writeprotect_ranges.c: Use 1UL if left shifting more than 32bits ......................................................................
Patch Set 4:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/62733/comment/ca7449dd_562a8093 PS2, Line 12: Still the bug is not fixed :(
I don't think it would work as the max value that bp_max can take is 15 since the array bits->bp is […]
The types can't help us because an underflow or a negative `bp_max - 2` both would result in undefined behavior. From C11 about the shift operators:
"If the value of the right operand is negative or is greater than or equal to the width of the promoted left operand, the behavior is undefined."
As far as I can see, the bug is only a theoretical one that depends on the arguments passed to this function (i.e. `bits->bp_bit_count`). I see two potential solutions: 1. Either add an assert() that would provide a contract how this function should be called, e.g. on the top of the function:
assert(bits->bp_bit_count > 1);
2. Defensive programming. We could add an `if` that returns an error if this happens.
Personally, I'm much a fan of 1. But I am not sure if it will be accepted by scan-build.