efi-boot-shim.git/Cryptlib, branch 15.8

Use -Wno-unused-but-set-variable for Cryptlib and OpenSSL

2023-06-21T19:04:38+00:00

Cryptlib and OpenSSL both currently throw warnings with some compilers using -Wunused-but-set-variable: clang -std=gnu11 -ggdb -ffreestanding -fmacro-prefix-map=/home/pjones/devel/github.com/shim/main/= -fno-stack-protector -fno-strict-aliasing -fpic -fshort-wchar -nostdinc -m64 -mno-mmx -mno-sse -mno-red-zone -Os -Wall -Wextra -Wno-missing-field-initializers -Wno-unused-parameter -Werror -I/home/pjones/devel/github.com/shim/main/Cryptlib -I/home/pjones/devel/github.com/shim/main/Cryptlib/Include -I/home/pjones/devel/github.com/shim/main/gnu-efi/inc -I/home/pjones/devel/github.com/shim/main/gnu-efi/inc/x86_64 -I/home/pjones/devel/github.com/shim/main/gnu-efi/inc/protocol -isystem /home/pjones/devel/github.com/shim/main/include/system -isystem /usr/lib64/clang/16/include -DMDE_CPU_X64 -c -o Pk/CryptX509.o Pk/CryptX509.c Pk/CryptX509.c:94:19: error: variable 'Index' set but not used [-Werror,-Wunused-but-set-variable] UINTN Index; ^ clang -std=gnu11 -ggdb -ffreestanding -fmacro-prefix-map=/home/pjones/devel/github.com/shim/main/= -fno-stack-protector -fno-strict-aliasing -fpic -fshort-wchar -nostdinc -m64 -mno-mmx -mno-sse -mno-red-zone -Os -Wall -Wextra -Wno-missing-field-initializers -Wno-empty-body -Wno-implicit-fallthrough -Wno-unused-parameter -Werror -I/home/pjones/devel/github.com/shim/main/Cryptlib/OpenSSL -I/home/pjones/devel/github.com/shim/main/Cryptlib -I/home/pjones/devel/github.com/shim/main/Cryptlib/OpenSSL/Include/ -I/home/pjones/devel/github.com/shim/main/Cryptlib/OpenSSL/crypto -I/home/pjones/devel/github.com/shim/main/Cryptlib/Include -I/home/pjones/devel/github.com/shim/main/gnu-efi/inc -I/home/pjones/devel/github.com/shim/main/gnu-efi/inc/x86_64 -I/home/pjones/devel/github.com/shim/main/gnu-efi/inc/protocol -I/home/pjones/devel/github.com/shim/main/Cryptlib/OpenSSL/crypto/asn1 -I/home/pjones/devel/github.com/shim/main/Cryptlib/OpenSSL/crypto/evp -I/home/pjones/devel/github.com/shim/main/Cryptlib/OpenSSL/crypto/modes -I/home/pjones/devel/github.com/shim/main/Cryptlib/OpenSSL/crypto/include -isystem /home/pjones/devel/github.com/shim/main/include/system -isystem /usr/lib64/clang/16/include -DL_ENDIAN -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -DOPENSSL_SMALL_FOOTPRINT -DPEDANTIC -DMDE_CPU_X64 -c -o crypto/asn1/t_x509.o crypto/asn1/t_x509.c crypto/asn1/t_x509.c:504:18: error: variable 'l' set but not used [-Werror,-Wunused-but-set-variable] int ret = 0, l, i; ^ Since we normally build with -Werror, these cause builds to fail in these cases. While the bad code should be addressed, it appears generally safe, so we should solve it upstream. This patch adds -Wno-unused-but-set-variable to the Cryptlib Makefile, and removes the conditionalization on gcc in the OpenSSL Makefile, as clang now has this argument, and since we don't support building with clang for the final build, it's useful to have clang-based tools working. Signed-off-by: Peter Jones

CryptoPkg/BaseCryptLib: Fix buffer overflow issue in realloc wrapper

2023-01-27T19:15:14+00:00

There is one long-standing problem in CRT realloc wrapper, which will cause the obvious buffer overflow issue when re-allocating one bigger memory block: void *realloc (void *ptr, size_t size) { // // BUG: hardcode OldSize == size! We have no any knowledge about // memory size of original pointer ptr. // return ReallocatePool ((UINTN) size, (UINTN) size, ptr); } This patch introduces one extra header to record the memory buffer size information when allocating memory block from malloc routine, and re-wrap the realloc() and free() routines to remove this BUG. Cc: Laszlo Ersek Cc: Ting Ye Cc: Jian J Wang Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Qin Long Reviewed-by: Jian J Wang Validated-by: Jian J Wang Cherry picked from https://github.com/tianocore/edk2.git, commit cf8197a39d07179027455421a182598bd6989999. Changes: * `SIGNATURE_32` -> `EFI_SIGNATURE_32` * Added definition of `MIN` Fixes https://github.com/rhboot/shim/issues/538 Signed-off-by: Nicholas Bishop

CryptoPkg/BaseCryptLib: fix NULL dereference

2022-11-15T18:07:29+00:00

AuthenticodeVerify() calls OpenSSLs d2i_PKCS7() API to parse asn encoded signed authenticode pkcs#7 data. when this successfully returns, a type check is done by calling PKCS7_type_is_signed() and then Pkcs7->d.sign->contents->type is used. It is possible to construct an asn1 blob that successfully decodes and have d2i_PKCS7() return a valid pointer and have PKCS7_type_is_signed() also return success but have Pkcs7->d.sign be a NULL pointer. Looking at how PKCS7_verify() [inside of OpenSSL] implements checking for pkcs7 structs it does the following: - call PKCS7_type_is_signed() - call PKCS7_get_detached() Looking into how PKCS7_get_detatched() is implemented, it checks to see if p7->d.sign is NULL or if p7->d.sign->contents->d.ptr is NULL. As such, the fix is to do the same as OpenSSL after calling d2i_PKCS7(). - Add call to PKS7_get_detached() to existing error handling Cc: Chao Zhang Cc: Jiewen Yao Signed-off-by: Jian J Wang Cherry-picked-from: https://github.com/tianocore/edk2/commit/26442d11e620a9e81c019a24a4ff38441c64ba10

Make CopyMem() work with EFI's declaration

2021-09-07T21:05:04+00:00

EFI_BOOT_SERVICES includes CopyMem() and SetMem() functions which are marked EFIAPI, and in the case of CopyMem() does not mark the source argument as CONST. This patch makes all our invocations work with that, so (once gnu-efi's implementation is fixed to match) we can use the existing implementation as the implementation in a mock EFI_BOOT_SERVICES. Signed-off-by: Peter Jones

More va_* work

2021-03-12T09:15:01+00:00

Be much more explicit about exactly which va_* stuff comes from which ABI in both shim and gnu-efi. This fixes the problem where we see: | (null):0:(null)() v->name:"(null)" v->rtname:"(null)" | (null):0:(null)() v->data_size:0 v->data:0x0 and similar messages where everything is NULL. Signed-off-by: Peter Jones

openssl: nerf some -W flags for clang.

2021-03-12T09:15:01+00:00

Signed-off-by: Peter Jones

make: use -Wextra (minus some obnoxious bits)

2021-03-10T20:54:20+00:00

gcc -Wextra, has a lot of good, useful checks, a few obnoxious checks, and a few absolutely insane checks. This enables -Wextra, but disables -Wmissing-field-initializers, because it is irrational nonsense that just leads to worse code. It also disables some specific things in the Cryptlib and Cryptlib/OpenSSL trees: Both: -Wno-unused-parameter - there are a fair number of functions that have to conform to some API or another but have arguments that are unused, but haven't been marked with UNUSED; we don't need to see warnings about them. Cryptlib/OpenSSL: -Wno-empty-body - functions that exist merely to populate some API -Wno-implicit-fallthrough - these probably should get fixed someday, but I bet upstream will do it and rebasing will solve it -Wno-old-style-declaration - this gripes if you write "const static" instead of "static const". Again I expect rebasing will fix it at some point. -Wno-unused-but-set-variable - self explanatory, and again, I expect a rebase to solve it someday. Signed-off-by: Peter Jones

openssl: fix various build errors and warnings

2021-03-10T20:54:20+00:00

There were a couple cases of "uninitialized variable" warnings in the imported OpenSSL code; I used the current OpenSSL code as a guide for picking the default values used here. On my dev system there is one remaining build warning in OpenSSL's crypto/asn1/x_pkey.c:X509_PKEY_new() function. Unfortunately it involves some preprocessor crimes and the fix would be a bit ugly. Fortunately it appears the warning here is harmless and can be ignored. As a point of reference, my build system is a current Arch install with GCC v10.2.0 and GNU-EFI v 3.0.12. Signed-off-by: Paul Moore

Fix stdarg to work the same everywhere.

2021-03-10T20:54:20+00:00

This gets us the same working definition for VA_* va_* etc everywhere, and it's the same definition edk2 is using. Signed-off-by: Peter Jones

Consolidate most of our standard lib functions to lib

2021-03-10T20:54:20+00:00

Signed-off-by: Peter Jones