From f26f86dd8558a1d00479afc416097227c8ac82c7 Mon Sep 17 00:00:00 2001 From: Sean Brogan Date: Tue, 23 Apr 2024 10:00:59 -0700 Subject: [PATCH] Bug: Fix incorrect CodeQL fix - REBASE with PR 6024 (#818) # Description In commit 29987744ef55bd8848f909b4672100f7c7b74da6 an unintentional code change in BmBoot BmExpandFileDevicePath was introduced. The change intended to fix a CodeQL error but introduced a failure with the boot managers ability to expand a "short" file device path boot option. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... ## How This Was Tested Tested on an x64 vm platform ## Integration Instructions n/a --- .../Library/UefiBootManagerLib/BmBoot.c | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c index a3fc3b6a38..54a547f8a5 100644 --- a/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c +++ b/MdeModulePkg/Library/UefiBootManagerLib/BmBoot.c @@ -718,16 +718,21 @@ BmExpandFileDevicePath ( ((MediaType == 2) && (BlockIo == NULL)) ) { + NextFullPath = AppendDevicePath (DevicePathFromHandle (Handles[Index]), FilePath); + if (NextFullPath == NULL) { + continue; + } + if (GetNext) { + // this is the break/exit condition. Occurs on first if FullPath input parameter was NULL + // or on the next loop after input parameter FullPath matches NextFullPath. + // NextFullPath will not be NULL so outer loop is broken too break; } - NextFullPath = AppendDevicePath (DevicePathFromHandle (Handles[Index]), FilePath); - if (NextFullPath != NULL) { - GetNext = (BOOLEAN)(CompareMem (NextFullPath, FullPath, GetDevicePathSize (NextFullPath)) == 0); - FreePool (NextFullPath); - NextFullPath = NULL; - } + GetNext = (BOOLEAN)(CompareMem (NextFullPath, FullPath, GetDevicePathSize (NextFullPath)) == 0); + FreePool (NextFullPath); + NextFullPath = NULL; } }