[U-Boot] [PATCH] efi_loader: Fix partition offsets

[Alexander Graf]

On 07.12.17 12:45, Jonathan Gray wrote:
> On Thu, Dec 07, 2017 at 11:57:43AM +0100, Heinrich Schuchardt wrote:
>> On 12/07/2017 08:00 AM, Jonathan Gray wrote:
>>> On Fri, Dec 01, 2017 at 04:10:33PM +0100, Alexander Graf wrote:
>>>> Commit 884bcf6f65 (efi_loader: use proper device-paths for partitions) tried
>>>> to introduce the el torito scheme to all partition table types: Spawn
>>>> individual disk objects for each partition on a disk.
>>>>
>>>> Unfortunately, that code ended up creating partitions with offset=0 which meant
>>>> that anyone accessing these objects gets data from the raw block device instead
>>>> of the partition.
>>>>
>>>> Furthermore, all the el torito logic to spawn devices for partitions was
>>>> duplicated. So let's merge the two code paths and give partition disk objects
>>>> good offsets to work from, so that payloads can actually make use of them.
>>>>
>>>> Fixes: 884bcf6f65 (efi_loader: use proper device-paths for partitions)
>>>> Reported-by: Yousaf Kaukab <yousaf.kaukab at suse.com>
>>>> Signed-off-by: Alexander Graf <agraf at suse.de>
>>>
>>> This once again broke being able to find a DEVICE_PATH_TYPE_MEDIA_DEVICE
>>> node with the loaded image protocol on rpi_3 with mmc/usb.
>>
>> Could you, please, specify which software you are trying to call:
>> Linux EFI stub, Grub, or anything else?
> 
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/arm64/BOOTAA64.EFI
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/arm64/stand/efiboot/
> Disk image with fat/ffs filesystems
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/arm64/miniroot62.fs
> 
> though it would likely show up on other archs as well
> 
> armv7 equivalents of the above
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/armv7/BOOTARM.EFI
> https://cvsweb.openbsd.org/cgi-bin/cvsweb/src/sys/arch/armv7/stand/efiboot/
> https://fastly.cdn.openbsd.org/pub/OpenBSD/snapshots/armv7/miniroot-am335x-62.fs
> 
>>
>> Which patches did you consider?
>> Did you apply these patch series that are not yet in efi-next?
>> efi_loader: correct media device paths
>> efi_loader: avoid use after free
> 
> just master
> c8e1ca3ebfd21915f6f2e399c9ca1cd3d7a4b076 tools: omapimage: fix corner-case in byteswap path
> 
> with a small patch to force calling gnu sed for some non-portable
> sed use in check-config.sh
> 
> 'efi_loader: avoid use after free' doesn't help
> 'efi_loader: correct media device paths' doesn't either

As a quick heads-up: The device path matching is broken. The patch below
should fix it, but I want to create a travis-ci case around that first
and also wrap it up more nicely.

Alex


diff --git a/lib/efi_loader/efi_device_path.c
b/lib/efi_loader/efi_device_path.c
index b4e2f933cb..d12a38c450 100644
--- a/lib/efi_loader/efi_device_path.c
+++ b/lib/efi_loader/efi_device_path.c
@@ -126,6 +126,7 @@ static struct efi_object *find_obj(struct
efi_device_path *dp, bool short_path,
 				   struct efi_device_path **rem)
 {
 	struct efi_object *efiobj;
+	unsigned int dp_size = efi_dp_size(dp);

 	list_for_each_entry(efiobj, &efi_obj_list, link) {
 		struct efi_handler *handler;
@@ -141,10 +142,18 @@ static struct efi_object *find_obj(struct
efi_device_path *dp, bool short_path,
 		do {
 			if (efi_dp_match(dp, obj_dp) == 0) {
 				if (rem) {
+					/*
+					 * Allow partial matches, but inform
+					 * the caller.
+					 */
 					*rem = ((void *)dp) +
 						efi_dp_size(obj_dp);
+					return efiobj;
+				} else {
+					/* Only return on exact matches */
+					if (efi_dp_size(obj_dp) == dp_size)
+						return efiobj;
 				}
-				return efiobj;
 			}

 			obj_dp = shorten_path(efi_dp_next(obj_dp));
@@ -164,8 +173,14 @@ struct efi_object *efi_dp_find_obj(struct
efi_device_path *dp,
 {
 	struct efi_object *efiobj;

-	efiobj = find_obj(dp, false, rem);
+	/* Search for an exact match first */
+	efiobj = find_obj(dp, false, NULL);

+	/* Then for a fuzzy match */
+	if (!efiobj)
+		efiobj = find_obj(dp, false, rem);
+
+	/* And now for a fuzzy short match */
 	if (!efiobj)
 		efiobj = find_obj(dp, true, rem);