Bug with data marshaling in SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c

[wmjdgla]

Tpm2PcrRead is doing a member-by-member copy of the input TPML_PCR_SELECTION parameter rather than marshaling it:

edk2/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c

Lines 359 to 364 in b7f8779

	SendBuffer.PcrSelectionIn.count = SwapBytes32 (PcrSelectionIn->count);
	for (Index = 0; Index < PcrSelectionIn->count; Index++) {
	SendBuffer.PcrSelectionIn.pcrSelections[Index].hash = SwapBytes16 (PcrSelectionIn->pcrSelections[Index].hash);
	SendBuffer.PcrSelectionIn.pcrSelections[Index].sizeofSelect = PcrSelectionIn->pcrSelections[Index].sizeofSelect;
	CopyMem (&SendBuffer.PcrSelectionIn.pcrSelections[Index].pcrSelect, &PcrSelectionIn->pcrSelections[Index].pcrSelect, SendBuffer.PcrSelectionIn.pcrSelections[Index].sizeofSelect);
	}

This results in the TPM2_PCR_READ_COMMAND bytes being malformed whenever sizeofSelect != PCR_SELECT_MAX.

Further down, Tpm2PcrAllocate also marshals an input TPML_PCR_SELECTION parameter and it is done correctly:

edk2/SecurityPkg/Library/Tpm2CommandLib/Tpm2Integrity.c

Lines 515 to 524 in b7f8779

	WriteUnaligned32 ((UINT32 *)Buffer, SwapBytes32 (PcrAllocation->count));
	Buffer += sizeof (UINT32);
	for (Index = 0; Index < PcrAllocation->count; Index++) {
	WriteUnaligned16 ((UINT16 *)Buffer, SwapBytes16 (PcrAllocation->pcrSelections[Index].hash));
	Buffer += sizeof (UINT16);
	*(UINT8 *)Buffer = PcrAllocation->pcrSelections[Index].sizeofSelect;
	Buffer++;
	CopyMem (Buffer, PcrAllocation->pcrSelections[Index].pcrSelect, PcrAllocation->pcrSelections[Index].sizeofSelect);
	Buffer += PcrAllocation->pcrSelections[Index].sizeofSelect;
	}

[jyao1]

Yes, agree this is a bug. Do you want to propose a patch?