Hello community, here is the log from the commit of package shim for openSUSE:Factory checked in at 2018-04-17 11:10:28 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Comparing /work/SRC/openSUSE:Factory/shim (Old) and /work/SRC/openSUSE:Factory/.shim.new (New) ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
Package is "shim" Tue Apr 17 11:10:28 2018 rev:67 rq:595022 version:14 Changes: -------- --- /work/SRC/openSUSE:Factory/shim/shim.changes 2018-04-04 11:03:17.936649114 +0200 +++ /work/SRC/openSUSE:Factory/.shim.new/shim.changes 2018-04-17 11:10:32.514039687 +0200 @@ -1,0 +2,8 @@ +Tue Apr 10 03:45:39 UTC 2018 - [email protected] + +- Add shim-bsc1088585-handle-mok-allocations-better.patch to avoid + double-freeing after enrolling a key from the disk (bsc#1088585) + + Also refresh shim-opensuse-cert-prompt.patch due to the change + in MokManager.c + +------------------------------------------------------------------- New: ---- shim-bsc1088585-handle-mok-allocations-better.patch ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ Other differences: ------------------ ++++++ shim.spec ++++++ --- /var/tmp/diff_new_pack.QpIsnx/_old 2018-04-17 11:10:33.925973446 +0200 +++ /var/tmp/diff_new_pack.QpIsnx/_new 2018-04-17 11:10:33.929973258 +0200 @@ -54,6 +54,8 @@ Patch4: shim-remove-cryptpem.patch # PATCH-FIX-UPSTREAM shim-httpboot-amend-device-path.patch bsc#1065370 [email protected] -- Amend the device path matching rule for httpboot Patch5: shim-httpboot-amend-device-path.patch +# PATCH-FIX-UPSTREAM shim-bsc1088585-handle-mok-allocations-better.patch bsc#1088585 [email protected] -- Handle the mok parameter allocations better +Patch6: shim-bsc1088585-handle-mok-allocations-better.patch # PATCH-FIX-OPENSUSE shim-change-debug-file-path.patch [email protected] -- Change the default debug file path Patch50: shim-change-debug-file-path.patch # PATCH-FIX-OPENSUSE shim-opensuse-cert-prompt.patch [email protected] -- Show the prompt to ask whether the user trusts openSUSE certificate or not @@ -103,6 +105,7 @@ %patch3 -p1 %patch4 -p1 %patch5 -p1 +%patch6 -p1 %patch50 -p1 %if 0%{?is_opensuse} == 1 %patch100 -p1 ++++++ shim-bsc1088585-handle-mok-allocations-better.patch ++++++ >From c232e8577b0608664fd4ce7a6b24b8ed7d2fc7a4 Mon Sep 17 00:00:00 2001 From: Peter Jones <[email protected]> Date: Wed, 27 Sep 2017 14:17:20 -0400 Subject: [PATCH] MokManager: handle mok parameter allocations better. Covscan daftly claims: 288. var_compare_op: Comparing MokSB to null implies that MokSB might be null. 2330 if (MokSB) { 2331 menu_strings[i] = L"Change Secure Boot state"; 2332 menu_item[i] = MOK_CHANGE_SB; 2333 i++; 2334 } 2335 ... 2358 choice = console_select(perform_mok_mgmt, menu_strings, 0); 2359 if (choice < 0) 2360 goto out; ... 2362 switch (menu_item[choice]) { ... 2395 case MOK_CHANGE_SB: CID 182841 (#1 of 1): Dereference after null check (FORWARD_NULL)293. var_deref_model: Passing null pointer MokSB to mok_sb_prompt, which dereferences it. [show details] 2396 efi_status = mok_sb_prompt(MokSB, MokSBSize); Which is, of course, entirely false, beause for menu_item[choice] to be MOK_CHANGE_SB, MokSB must be !NULL. And then: 252. Condition efi_status == 0, taking true branch. 2397 if (efi_status == EFI_SUCCESS) 2398 MokSB = NULL; This guarantees it won't be in the list the next time through the loop. This adds tests for NULLness before mok_sb_prompt(), just to make it more clear to covscan what's going on. Also do the same thing for all of: MOK_CHANGE_SB MOK_SET_PW MOK_CHANGE_DB MOK_ENROLL_MOKX MOK_DELETE_MOKX I also Lindent-ed everything I had to touch. Three other minor errors are also fixed: 1) the loop in enter_mok_menu() leaked the menu allocations each time through the loop 2) mok_sb_prompt(), mok_pw_prompt(), and mok_db_prompt() all call FreePool() on their respective variables (MokSB, etc), and check_mok_request() also calls FreePool() on these. This sounds horrible, but it turns out it's not an issue, because they only free them in their EFI_SUCCESS paths, and enter_mok_menu() resets the system if any of the mok_XX_prompt() calls actually returned EFI_SUCCESS, so we never get back to check_mok_request() for it to do its FreePool() calls. 3) the loop in enter_mok_menu() winds up introducing a double free in the call to free_menu(), but we also can't hit this bug, because all the exit paths from the loop are "goto out" (or return error) rather than actually exiting on the loop conditional. Signed-off-by: Peter Jones <[email protected]> (cherry picked from commit a32651360552559ee6a8978b5bcdc6e7dcc72b8c) Gary Lin: Fixed the conflict against shim 14. --- MokManager.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/MokManager.c b/MokManager.c index 55af321..42bf72d 100644 --- a/MokManager.c +++ b/MokManager.c @@ -1060,9 +1060,6 @@ static EFI_STATUS mok_enrollment_prompt (void *MokNew, UINTN MokNewSize, int aut } } - if (MokNew) - FreePool (MokNew); - return EFI_SUCCESS; } @@ -1609,9 +1606,6 @@ static EFI_STATUS mok_sb_prompt (void *MokSB, UINTN MokSBSize) { } } - if (MokSB) - FreePool(MokSB); - return EFI_SUCCESS; } @@ -1729,9 +1723,6 @@ static EFI_STATUS mok_db_prompt (void *MokDB, UINTN MokDBSize) { } } - if (MokDB) - FreePool(MokDB); - return EFI_SUCCESS; } @@ -1800,9 +1791,6 @@ static EFI_STATUS mok_pw_prompt (void *MokPW, UINTN MokPWSize) { mokpw_done: LibDeleteVariable(L"MokPW", &shim_lock_guid); - if (MokPW) - FreePool(MokPW); - return EFI_SUCCESS; } @@ -2184,8 +2172,8 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokXNew, UINTN MokXNewSize, void *MokXDel, UINTN MokXDelSize) { - CHAR16 **menu_strings; - mok_menu_item *menu_item; + CHAR16 **menu_strings = NULL; + mok_menu_item *menu_item = NULL; int choice = 0; int mok_changed = 0; EFI_STATUS efi_status; @@ -2357,11 +2345,23 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, efi_status = mok_reset_prompt(FALSE); break; case MOK_ENROLL_MOK: + if (!MokNew) { + Print(L"MokManager: internal error: %s", + L"MokNew was !NULL but is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } efi_status = mok_enrollment_prompt(MokNew, MokNewSize, TRUE, FALSE); if (efi_status == EFI_SUCCESS) MokNew = NULL; break; case MOK_DELETE_MOK: + if (!MokDel) { + Print(L"MokManager: internal error: %s", + L"MokDel was !NULL but is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } efi_status = mok_deletion_prompt(MokDel, MokDelSize, FALSE); if (efi_status == EFI_SUCCESS) MokDel = NULL; @@ -2370,26 +2370,56 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, efi_status = mok_reset_prompt(TRUE); break; case MOK_ENROLL_MOKX: + if (!MokXNew) { + Print(L"MokManager: internal error: %s", + L"MokXNew was !NULL but is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } efi_status = mok_enrollment_prompt(MokXNew, MokXNewSize, TRUE, TRUE); if (efi_status == EFI_SUCCESS) MokXNew = NULL; break; case MOK_DELETE_MOKX: + if (!MokXDel) { + Print(L"MokManager: internal error: %s", + L"MokXDel was !NULL but is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } efi_status = mok_deletion_prompt(MokXDel, MokXDelSize, TRUE); if (efi_status == EFI_SUCCESS) MokXDel = NULL; break; case MOK_CHANGE_SB: + if (!MokSB) { + Print(L"MokManager: internal error: %s", + L"MokSB was !NULL but is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } efi_status = mok_sb_prompt(MokSB, MokSBSize); if (efi_status == EFI_SUCCESS) MokSB = NULL; break; case MOK_SET_PW: + if (!MokPW) { + Print(L"MokManager: internal error: %s", + L"MokPW was !NULL but is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } efi_status = mok_pw_prompt(MokPW, MokPWSize); if (efi_status == EFI_SUCCESS) MokPW = NULL; break; case MOK_CHANGE_DB: + if (!MokDB) { + Print(L"MokManager: internal error: %s", + L"MokDB was !NULL but is now NULL\n"); + ret = EFI_ABORTED; + goto out; + } efi_status = mok_db_prompt(MokDB, MokDBSize); if (efi_status == EFI_SUCCESS) MokDB = NULL; @@ -2406,6 +2436,8 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, mok_changed = 1; free_menu(menu_item, menu_strings); + menu_item = NULL; + menu_strings = NULL; } out: -- 2.16.2 ++++++ shim-opensuse-cert-prompt.patch ++++++ --- /var/tmp/diff_new_pack.QpIsnx/_old 2018-04-17 11:10:34.065966878 +0200 +++ /var/tmp/diff_new_pack.QpIsnx/_new 2018-04-17 11:10:34.073966503 +0200 @@ -1,4 +1,4 @@ -From 7472a6ee1f01466df1a1de65de669ed0c20b12c4 Mon Sep 17 00:00:00 2001 +From aab03ce2522a3610ecfd5e2f9e896a1ccdd5a94a Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin <[email protected]> Date: Tue, 18 Feb 2014 17:29:19 +0800 Subject: [PATCH 1/3] Show the build-in certificate prompt @@ -137,10 +137,10 @@ if (EFI_ERROR(efi_status)) { Print(L"Something has gone seriously wrong: %r\n", efi_status); -- -2.15.1 +2.16.2 -From 3e3cf4589edf350c8c33d0f5069c6868c2810b80 Mon Sep 17 00:00:00 2001 +From d377f58aadd8c5579a922ef3c237d3ed25bb6d00 Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin <[email protected]> Date: Thu, 20 Feb 2014 16:57:08 +0800 Subject: [PATCH 2/3] Support revoking the openSUSE cert @@ -156,10 +156,10 @@ 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/MokManager.c b/MokManager.c -index 55af321..678a9d9 100644 +index 42bf72d..7a2b5fe 100644 --- a/MokManager.c +++ b/MokManager.c -@@ -1806,6 +1806,33 @@ mokpw_done: +@@ -1794,6 +1794,33 @@ mokpw_done: return EFI_SUCCESS; } @@ -193,7 +193,7 @@ static BOOLEAN verify_certificate(UINT8 *cert, UINTN size) { X509 *X509Cert; -@@ -2162,6 +2189,7 @@ typedef enum { +@@ -2150,6 +2177,7 @@ typedef enum { MOK_CHANGE_SB, MOK_SET_PW, MOK_CHANGE_DB, @@ -201,7 +201,7 @@ MOK_KEY_ENROLL, MOK_HASH_ENROLL } mok_menu_item; -@@ -2182,7 +2210,8 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, +@@ -2170,7 +2198,8 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, void *MokPW, UINTN MokPWSize, void *MokDB, UINTN MokDBSize, void *MokXNew, UINTN MokXNewSize, @@ -209,9 +209,9 @@ + void *MokXDel, UINTN MokXDelSize, + void *ClearVerify, UINTN ClearVerifySize) { - CHAR16 **menu_strings; - mok_menu_item *menu_item; -@@ -2262,6 +2291,9 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, + CHAR16 **menu_strings = NULL; + mok_menu_item *menu_item = NULL; +@@ -2250,6 +2279,9 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, if (MokDB) menucount++; @@ -221,7 +221,7 @@ menu_strings = AllocateZeroPool(sizeof(CHAR16 *) * (menucount + 1)); if (!menu_strings) -@@ -2334,6 +2366,12 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, +@@ -2322,6 +2354,12 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, i++; } @@ -234,7 +234,7 @@ menu_strings[i] = L"Enroll key from disk"; menu_item[i] = MOK_KEY_ENROLL; i++; -@@ -2394,6 +2432,9 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, +@@ -2424,6 +2462,9 @@ static EFI_STATUS enter_mok_menu(EFI_HANDLE image_handle, if (efi_status == EFI_SUCCESS) MokDB = NULL; break; @@ -244,7 +244,7 @@ case MOK_KEY_ENROLL: efi_status = mok_key_enroll(); break; -@@ -2424,6 +2465,7 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) +@@ -2456,6 +2497,7 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) EFI_GUID shim_lock_guid = SHIM_LOCK_GUID; UINTN MokNewSize = 0, MokDelSize = 0, MokSBSize = 0, MokPWSize = 0; UINTN MokDBSize = 0, MokXNewSize = 0, MokXDelSize = 0; @@ -252,7 +252,7 @@ void *MokNew = NULL; void *MokDel = NULL; void *MokSB = NULL; -@@ -2431,6 +2473,7 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) +@@ -2463,6 +2505,7 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) void *MokDB = NULL; void *MokXNew = NULL; void *MokXDel = NULL; @@ -260,7 +260,7 @@ EFI_STATUS status; status = get_variable(L"MokNew", (UINT8 **)&MokNew, &MokNewSize, -@@ -2503,9 +2546,20 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) +@@ -2535,9 +2578,20 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) console_error(L"Could not retrieve MokXDel", status); } @@ -282,7 +282,7 @@ if (MokNew) FreePool (MokNew); -@@ -2528,6 +2582,9 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) +@@ -2560,6 +2614,9 @@ static EFI_STATUS check_mok_request(EFI_HANDLE image_handle) if (MokXDel) FreePool (MokXDel); @@ -306,10 +306,10 @@ if (efi_status != EFI_SUCCESS) { -- -2.15.1 +2.16.2 -From b5348293dd95c6627f8fde0344650e006acc181b Mon Sep 17 00:00:00 2001 +From 5a60e36a5c2bad616bc842d7ffaa6acc1493650f Mon Sep 17 00:00:00 2001 From: Gary Ching-Pang Lin <[email protected]> Date: Fri, 7 Mar 2014 16:17:20 +0800 Subject: [PATCH 3/3] Delete openSUSE_Verify the right way @@ -322,10 +322,10 @@ 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/MokManager.c b/MokManager.c -index 678a9d9..c3f8f45 100644 +index 7a2b5fe..feae113 100644 --- a/MokManager.c +++ b/MokManager.c -@@ -1820,7 +1820,10 @@ static INTN mok_clear_verify_prompt(void *ClearVerify, UINTN ClearVerifySize) { +@@ -1808,7 +1808,10 @@ static INTN mok_clear_verify_prompt(void *ClearVerify, UINTN ClearVerifySize) { if (status != EFI_SUCCESS) return -1; @@ -338,5 +338,5 @@ console_error(L"Failed to delete openSUSE_Verify", status); return -1; -- -2.15.1 +2.16.2
