vboot/vboot_kernel: change recovery behaviour on kernel failure
On load kernel failure, only call vb2api_fail when in normal
mode.
Previously, the behaviour here was to only call when attempting
to load a kernel from a fixed disk. This maps to (1) normal
mode, and (2) developer mode when booting from an internal disk.
Excluding (2) creates a more consistent experience in developer
mode, and also prepares for a world where recovery kernels might
exist on disk.
This CL is part of a series to merge vboot1 and vboot2.0
kernel verification code; see b/181739551.
BUG=b:181739551, b:188121855
TEST=make clean && make runtests
BRANCH=none
Signed-off-by: Joel Kitching <kitching@google.com>
Change-Id: Ic2c55a073b036be98f4ce9b2e0c7fb3209de74c8
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/vboot_reference/+/2917623
Tested-by: Joel Kitching <kitching@chromium.org>
Commit-Queue: Joel Kitching <kitching@chromium.org>
Reviewed-by: Julius Werner <jwerner@chromium.org>
diff --git a/firmware/lib/vboot_api_kernel.c b/firmware/lib/vboot_api_kernel.c
index b28cc44..93de040 100644
--- a/firmware/lib/vboot_api_kernel.c
+++ b/firmware/lib/vboot_api_kernel.c
@@ -106,7 +106,8 @@
}
/* If we drop out of the loop, we didn't find any usable kernel. */
- if (disk_flags & VB_DISK_FLAG_FIXED) {
+ if (!(ctx->flags & VB2_CONTEXT_RECOVERY_MODE) &&
+ !(ctx->flags & VB2_CONTEXT_DEVELOPER_MODE)) {
switch (rv) {
case VB2_ERROR_LK_INVALID_KERNEL_FOUND:
vb2api_fail(ctx, VB2_RECOVERY_RW_INVALID_OS, rv);
diff --git a/tests/vboot_api_kernel_tests.c b/tests/vboot_api_kernel_tests.c
index 83b6d6c..9294fd7 100644
--- a/tests/vboot_api_kernel_tests.c
+++ b/tests/vboot_api_kernel_tests.c
@@ -29,6 +29,7 @@
const char *name;
/* inputs for test case */
+ uint32_t ctx_flags;
uint32_t want_flags;
vb2_error_t diskgetinfo_return_val;
disk_desc_t disks_to_provide[MAX_TEST_DISKS];
@@ -53,6 +54,7 @@
test_case_t test[] = {
{
.name = "first drive (removable)",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_REMOVABLE | VB_DISK_FLAG_FIXED,
.disks_to_provide = {
{4096, 100, VB_DISK_FLAG_REMOVABLE, pickme},
@@ -70,6 +72,7 @@
},
{
.name = "first drive (fixed)",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_REMOVABLE | VB_DISK_FLAG_FIXED,
.disks_to_provide = {
{4096, 100, VB_DISK_FLAG_FIXED, pickme},
@@ -87,6 +90,7 @@
},
{
.name = "first removable drive",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_REMOVABLE,
.disks_to_provide = {
/* too small */
@@ -117,6 +121,7 @@
},
{
.name = "first removable drive (skip external GPT)",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_REMOVABLE,
.disks_to_provide = {
/* too small */
@@ -149,6 +154,7 @@
},
{
.name = "second removable drive",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_REMOVABLE,
.disks_to_provide = {
/* wrong flags */
@@ -167,6 +173,7 @@
},
{
.name = "first fixed drive",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_FIXED,
.disks_to_provide = {
/* too small */
@@ -199,6 +206,7 @@
},
{
.name = "no drives at all",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_FIXED,
.disks_to_provide = {},
.disk_count_to_return = DEFAULT_COUNT,
@@ -211,6 +219,7 @@
},
{
.name = "VbExDiskGetInfo() error",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_FIXED,
.disks_to_provide = {
{512, 10, VB_DISK_FLAG_REMOVABLE, 0},
@@ -226,6 +235,7 @@
},
{
.name = "invalid kernel",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_FIXED,
.disks_to_provide = {
/* too small */
@@ -257,6 +267,7 @@
},
{
.name = "invalid kernel, order flipped",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_FIXED,
.disks_to_provide = {
{512, 1000, VB_DISK_FLAG_FIXED, "stateful partition"},
@@ -274,6 +285,7 @@
},
{
.name = "no Chrome OS partitions",
+ .ctx_flags = 0,
.want_flags = VB_DISK_FLAG_FIXED,
.disks_to_provide = {
{512, 100, VB_DISK_FLAG_FIXED, "stateful partition"},
@@ -291,6 +303,43 @@
},
{
.name = "invalid kernel (removable)",
+ .ctx_flags = 0,
+ .want_flags = VB_DISK_FLAG_REMOVABLE,
+ .disks_to_provide = {
+ {512, 100, VB_DISK_FLAG_REMOVABLE, "corrupted"},
+ {512, 100, VB_DISK_FLAG_REMOVABLE, "data"},
+ },
+ .disk_count_to_return = DEFAULT_COUNT,
+ .diskgetinfo_return_val = VB2_SUCCESS,
+ .loadkernel_return_val = {VB2_ERROR_LK_INVALID_KERNEL_FOUND,
+ VB2_ERROR_LK_NO_KERNEL_FOUND},
+
+ .expected_recovery_request_val = VB2_RECOVERY_RW_INVALID_OS,
+ .expected_to_find_disk = DONT_CARE,
+ .expected_to_load_disk = 0,
+ .expected_return_val = VB2_ERROR_LK_INVALID_KERNEL_FOUND,
+ },
+ {
+ .name = "invalid kernel (removable, rec mode)",
+ .ctx_flags = VB2_CONTEXT_RECOVERY_MODE,
+ .want_flags = VB_DISK_FLAG_REMOVABLE,
+ .disks_to_provide = {
+ {512, 100, VB_DISK_FLAG_REMOVABLE, "corrupted"},
+ {512, 100, VB_DISK_FLAG_REMOVABLE, "data"},
+ },
+ .disk_count_to_return = DEFAULT_COUNT,
+ .diskgetinfo_return_val = VB2_SUCCESS,
+ .loadkernel_return_val = {VB2_ERROR_LK_INVALID_KERNEL_FOUND,
+ VB2_ERROR_LK_NO_KERNEL_FOUND},
+
+ .expected_recovery_request_val = VB2_RECOVERY_NOT_REQUESTED,
+ .expected_to_find_disk = DONT_CARE,
+ .expected_to_load_disk = 0,
+ .expected_return_val = VB2_ERROR_LK_INVALID_KERNEL_FOUND,
+ },
+ {
+ .name = "invalid kernel (removable, dev mode)",
+ .ctx_flags = VB2_CONTEXT_DEVELOPER_MODE,
.want_flags = VB_DISK_FLAG_REMOVABLE,
.disks_to_provide = {
{512, 100, VB_DISK_FLAG_REMOVABLE, "corrupted"},
@@ -308,6 +357,23 @@
},
{
.name = "no kernel (removable)",
+ .ctx_flags = 0,
+ .want_flags = VB_DISK_FLAG_REMOVABLE,
+ .disks_to_provide = {
+ {512, 100, VB_DISK_FLAG_REMOVABLE, "data"},
+ },
+ .disk_count_to_return = DEFAULT_COUNT,
+ .diskgetinfo_return_val = VB2_SUCCESS,
+ .loadkernel_return_val = {VB2_ERROR_LK_NO_KERNEL_FOUND},
+
+ .expected_recovery_request_val = VB2_RECOVERY_RW_NO_KERNEL,
+ .expected_to_find_disk = DONT_CARE,
+ .expected_to_load_disk = 0,
+ .expected_return_val = VB2_ERROR_LK_NO_KERNEL_FOUND,
+ },
+ {
+ .name = "no kernel (removable, rec mode)",
+ .ctx_flags = VB2_CONTEXT_RECOVERY_MODE,
.want_flags = VB_DISK_FLAG_REMOVABLE,
.disks_to_provide = {
{512, 100, VB_DISK_FLAG_REMOVABLE, "data"},
@@ -464,7 +530,8 @@
for (i = 0; i < num_tests; i++) {
printf("Test case: %s ...\n", test[i].name);
ResetMocks(i);
- TEST_EQ(VbTryLoadKernel(ctx, test[i].want_flags),
+ ctx->flags = t->ctx_flags;
+ TEST_EQ(VbTryLoadKernel(ctx, t->want_flags),
t->expected_return_val, " return value");
TEST_EQ(got_recovery_request_val,
t->expected_recovery_request_val, " recovery_request");
