From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=PIug4oP2; spf=pass (domain: linaro.org, ip: 209.85.128.52, mailfrom: ard.biesheuvel@linaro.org) Received: from mail-wm1-f52.google.com (mail-wm1-f52.google.com [209.85.128.52]) by groups.io with SMTP; Tue, 03 Sep 2019 09:51:51 -0700 Received: by mail-wm1-f52.google.com with SMTP id n2so251660wmk.4 for ; Tue, 03 Sep 2019 09:51:51 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=Jg9tizxEz7K/ZD6KHa/0d08Inq6DKcyugG+2n/NCjDs=; b=PIug4oP2wyzj5VILfo5XPTbwz4/72rwrsKt5WP7imwJup0gb5dNCFP01UVuk7J5DUO u3Q8ja9OXzcYivuqE6bKVZzKRxmyh8FeLQvAR1XwfDNwCSbGV1ysm6/025kQ7MzQHIcR cQJIwMK0FRoCOOlckiKsHGiWsFe30cp3yE44DCZ3YZ8Q4ojH+fzb0leg5WmixZaWAins isluEm83Ek6FVBb6oswc3Z4G+euKPJgcLYlWGRHQDwHjqCyjytHtUANuO+yhUu5tWlij FgfWdvgyJuj4kzTW2VYd2/RlukAwFB7WiV+vkZowdGJTb1iVe855UZCrqxBmXhXhNO7c g6jA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=Jg9tizxEz7K/ZD6KHa/0d08Inq6DKcyugG+2n/NCjDs=; b=M7CzofDUSrkap9+VeEIkbfNpXn1MlyOCbDtyqx6gyuhX0+r8iyT/6kfmmKJnjx/4Hg fPQeOr4SyecC7uUeZdRsByuL76YU7dB9TU35eVRjRgCtkkCcHqtqxNreNdJxCIak4rG9 9mVaYXZgoTp21CoPgqsLWp3nNek96k2fDB/vFy01wjYIBhZaLjh/jIPP0GIMYg1oS3k3 mAyfMSn5igWoLDm8PQsAs5qWaljBcCTUvd58+wLG1vwSR2jJXqloUOD6BI7uanrQjb/G KpqWMOGMXNxatqGDfbKLuKlku+2RT+9hXLDPVPQ0mFyOq4nWEha8d2Fun8DZvhxTxLhE ytDg== X-Gm-Message-State: APjAAAWPrjvpMsKwJNeL86e5Np2au+sJNuBz6tGEhRztW40EIfVvEdRs ts5Gbv1pQ0ryzs5doWaxWnTsV3vRci1yCuIkqdWscgFBluBoRQ== X-Google-Smtp-Source: APXvYqwAyVvVghB0tJKzYK2mpNYD0ZLvQkfyhtadbGZT/n4HbbMtYgQVQXVp3fyseYmKRegMaBf9Opa6rUgg47KcqKw= X-Received: by 2002:a05:600c:491:: with SMTP id d17mr256067wme.61.1567529509719; Tue, 03 Sep 2019 09:51:49 -0700 (PDT) MIME-Version: 1.0 References: <20190903163801.28652-1-lersek@redhat.com> In-Reply-To: <20190903163801.28652-1-lersek@redhat.com> From: "Ard Biesheuvel" Date: Tue, 3 Sep 2019 09:51:38 -0700 Message-ID: Subject: Re: [edk2-devel] [PATCH] ArmVirtPkg/PlatformBootManagerLib: unload image on EFI_SECURITY_VIOLATION To: edk2-devel-groups-io , Laszlo Ersek Cc: Dandan Bi , Leif Lindholm Content-Type: text/plain; charset="UTF-8" On Tue, 3 Sep 2019 at 09:38, Laszlo Ersek wrote: > > The LoadImage() boot service is a bit unusual in that it allocates > resources in a particular failure case; namely, it produces a valid > "ImageHandle" when it returns EFI_SECURITY_VIOLATION. This is supposed to > happen e.g. when Secure Boot verification fails for the image, but the > platform policy for the particular image origin (such as "fixed media" or > "removable media") is DEFER_EXECUTE_ON_SECURITY_VIOLATION. The return code > allows platform logic to selectively override the verification failure, > and launch the image nonetheless. > > ArmVirtPkg/PlatformBootManagerLib does not override EFI_SECURITY_VIOLATION > for the kernel image loaded from fw_cfg -- any LoadImage() error is > considered fatal. When we simply treat EFI_SECURITY_VIOLATION like any > other LoadImage() error, we leak the resources associated with > "KernelImageHandle". From a resource usage perspective, > EFI_SECURITY_VIOLATION must be considered "success", and rolled back. > > Implement this rollback, without breaking the proper "nesting" of error > handling jumps and labels. > > Cc: Ard Biesheuvel > Cc: Dandan Bi > Cc: Leif Lindholm > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=1992 > Fixes: 23d04b58e27b382bbd3f9b16ba9adb1cb203dad5 > Signed-off-by: Laszlo Ersek Acked-by: Ard Biesheuvel > --- > > Notes: > Repo: https://github.com/lersek/edk2.git > Branch: ldimg_armvirt_bz1992 > > ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c > index 3cc83e3b7b95..d3851fd75fa5 100644 > --- a/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c > +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/QemuKernel.c > @@ -968,53 +968,60 @@ TryRunningQemuKernel ( > > // > // Create a device path for the kernel image to be loaded from that will call > // back into our file system. > // > KernelDevicePath = FileDevicePath (FileSystemHandle, KernelBlob->Name); > if (KernelDevicePath == NULL) { > DEBUG ((EFI_D_ERROR, "%a: failed to allocate kernel device path\n", > __FUNCTION__)); > Status = EFI_OUT_OF_RESOURCES; > goto UninstallProtocols; > } > > // > // Load the image. This should call back into our file system. > // > Status = gBS->LoadImage ( > FALSE, // BootPolicy: exact match required > gImageHandle, // ParentImageHandle > KernelDevicePath, > NULL, // SourceBuffer > 0, // SourceSize > &KernelImageHandle > ); > if (EFI_ERROR (Status)) { > DEBUG ((EFI_D_ERROR, "%a: LoadImage(): %r\n", __FUNCTION__, Status)); > - goto FreeKernelDevicePath; > + if (Status != EFI_SECURITY_VIOLATION) { > + goto FreeKernelDevicePath; > + } > + // > + // From the resource allocation perspective, EFI_SECURITY_VIOLATION means > + // "success", so we must roll back the image loading. > + // > + goto UnloadKernelImage; > } > > // > // Construct the kernel command line. > // > Status = gBS->OpenProtocol ( > KernelImageHandle, > &gEfiLoadedImageProtocolGuid, > (VOID **)&KernelLoadedImage, > gImageHandle, // AgentHandle > NULL, // ControllerHandle > EFI_OPEN_PROTOCOL_GET_PROTOCOL > ); > ASSERT_EFI_ERROR (Status); > > if (CommandLineBlob->Data == NULL) { > KernelLoadedImage->LoadOptionsSize = 0; > } else { > // > // Verify NUL-termination of the command line. > // > if (CommandLineBlob->Data[CommandLineBlob->Size - 1] != '\0') { > DEBUG ((EFI_D_ERROR, "%a: kernel command line is not NUL-terminated\n", > __FUNCTION__)); > Status = EFI_PROTOCOL_ERROR; > goto UnloadKernelImage; > -- > 2.19.1.3.g30247aa5d201 > > > ------------ > Groups.io Links: You receive all messages sent to this group. > > View/Reply Online (#46710): https://edk2.groups.io/g/devel/message/46710 > Mute This Topic: https://groups.io/mt/33128626/1761188 > Group Owner: devel+owner@edk2.groups.io > Unsubscribe: https://edk2.groups.io/g/devel/unsub [ard.biesheuvel@linaro.org] > ------------ >