On 08/21/15 20:28, Michael Zimmermann wrote:
> I'm using this lib on ARM and it generates unaligned access in it's current
> state due to the heavy use of pointer casts.
> This changes the binary format so all previously saved data will be lost.
>
> diff --git a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c
> b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c
> index 6822c5c..f6c92c1 100644
(1) malformed patch
(2) subject line doesn't spell out package & component name (should
start with OvmfPkg: SerializeVariablesLib: ...)
(3) what are you using this for on ARM? This library had been meant as a
temporary stop-gap for OVMF until OVMF got flash (and therefore real
non-volatile variable support). In general the way forward here should
be removing this infrastructure from OvmfPkg, not adopting it elsewhere.
Thanks
Laszlo
> --- a/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c
> +++ b/OvmfPkg/Library/SerializeVariablesLib/SerializeVariablesLib.c
> @@ -80,13 +80,13 @@ UnpackVariableFromBuffer (
> }
>
> *Name = (CHAR16*) (BytePtr + Offset);
> - Offset = Offset + *(UINT32*)BytePtr;
> + Offset = Offset + ALIGN_VALUE(*NameSize, sizeof(UINT32));
> if (Offset > MaxSize) {
> return EFI_INVALID_PARAMETER;
> }
>
> *Guid = (EFI_GUID*) (BytePtr + Offset);
> - Offset = Offset + sizeof (EFI_GUID);
> + Offset = Offset + ALIGN_VALUE(sizeof (EFI_GUID), sizeof(UINT32));
> if (Offset > MaxSize) {
> return EFI_INVALID_PARAMETER;
> }
> @@ -104,7 +104,7 @@ UnpackVariableFromBuffer (
> }
>
> *Data = (VOID*) (BytePtr + Offset);
> - Offset = Offset + *DataSize;
> + Offset = Offset + ALIGN_VALUE(*DataSize, sizeof(UINT32));
> if (Offset > MaxSize) {
> return EFI_INVALID_PARAMETER;
> }
> @@ -284,26 +284,26 @@ IterateVariablesCallbackSetSystemVariable (
> IN VOID *Data
> )
> {
> - EFI_STATUS Status;
> - STATIC CONST UINT32 AuthMask =
> - EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
> - EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> -
> - Status = gRT->SetVariable (
> - VariableName,
> - VendorGuid,
> - Attributes,
> - DataSize,
> - Data
> - );
> -
> - if (Status == EFI_SECURITY_VIOLATION && (Attributes & AuthMask) != 0) {
> - DEBUG ((DEBUG_WARN, "%a: setting authenticated variable \"%s\" "
> - "failed with EFI_SECURITY_VIOLATION, ignoring\n", __FUNCTION__,
> - VariableName));
> - Status = EFI_SUCCESS;
> - }
> - return Status;
> + EFI_STATUS Status;
> + STATIC CONST UINT32 AuthMask =
> + EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS |
> + EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS;
> +
> + Status = gRT->SetVariable (
> + VariableName,
> + VendorGuid,
> + Attributes,
> + DataSize,
> + Data
> + );
> +
> + if (Status == EFI_SECURITY_VIOLATION && (Attributes & AuthMask) != 0) {
> + DEBUG ((DEBUG_WARN, "%a: setting authenticated variable \"%s\" "
> + "failed with EFI_SECURITY_VIOLATION, ignoring\n", __FUNCTION__,
> + VariableName));
> + Status = EFI_SUCCESS;
> + }
> + return Status;
> }
>
>
> @@ -366,7 +366,7 @@ AppendToBuffer (
> Size
> );
>
> - Instance->DataSize = NewSize;
> + Instance->DataSize = ALIGN_VALUE(NewSize, sizeof(UINT32));
> }
>
>
> @@ -764,12 +764,12 @@ SerializeVariablesAddVariable (
> SerializedNameSize = (UINT32) StrSize (VariableName);
>
> SerializedSize =
> - sizeof (SerializedNameSize) +
> - SerializedNameSize +
> - sizeof (*VendorGuid) +
> - sizeof (Attributes) +
> - sizeof (SerializedDataSize) +
> - DataSize;
> + ALIGN_VALUE(sizeof (SerializedNameSize), sizeof(UINT32)) +
> + ALIGN_VALUE(SerializedNameSize, sizeof(UINT32)) +
> + ALIGN_VALUE(sizeof (*VendorGuid), sizeof(UINT32)) +
> + ALIGN_VALUE(sizeof (Attributes), sizeof(UINT32)) +
> + ALIGN_VALUE(sizeof (SerializedDataSize), sizeof(UINT32)) +
> + ALIGN_VALUE(DataSize, sizeof(UINT32));
>
> Status = EnsureExtraBufferSpace (
> Instance,
> _______________________________________________
> edk2-devel mailing list
> [email protected]
> https://lists.01.org/mailman/listinfo/edk2-devel
>
_______________________________________________
edk2-devel mailing list
[email protected]
https://lists.01.org/mailman/listinfo/edk2-devel
