Differential D34698

efibootmgr: exit loop if device path size is zero
Needs ReviewPublic
Actions

Authored by williams_ct1.xyz on Mar 28 2022, 7:36 PM.

Details

Reviewers

imp

Summary

An infinite loop can occur when running: efibootmgr -v

Diff Detail

Repository

rS FreeBSD src repository - subversion

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 44915
Build 41803: arc lint + arc unit

Event Timeline

williams_ct1.xyz requested review of this revision.Mar 28 2022, 7:36 PM

williams_ct1.xyz created this revision.

Harbormaster completed remote builds in B44908: Diff 104308.Mar 28 2022, 7:36 PM

williams_ct1.xyz retitled this revision from exit loop if device path size is zero to efibootmgr: exit loop if device path size is zero.Mar 28 2022, 7:43 PM

williams_ct1.xyz edited the summary of this revision. (Show Details)Mar 28 2022, 7:49 PM

imp added inline comments.Mar 28 2022, 10:34 PM

usr.sbin/efibootmgr/efibootmgr.c
785	Do we need a if ((uintptr_t)dp+dp_size > (uintptr_t)edp) break; here? We should NEVER see dp_size == 0 here, so I'm curious about when that popped up. On the other hand, it's not a bad thing to be a little paranoid while walking binary blobs we get from a BIOS that may or may not be friendly.

I checked the contained calls, but didn't know enough about the efi. So added a hex dump to verify.

Boot to FW : false
BootCurrent: 0001
Timeout    : 1 seconds
BootOrder  : 0001, 0002, 0003, 0004, 0005, 0006
+Boot0001* UEFI OS (dp_size:005e max:0060) HD(1,GPT,a5e20892-ed10-11ea-87d6-74d02bc504b2,0x28,0x640)/File(\EFI\BOOT\BOOTX64.EFI)
                   (dp_size:0000 max:0002) 
           (INFINITE LOOP)
           0000 01 00 00 00 60 00 55 00 45 00 46 00 49 00 20 00 ....`.U.E.F.I. .
           0010 4f 00 53 00 00 00 04 01 2a 00 01 00 00 00 28 00 O.S.....*.....(.
           0020 00 00 00 00 00 00 40 06 00 00 00 00 00 00 92 08 ......@.........
           0030 e2 a5 10 ed ea 11 87 d6 74 d0 2b c5 04 b2 02 02 ........t.+.....
           0040 04 04 30 00 5c 00 45 00 46 00 49 00 5c 00 42 00 ..0.\.E.F.I.\.B.
           0050 4f 00 4f 00 54 00 5c 00 42 00 4f 00 4f 00 54 00 O.O.T.\.B.O.O.T.
           0060 58 00 36 00 34 00 2e 00 45 00 46 00 49 00 00 00 X.6.4...E.F.I...
           0070 7f ff 04 00 54 00                               ...T.

 Boot0002* UEFI OS (dp_size:0018 max:0078) VenHw(99e275e7-75a0-4b37-a2e6-c5385e6c00cb)
                   (dp_size:005e max:0060) HD(1,GPT,1c839f46-9b87-11e6-9f1c-74d02bc504b2,0x28,0x640)/File(\EFI\BOOT\BOOTX64.EFI)
                   (dp_size:0000 max:0002) 
           (INFINITE LOOP)
           0000 09 00 00 00 78 00 55 00 45 00 46 00 49 00 20 00 ....x.U.E.F.I. .
           0010 4f 00 53 00 00 00 01 04 14 00 e7 75 e2 99 a0 75 O.S........u...u
           0020 37 4b a2 e6 c5 38 5e 6c 00 cb 7f ff 04 00 04 01 7K...8^l.......
           0030 2a 00 01 00 00 00 28 00 00 00 00 00 00 00 40 06 *.....(.......@.
           0040 00 00 00 00 00 00 46 9f 83 1c 87 9b e6 11 9f 1c ......F.........
           0050 74 d0 2b c5 04 b2 02 02 04 04 30 00 5c 00 45 00 t.+.......0.\.E.
           0060 46 00 49 00 5c 00 42 00 4f 00 4f 00 54 00 5c 00 F.I.\.B.O.O.T.\.
           0070 42 00 4f 00 4f 00 54 00 58 00 36 00 34 00 2e 00 B.O.O.T.X.6.4...
           0080 45 00 46 00 49 00 00 00 7f ff 04 00 00 16       E.F.I........

 Boot0003* UEFI OS (dp_size:005e max:0060) HD(1,GPT,a5e20892-ed10-11ea-87d6-74d02bc504b2,0x28,0x960)/File(\EFI\BOOT\BOOTX64.EFI)
                       ada0p1:/EFI/BOOT/BOOTX64.EFI (null)
                   (dp_size:0000 max:0002) 
           (INFINITE LOOP)
           0000 01 00 00 00 60 00 55 00 45 00 46 00 49 00 20 00 ....`.U.E.F.I. .
           0010 4f 00 53 00 00 00 04 01 2a 00 01 00 00 00 28 00 O.S.....*.....(.
           0020 00 00 00 00 00 00 60 09 00 00 00 00 00 00 92 08 ......`.........
           0030 e2 a5 10 ed ea 11 87 d6 74 d0 2b c5 04 b2 02 02 ........t.+.....
           0040 04 04 30 00 5c 00 45 00 46 00 49 00 5c 00 42 00 ..0.\.E.F.I.\.B.
           0050 4f 00 4f 00 54 00 5c 00 42 00 4f 00 4f 00 54 00 O.O.T.\.B.O.O.T.
           0060 58 00 36 00 34 00 2e 00 45 00 46 00 49 00 00 00 X.6.4...E.F.I...
           0070 7f ff 04 00 00 00                               .....

 Boot0004* UEFI:CD/DVD Drive (dp_size:000d max:000d) BBS(0x81,,0x0)
           0000 01 00 00 00 0d 00 55 00 45 00 46 00 49 00 3a 00 ......U.E.F.I.:.
           0010 43 00 44 00 2f 00 44 00 56 00 44 00 20 00 44 00 C.D./.D.V.D. .D.
           0020 72 00 69 00 76 00 65 00 00 00 05 01 09 00 81 00 r.i.v.e.........
           0030 00 00 00 7f ff 04 00                            ......

 Boot0005* UEFI:Removable Device (dp_size:000d max:000d) BBS(0x82,,0x0)
           0000 01 00 00 00 0d 00 55 00 45 00 46 00 49 00 3a 00 ......U.E.F.I.:.
           0010 52 00 65 00 6d 00 6f 00 76 00 61 00 62 00 6c 00 R.e.m.o.v.a.b.l.
           0020 65 00 20 00 44 00 65 00 76 00 69 00 63 00 65 00 e. .D.e.v.i.c.e.
           0030 00 00 05 01 09 00 82 00 00 00 00 7f ff 04 00    ..............

 Boot0006* UEFI:Network Device (dp_size:000d max:000d) BBS(0x83,,0x0)
           0000 01 00 00 00 0d 00 55 00 45 00 46 00 49 00 3a 00 ......U.E.F.I.:.
           0010 4e 00 65 00 74 00 77 00 6f 00 72 00 6b 00 20 00 N.e.t.w.o.r.k. .
           0020 44 00 65 00 76 00 69 00 63 00 65 00 00 00 05 01 D.e.v.i.c.e.....
           0030 09 00 83 00 00 00 00 7f ff 04 00                ..........

(max is the intptr_t edp - dp)

efidp_format_device_path and efi_device_path_to_unix_path seem sane, but defensively the following could also be added:

Variant A

		dp_max_size = (intptr_t)edp - (intptr_t)dp;
		dp_size = efidp_size(dp);
		if (dp_size == 0)
			break;
		if (dp_size > dp_max_size)
			break; // error message suggestion???
		efidp_format_device_path(buf, sizeof(buf), dp, dp_max_size);

or Variant B

		dp_max_size = (intptr_t)edp - (intptr_t)dp;
		dp_size = efidp_size(dp);
		if (dp_size == 0 || dp_size > dp_max_size)
			break; // maybe check for the 0x7fff, error msg??
		efidp_format_device_path(buf, sizeof(buf), dp, dp_max_size);