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

[Jonathan Gray]

On Fri, Dec 08, 2017 at 06:55:02AM +0100, Alexander Graf wrote:
> 
> 
> 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

Yes, this does fix it.  Thanks.

Your mail client wrapped lines so I had to adjust it to apply.  Including
it again with that fixed:

diff --git a/lib/efi_loader/efi_device_path.c b/lib/efi_loader/efi_device_path.c
index b4e2f933cb..24a4f40c00 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);