Change in flashrom[master]: SFDP: drop static table length check to make newer SFDP revisions work
Attention is currently required from: Edward O'Callaghan, Felix Singer, Angel Pons, Anastasia Klimchuk, Michael Niewöhner. Nico Huber has posted comments on this change. ( https://review.coreboot.org/c/flashrom/+/60055 )
Change subject: SFDP: drop static table length check to make newer SFDP revisions work ......................................................................
Patch Set 2:
(1 comment)
Commit Message:
https://review.coreboot.org/c/flashrom/+/60055/comment/eca0edc4_33f2e601 PS1, Line 28: ***RFC*** We could maintain a list of revisions/lengths but is it really : worth it? :
and that the major version is 1.
That's already checked.
Ack. I missed that the actual action is inside an else-if condition.
But..... According to JESD216 there is a conflict between JESD216 and Intel legacy. It reads like Intel used 1.x, x=[1-4] \o/ I couldn't find *anything* about that, though.......
Well, the code seems to assume legacy Intel has a length of 16. It also seems unlikely that any newer legacy-Intel version would show up with a higher length. So we'd just have to maintain the current quirk: always allow length 16 for major version 1. We allow that already, so it won't get worse.
Checking the minimum length would avoid undefined behavior (accessing bytes we didn't read). IMO, that's all we have to do. We can either use the minimum we need (36, currently) or the minimum mandated by the spec. So either
} else if (len != 4 * 4 && len < 9 * 4) {
or trying to be more accurate
} else if (len != 4 * 4 && len < mandated_length[minor_idx]) {
with `minor_idx = MIN(ARRAY_SIZE(mandated_length) - 1, hdrs[i].v_minor);`.
-
Nico Huber (Code Review)