On 1/4/24 16:06, Gerd Hoffmann wrote: > Hi, > >>>> - if the StartId is 0x55aa, then we need to look further, beause we >>>> can't decide yet. For example, if State is VAR_HEADER_VALID_ONLY (0x7f), >>>> then it might be fine for the variable header (at the very end of the >>>> varstore) *not* to be followed by payload bytes (name, data). >>> >>> Not sure this makes sense. VAR_HEADER_VALID_ONLY is a temporary state, >>> while the variable driver writes name and data just after the header, >>> to be updated to VAR_ADDED when the write completed successfully. So >>> I'd expect to never find a header without space for name + data. >> >> - Do we know for sure that VAR_HEADER_VALID_ONLY is never expected to be >> seen? > > Writing goes like this: > > (1) find free space > (2) write header, with VAR_HEADER_VALID_ONLY. > (3) write name + data > (4) update header, set state = VAR_ADDED. > >> What if the variable update design defines VAR_HEADER_VALID_ONLY >> specifically so that the variable driver can recover from a power loss >> "in the middle"? > > Power loss in step (3) can surely lead to variables in > VAR_HEADER_VALID_ONLY state, and I'd expect the variable driver can > actually recover from that. > > [ side note: The (2) write should be small enough that it fits into the > flash block write buffer (128 bytes). Which could be > important to maintain variable store consistency. ] > > Nevertheless we should never find a header at the end of the variable > store, without space allocated for name + date. Minimal space for the > name is 4 bytes (one char16 + '\0'), for the data 1 byte, alignment > rounds the latter to 4 bytes too, so this should be true: > > VarOffset + sizeof(*VarHeader) + 8 <= VariableStoreHeader->Size > >> So I figure, if we accept VAR_HEADER_VALID_ONLY in that logic, then we >> should also accept VAR_HEADER_VALID_ONLY if it's at the very end of >> the varstore. > > Disagree, see above. Storing the header at a place which leaves no room > for name + data doesn't make sense to me.
OK, that sounds convincing, thanks! Laszlo > We could go the extra mile and look at the next StartId location, verify > StartId != 0x55aa, in the no-space-left-for-header case. > > take care, > Gerd > -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#113283): https://edk2.groups.io/g/devel/message/113283 Mute This Topic: https://groups.io/mt/103171811/21656 Group Owner: devel+ow...@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/leave/9847357/21656/1706620634/xyzzy [arch...@mail-archive.com] -=-=-=-=-=-=-=-=-=-=-=-
