From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id 3BF817803CD for ; Tue, 19 Dec 2023 04:50:31 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=H2bZ2kfsRonfaflr2UEtAETyfaHmM/eiOjH/onIqzOw=; c=relaxed/simple; d=groups.io; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References:MIME-Version:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Transfer-Encoding; s=20140610; t=1702961429; v=1; b=c+tUOqz6MSziEVGCKwwetxk+YoDu/Zce4eepD4BA9cifXoHO02M3XhJRGA2raJqJlfCtWL3/ zzw0bLTgjMmQM9WY7MipjlEPshn0bkLZnyWB8/EpqTXF96SaLG2dwKBLy8BSZKIFBnYsla5LZ3L pBeWbZCoki/LidMZv7fG5/UM= X-Received: by 127.0.0.2 with SMTP id TpRmYY7687511xj4yZggMiM8; Mon, 18 Dec 2023 20:50:29 -0800 X-Received: from mgamail.intel.com (mgamail.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web11.5762.1702961425788074891 for ; Mon, 18 Dec 2023 20:50:29 -0800 X-IronPort-AV: E=McAfee;i="6600,9927,10928"; a="481798137" X-IronPort-AV: E=Sophos;i="6.04,287,1695711600"; d="scan'208";a="481798137" X-Received: from fmsmga008.fm.intel.com ([10.253.24.58]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Dec 2023 20:50:29 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10928"; a="841752586" X-IronPort-AV: E=Sophos;i="6.04,287,1695711600"; d="scan'208";a="841752586" X-Received: from liyi4-desktop.ccr.corp.intel.com ([10.239.153.29]) by fmsmga008-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 18 Dec 2023 20:50:27 -0800 From: "Li, Yi" To: devel@edk2.groups.io Cc: Yi Li , Liming Gao , Michael D Kinney , Wei6 Xu Subject: [edk2-devel] [PATCH 2/2] FmpDevicePkg: Add DECLARE_LENGTH opcode of dependency expression Date: Tue, 19 Dec 2023 12:50:13 +0800 Message-ID: <20231219045013.1425-3-yi1.li@intel.com> In-Reply-To: <20231219045013.1425-1-yi1.li@intel.com> References: <20231219045013.1425-1-yi1.li@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,yi1.li@intel.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-Gm-Message-State: CoAkO631pvguOzL8bAW4W7kax7686176AA= Content-Transfer-Encoding: 8bit X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=c+tUOqz6; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=intel.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io Mantis: https://mantis.uefi.org/mantis/view.php?id=2025 To avoid messy parsing of the Depex section of a Capsule, it would be a lot easier for everyone involved if we preceded the Capsule Depex Section with a length declaration. It provides simple bounds checking to avoid having to parse the op-codes, but in the case of a malformed depex being parsed, avoid other issues which can be messy. REF: UEFI spec 2.10 Table 23.4 Signed-off-by: Yi Li Cc: Liming Gao Cc: Michael D Kinney Cc: Wei6 Xu --- .../FmpDependencyLib/FmpDependencyLib.c | 35 ++++++++ .../PrivateInclude/FmpLastAttemptStatus.h | 3 + .../EvaluateDependencyUnitTest.c | 84 ++++++++++++++++--- 3 files changed, 110 insertions(+), 12 deletions(-) diff --git a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c index 76a1ee3f40db..50662e74e065 100644 --- a/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c +++ b/FmpDevicePkg/Library/FmpDependencyLib/FmpDependencyLib.c @@ -234,6 +234,7 @@ EvaluateDependency ( GUID ImageTypeId; UINT32 Version; UINT32 LocalLastAttemptStatus; + UINT32 DeclaredLength; LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_SUCCESS; @@ -489,6 +490,37 @@ EvaluateDependency ( } return Element1.Value.Boolean; + case EFI_FMP_DEP_DECLARE_LENGTH: + if (Iterator + sizeof (UINT32) >= (UINT8 *)Dependencies->Dependencies + DependenciesSize ) { + DEBUG ((DEBUG_ERROR, "EvaluateDependency: DECLARE_LENGTH extends beyond end of dependency expression!\n")); + LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_DECLARE_LENGTH_BEYOND_DEPEX; + goto Error; + } + + // + // This opcode must be the first one in a dependency expression. + // + if (Iterator != Dependencies->Dependencies) { + DEBUG ((DEBUG_ERROR, "EvaluateDependency: DECLARE_LENGTH is not the first opcode!\n")); + LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_DECLARE_LENGTH_NOT_FIRST_OPCODE; + goto Error; + } + + DeclaredLength = *(UINT32 *)(Iterator + 1); + if (DeclaredLength != DependenciesSize) { + DEBUG ((DEBUG_ERROR, "EvaluateDependency: DECLARE_LENGTH is not equal to length of dependency expression!\n")); + LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_DECLARE_LENGTH_INCORRECT; + goto Error; + } + + Status = Push (DeclaredLength, VersionType); + if (EFI_ERROR (Status)) { + LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_PUSH_FAILURE; + goto Error; + } + + Iterator = Iterator + sizeof (UINT32); + break; default: DEBUG ((DEBUG_ERROR, "EvaluateDependency: Unknown Opcode - %02x!\n", *Iterator)); LocalLastAttemptStatus = LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_UNKNOWN_OPCODE; @@ -574,6 +606,9 @@ ValidateDependency ( } return TRUE; + case EFI_FMP_DEP_DECLARE_LENGTH: + Depex += sizeof (UINT32) + 1; + break; default: return FALSE; } diff --git a/FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h b/FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h index 39a55dd2c643..aaa3334909c8 100644 --- a/FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h +++ b/FmpDevicePkg/PrivateInclude/FmpLastAttemptStatus.h @@ -66,6 +66,9 @@ enum LAST_ATTEMPT_STATUS_EXPANDED_ERROR_LIST { LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_FMP_NOT_FOUND, LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_PUSH_FAILURE, LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_POP_FAILURE, + LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_DECLARE_LENGTH_NOT_FIRST_OPCODE, + LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_DECLARE_LENGTH_BEYOND_DEPEX, + LAST_ATTEMPT_STATUS_DEPENDENCY_LIB_ERROR_DECLARE_LENGTH_INCORRECT, /// /// Last attempt status codes used in FmpDependencyCheckLib diff --git a/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c b/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c index 0edb7f67306f..352887af2c5e 100644 --- a/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c +++ b/FmpDevicePkg/Test/UnitTest/Library/FmpDependencyLib/EvaluateDependencyUnitTest.c @@ -125,19 +125,75 @@ static UINT8 mExpression11[] = { EFI_FMP_DEP_END }; +// Valid Dependency Expression 7: With correct declared length +static UINT8 mExpression12[] = { + EFI_FMP_DEP_DECLARE_LENGTH, 0x35, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_VERSION, 0x01, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0xFA, 0x4D, 0x14, 0x97,0x8E, 0xEB, 0x4D, 0xD1, 0x8B, 0x4D, 0x39, 0x88, 0x24, 0x96, 0x56, 0x42, + EFI_FMP_DEP_GT, + EFI_FMP_DEP_PUSH_VERSION, 0x03, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0x70, 0x73, 0x2A, 0xA4,0x3A, 0x43, 0x4D, 0x68, 0x9A, 0xA1, 0xDE, 0x62, 0x23, 0x30, 0x6C, 0xF3, + EFI_FMP_DEP_GTE, + EFI_FMP_DEP_AND, + EFI_FMP_DEP_END +}; + +// Valid Dependency Expression 7: With longer declared length +static UINT8 mExpression13[] = { + EFI_FMP_DEP_DECLARE_LENGTH, 0x3B, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_VERSION, 0x01, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0xFA, 0x4D, 0x14, 0x97,0x8E, 0xEB, 0x4D, 0xD1, 0x8B, 0x4D, 0x39, 0x88, 0x24, 0x96, 0x56, 0x42, + EFI_FMP_DEP_GT, + EFI_FMP_DEP_PUSH_VERSION, 0x03, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0x70, 0x73, 0x2A, 0xA4,0x3A, 0x43, 0x4D, 0x68, 0x9A, 0xA1, 0xDE, 0x62, 0x23, 0x30, 0x6C, 0xF3, + EFI_FMP_DEP_GTE, + EFI_FMP_DEP_AND, + EFI_FMP_DEP_END +}; + +// Valid Dependency Expression 7: With shorter declared length +static UINT8 mExpression14[] = { + EFI_FMP_DEP_DECLARE_LENGTH, 0x1B, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_VERSION, 0x01, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0xFA, 0x4D, 0x14, 0x97,0x8E, 0xEB, 0x4D, 0xD1, 0x8B, 0x4D, 0x39, 0x88, 0x24, 0x96, 0x56, 0x42, + EFI_FMP_DEP_GT, + EFI_FMP_DEP_PUSH_VERSION, 0x03, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0x70, 0x73, 0x2A, 0xA4,0x3A, 0x43, 0x4D, 0x68, 0x9A, 0xA1, 0xDE, 0x62, 0x23, 0x30, 0x6C, 0xF3, + EFI_FMP_DEP_GTE, + EFI_FMP_DEP_AND, + EFI_FMP_DEP_END +}; + +// Valid Dependency Expression 7: DECLARE_LENGTH opcode is not first one +static UINT8 mExpression15[] = { + EFI_FMP_DEP_PUSH_VERSION, 0x01, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0xFA, 0x4D, 0x14, 0x97,0x8E, 0xEB, 0x4D, 0xD1, 0x8B, 0x4D, 0x39, 0x88, 0x24, 0x96, 0x56, 0x42, + EFI_FMP_DEP_GT, + EFI_FMP_DEP_DECLARE_LENGTH, 0x35, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_VERSION, 0x03, 0x00, 0x00, 0x00, + EFI_FMP_DEP_PUSH_GUID, 0x70, 0x73, 0x2A, 0xA4,0x3A, 0x43, 0x4D, 0x68, 0x9A, 0xA1, 0xDE, 0x62, 0x23, 0x30, 0x6C, 0xF3, + EFI_FMP_DEP_GTE, + EFI_FMP_DEP_AND, + EFI_FMP_DEP_END +}; + // ------------------------------------------------Test Depex------Depex Size----------------Expected Result -static BASIC_TEST_CONTEXT mBasicTestTrue1 = { mExpression1, sizeof (mExpression1), TRUE }; -static BASIC_TEST_CONTEXT mBasicTestTrue2 = { mExpression2, sizeof (mExpression2), TRUE }; -static BASIC_TEST_CONTEXT mBasicTestFalse1 = { mExpression3, sizeof (mExpression3), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestFalse2 = { mExpression4, sizeof (mExpression4), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid1 = { mExpression1, sizeof (mExpression1) - 1, FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid2 = { mExpression5, sizeof (mExpression5), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid3 = { mExpression6, sizeof (mExpression6), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid4 = { mExpression7, sizeof (mExpression7), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid5 = { mExpression8, sizeof (mExpression8), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid6 = { mExpression9, sizeof (mExpression9), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid7 = { mExpression10, sizeof (mExpression10), FALSE }; -static BASIC_TEST_CONTEXT mBasicTestInvalid8 = { mExpression11, sizeof (mExpression11), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestTrue1 = { mExpression1, sizeof (mExpression1), TRUE }; +static BASIC_TEST_CONTEXT mBasicTestTrue2 = { mExpression2, sizeof (mExpression2), TRUE }; +static BASIC_TEST_CONTEXT mBasicTestFalse1 = { mExpression3, sizeof (mExpression3), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestFalse2 = { mExpression4, sizeof (mExpression4), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid1 = { mExpression1, sizeof (mExpression1) - 1, FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid2 = { mExpression5, sizeof (mExpression5), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid3 = { mExpression6, sizeof (mExpression6), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid4 = { mExpression7, sizeof (mExpression7), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid5 = { mExpression8, sizeof (mExpression8), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid6 = { mExpression9, sizeof (mExpression9), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid7 = { mExpression10, sizeof (mExpression10), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid8 = { mExpression11, sizeof (mExpression11), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestValid1 = { mExpression12, sizeof (mExpression12), TRUE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid9 = { mExpression13, sizeof (mExpression13), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid10 = { mExpression14, sizeof (mExpression14), FALSE }; +static BASIC_TEST_CONTEXT mBasicTestInvalid11 = { mExpression15, sizeof (mExpression15), FALSE }; /** Unit test for EvaluateDependency() API of the FmpDependencyLib. @@ -233,6 +289,10 @@ UnitTestingEntry ( AddTestCase (DepexEvalTests, "Error: Operand and operator mismatch", "Test10", EvaluateDependencyTest, NULL, NULL, &mBasicTestInvalid6); AddTestCase (DepexEvalTests, "Error: GUID is NOT FOUND", "Test11", EvaluateDependencyTest, NULL, NULL, &mBasicTestInvalid7); AddTestCase (DepexEvalTests, "Error: Stack Underflow", "Test12", EvaluateDependencyTest, NULL, NULL, &mBasicTestInvalid8); + AddTestCase (DepexEvalTests, "Evaluate to True - 3", "Test13", EvaluateDependencyTest, NULL, NULL, &mBasicTestValid1); + AddTestCase (DepexEvalTests, "Error: Declared length too long", "Test14", EvaluateDependencyTest, NULL, NULL, &mBasicTestInvalid9); + AddTestCase (DepexEvalTests, "Error: Declared length too short", "Test15", EvaluateDependencyTest, NULL, NULL, &mBasicTestInvalid10); + AddTestCase (DepexEvalTests, "Error: DECLARE_LENGTH is not first opcode", "Test16", EvaluateDependencyTest, NULL, NULL, &mBasicTestInvalid11); // // Execute the tests. -- 2.42.0.windows.2 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112688): https://edk2.groups.io/g/devel/message/112688 Mute This Topic: https://groups.io/mt/103257910/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=-=-=-=-=-=-=-=-=-=-=-