Skip to content

Commit 13373e8

Browse files
committed
DasharoModulePkg/DasharoVariablesLib: fix non-deterministic measurements
This fixes "SecurityPkg: measure Dasharo variables before boot". gRT->GetNextVariableName() doesn't return variables in any fixed order. Seems like the order matches order in SMMSTORE. This means that measuring variables while enumerating them will produce different results depending on which variables were update last (setting a variable in SMMSTORE is marking old entry as deleted and appending of a new one). Sort list of variables that share the same GUID before measuring any of them to impose a fixed order. Also fix spacing in several places. Signed-off-by: Sergii Dmytruk <[email protected]>
1 parent ae8cb4a commit 13373e8

File tree

1 file changed

+86
-4
lines changed

1 file changed

+86
-4
lines changed

DasharoModulePkg/Library/DasharoVariablesLib/DasharoVariablesLib.c

Lines changed: 86 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,27 @@ MeasureVariable (
338338
return Status;
339339
}
340340

341+
/**
342+
A comparison function for sorting an array of variable names.
343+
344+
@param Buffer1 Pointer to pointer of the first variable name.
345+
@param Buffer2 Pointer to pointer of the second variable name.
346+
347+
@retval <0 The first variable name is less than the second one.
348+
@retval =0 The names are equal.
349+
@retval >0 The first variable name is greater than the second one.
350+
**/
351+
STATIC
352+
INTN
353+
EFIAPI
354+
CompareVariableNames (
355+
IN CONST VOID *Buffer1,
356+
IN CONST VOID *Buffer2
357+
)
358+
{
359+
return StrCmp (*(CONST CHAR16 **) Buffer1, *(CONST CHAR16 **) Buffer2);
360+
}
361+
341362
/**
342363
Measures single all existing variables with the specified GUID.
343364
@@ -357,12 +378,25 @@ MeasureVariables (
357378
UINTN MaxNameSize;
358379
UINTN NameSize;
359380
EFI_GUID Guid;
381+
CHAR16 **Names;
382+
UINTN NameCount;
383+
CHAR16 SortBuf;
384+
UINTN MaxNameCount;
385+
UINTN Index;
360386

361-
MaxNameSize = 32*sizeof (CHAR16);
387+
MaxNameSize = 32 * sizeof (CHAR16);
362388
Name = AllocateZeroPool (MaxNameSize);
363389
if (Name == NULL)
364390
return EFI_OUT_OF_RESOURCES;
365391

392+
MaxNameCount = 32;
393+
NameCount = 0;
394+
Names = AllocatePool (MaxNameCount * sizeof (*Names));
395+
if (Names == NULL) {
396+
FreePool(Name);
397+
return EFI_OUT_OF_RESOURCES;
398+
}
399+
366400
while (TRUE) {
367401
NameSize = MaxNameSize;
368402
Status = gRT->GetNextVariableName (&NameSize, Name, &Guid);
@@ -373,7 +407,7 @@ MeasureVariables (
373407
break;
374408
}
375409

376-
StrnCpyS (NewBuf, NameSize/sizeof (CHAR16), Name, MaxNameSize/sizeof (CHAR16));
410+
StrnCpyS (NewBuf, NameSize / sizeof (CHAR16), Name, MaxNameSize / sizeof (CHAR16));
377411
FreePool (Name);
378412

379413
Name = NewBuf;
@@ -390,11 +424,59 @@ MeasureVariables (
390424
if (EFI_ERROR (Status))
391425
break;
392426

393-
if (CompareGuid (&Guid, Vendor))
394-
MeasureVariable (Name, Vendor);
427+
if (!CompareGuid (&Guid, Vendor))
428+
continue;
429+
430+
if (NameCount == MaxNameCount - 1) {
431+
Names = ReallocatePool (
432+
MaxNameCount * sizeof (*Names),
433+
2 * MaxNameCount * sizeof (*Names),
434+
Names
435+
);
436+
if (Names == NULL) {
437+
Status = EFI_OUT_OF_RESOURCES;
438+
break;
439+
}
440+
441+
MaxNameCount *= 2;
442+
}
443+
444+
Names[NameCount] = AllocateCopyPool (NameSize, Name);
445+
if (Names[NameCount] == NULL) {
446+
Status = EFI_OUT_OF_RESOURCES;
447+
break;
448+
}
449+
450+
NameCount++;
395451
}
396452

453+
if (Status == EFI_SUCCESS) {
454+
//
455+
// Achieve predictable ordering of variables by sorting them by name within
456+
// a particular vendor.
457+
//
458+
QuickSort (
459+
Names,
460+
NameCount,
461+
sizeof (*Names),
462+
CompareVariableNames,
463+
&SortBuf
464+
);
465+
466+
for (Index = 0; Index < NameCount; Index++) {
467+
Status = MeasureVariable (Names[Index], Vendor);
468+
if (EFI_ERROR (Status)) {
469+
DEBUG ((EFI_D_WARN, "%a(): Failed to measure variable: %g:%s.\n",
470+
__FUNCTION__, Vendor, Name));
471+
}
472+
}
473+
}
474+
475+
for (Index = 0; Index < NameCount; Index++)
476+
FreePool (Names[Index]);
477+
397478
FreePool (Name);
479+
FreePool (Names);
398480
return Status;
399481
}
400482

0 commit comments

Comments
 (0)