Skip to content

Commit

Permalink
shim: Allow data after the end of device path node in load options
Browse files Browse the repository at this point in the history 
When looking for load option optional data, the parser asserts that the
byte after the end of device path node is the same as what the file path
length says it should be. While unusual, it is valid if the end of
device path node comes before the end of the file path list.

That supports some unusual Dell load options where there are two device
paths in the list but the first is terminated by an End Entire Device
Path. Maybe they intended to use an End Device Path Instance node there?
Who knows. Either way, treating it as invalid ends up trying to read
paths from the beginning of the option with obviously poor results.

Fixes: #649

Signed-off-by: Dan Nicholson <dbn@endlessos.org>
  • Loading branch information
@dbnicholson
dbnicholson committed Oct 2, 2024
1 parent e064e7d commit 2cabe53
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 7 deletions.
8 changes: 4 additions & 4 deletions load-options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,14 @@ get_load_option_optional_data(VOID *data, UINT32 data_size,
*/
i += dp.len;
}
if (i != fplistlen)
if (i > fplistlen)
return EFI_INVALID_PARAMETER;

/*
* if there's any space left, it's "optional data"
* Anything left after the file path list is optional data.
*/
*od = cur + i;
*ods = limit - i;
*od = cur + fplistlen;
*ods = limit - fplistlen;
return EFI_SUCCESS;
}

Expand Down
77 changes: 74 additions & 3 deletions test-load-options.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ test_parse_load_options(char *load_option_data,
assert_false_goto(EFI_ERROR(status), err, "\n");

assert_nonzero_goto(second_stage, err, "\n");
assert_not_equal_goto(second_stage, dummy_second_stage, err, "%p == %p\n");
assert_zero_goto(StrnCmp(second_stage, target_second_stage, 90),
err_print_second_stage, "%d != 0\n");
if (target_second_stage) {
assert_not_equal_goto(second_stage, dummy_second_stage, err, "%p == %p\n");
assert_zero_goto(StrnCmp(second_stage, target_second_stage, 90),
err_print_second_stage, "%d != 0\n");
} else
assert_equal_goto(second_stage, dummy_second_stage, err, "%p != %p\n");

assert_equal_goto(load_options_size, target_remaining_size, err_remaining, "%zu != %zu\n");
assert_equal_goto(load_options, target_remaining, err_remaining, "%p != %p\n");
Expand Down Expand Up @@ -255,6 +258,73 @@ test_bds_2(void)
target_remaining_size);
}

int
test_multi_end_dp(void)
{
/*
00000000 01 00 00 00 d0 00 4f 00 6e 00 62 00 6f 00 61 00 |......O.n.b.o.a.|
00000010 72 00 64 00 20 00 4e 00 49 00 43 00 28 00 49 00 |r.d. .N.I.C.(.I.|
00000020 50 00 56 00 34 00 29 00 00 00 02 01 0c 00 d0 41 |P.V.4.)........A|
00000030 03 0a 00 00 00 00 01 01 06 00 06 1f 03 0b 25 00 |..............%.|
00000040 2c ea 7f 0a 9f 69 00 00 00 00 00 00 00 00 00 00 |,....i..........|
00000050 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000060 00 03 0c 1b 00 00 00 00 00 00 00 00 00 00 00 00 |................|
00000070 00 00 00 00 00 00 00 00 00 00 00 00 7f ff 04 00 |................|
00000080 01 04 76 00 ef 47 64 2d c9 3b a0 41 ac 19 4d 51 |..v..Gd-.;.A..MQ|
00000090 d0 1b 4c e6 50 00 58 00 45 00 20 00 49 00 50 00 |..L.P.X.E. .I.P.|
000000a0 34 00 20 00 49 00 6e 00 74 00 65 00 6c 00 28 00 |4. .I.n.t.e.l.(.|
000000b0 52 00 29 00 20 00 45 00 74 00 68 00 65 00 72 00 |R.). .E.t.h.e.r.|
000000c0 6e 00 65 00 74 00 20 00 43 00 6f 00 6e 00 6e 00 |n.e.t. .C.o.n.n.|
000000d0 65 00 63 00 74 00 69 00 6f 00 6e 00 20 00 28 00 |e.c.t.i.o.n. .(.|
000000e0 36 00 29 00 20 00 49 00 32 00 31 00 39 00 2d 00 |6.). .I.2.1.9.-.|
000000f0 4c 00 4d 00 00 00 7f ff 04 00 00 00 42 4f |L.M.........BO|
*/
char load_option_data [] = {
0x01, 0x00, 0x00, 0x00, 0xd0, 0x00, 0x4f, 0x00,
0x6e, 0x00, 0x62, 0x00, 0x6f, 0x00, 0x61, 0x00,
0x72, 0x00, 0x64, 0x00, 0x20, 0x00, 0x4e, 0x00,
0x49, 0x00, 0x43, 0x00, 0x28, 0x00, 0x49, 0x00,
0x50, 0x00, 0x56, 0x00, 0x34, 0x00, 0x29, 0x00,
0x00, 0x00, 0x02, 0x01, 0x0c, 0x00, 0xd0, 0x41,
0x03, 0x0a, 0x00, 0x00, 0x00, 0x00, 0x01, 0x01,
0x06, 0x00, 0x06, 0x1f, 0x03, 0x0b, 0x25, 0x00,
0x2c, 0xea, 0x7f, 0x0a, 0x9f, 0x69, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x03, 0x0c, 0x1b, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x7f, 0xff, 0x04, 0x00,
0x01, 0x04, 0x76, 0x00, 0xef, 0x47, 0x64, 0x2d,
0xc9, 0x3b, 0xa0, 0x41, 0xac, 0x19, 0x4d, 0x51,
0xd0, 0x1b, 0x4c, 0xe6, 0x50, 0x00, 0x58, 0x00,
0x45, 0x00, 0x20, 0x00, 0x49, 0x00, 0x50, 0x00,
0x34, 0x00, 0x20, 0x00, 0x49, 0x00, 0x6e, 0x00,
0x74, 0x00, 0x65, 0x00, 0x6c, 0x00, 0x28, 0x00,
0x52, 0x00, 0x29, 0x00, 0x20, 0x00, 0x45, 0x00,
0x74, 0x00, 0x68, 0x00, 0x65, 0x00, 0x72, 0x00,
0x6e, 0x00, 0x65, 0x00, 0x74, 0x00, 0x20, 0x00,
0x43, 0x00, 0x6f, 0x00, 0x6e, 0x00, 0x6e, 0x00,
0x65, 0x00, 0x63, 0x00, 0x74, 0x00, 0x69, 0x00,
0x6f, 0x00, 0x6e, 0x00, 0x20, 0x00, 0x28, 0x00,
0x36, 0x00, 0x29, 0x00, 0x20, 0x00, 0x49, 0x00,
0x32, 0x00, 0x31, 0x00, 0x39, 0x00, 0x2d, 0x00,
0x4c, 0x00, 0x4d, 0x00, 0x00, 0x00, 0x7f, 0xff,
0x04, 0x00, 0x00, 0x00, 0x42, 0x4f
};
size_t load_option_data_size = sizeof(load_option_data);
char *target_remaining = &load_option_data[load_option_data_size - 2];
size_t target_remaining_size = 2;

return test_parse_load_options(load_option_data,
load_option_data_size,
"test.efi",
NULL,
target_remaining,
target_remaining_size);
}

int
main(void)
{
Expand All @@ -263,6 +333,7 @@ main(void)
test(test_bds_0);
test(test_bds_1);
test(test_bds_2);
test(test_multi_end_dp);
return status;
}

Expand Down

0 comments on commit 2cabe53

Please sign in to comment.