Use -Wno-unused-but-set-variable for Cryptlib and OpenSSL #576
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 <[email protected]>
vathpela
added a commit
that referenced
this pull request
Jan 23, 2024
What's changed * Various CVE fixes: CVE-2023-40546 mok: fix LogError() invocation CVE-2023-40547 - avoid incorrectly trusting HTTP headers CVE-2023-40548 Fix integer overflow on SBAT section size on 32-bit system CVE-2023-40549 Authenticode: verify that the signature header is in bounds. CVE-2023-40550 pe: Fix an out-of-bound read in verify_buffer_sbat() CVE-2023-40551: pe-relocate: Fix bounds check for MZ binaries * Add make infrastructure to set the NX_COMPAT flag by @vathpela in #530 * Make sbat_var.S parse right with buggy gcc/binutils by @vathpela in #535 * Drop invalid calls to CRYPTO_set_mem_functions by @nicholasbishop in #537 * pe: Align section size up to page size for mem attrs by @nicholasbishop in #539 * test-sbat: Fix exit code by @vathpela in #540 * pe: Add IS_PAGE_ALIGNED macro by @nicholasbishop in #541 * CryptoPkg/BaseCryptLib: Fix buffer overflow issue in realloc wrapper by @nicholasbishop in #546 * Don't loop forever in load_certs() with buggy firmware by @rmetrich in #547 * Block Debian grub binaries with SBAT < 4 by @steve-mcintyre in #550 * Shim unable to locate grubx64 in PXE boot mode when grubx64 is stored in a different file path by @Alberto-Perez-Guevara in #551 * Further improve load_certs() for non-compliant drivers/firmwares by @pbatard in #560 * pe: only process RelocDir->Size of reloc section by @mikebeaton in #562 * Rename 'msecs' to 'usecs' to avoid potential confusion by @aronowski in #563 * Optionally allow to keep shim protocol installed by @bluca in #565 * SBAT-related documents formatting and spelling by @aronowski in #566 * Add SbatLevel_Variable.txt to document the various revocations by @jsetje in #569 * Add a security contact email address in README.md by @vathpela in #572 * Use -Wno-unused-but-set-variable for Cryptlib and OpenSSL by @vathpela in #576 * mok: fix LogError() invocation by @vathpela in #577 * Minor housekeeping by @vathpela in #578 * Test ImageAddress() by @vathpela in #579 * FreePages() is used to return memory allocated by AllocatePages() by @dennis-tseng99 in #580 * Size should minus 1 when calculating 'RelocBaseEnd' by @jsetje in #581 * Verify signature before verifying sbat levels by @jsetje in #583 * Add libFuzzer support for csv.c and sbat.c by @vathpela in #584 * mok: Avoid underflow in maximum variable size calculation by @alpernebbi in #587 * Housekeeping by @vathpela in #605 Signed-off-by: Peter Jones <[email protected]>
brianredbeard
pushed a commit
to brianredbeard/redhat-efi-boot-shim
that referenced
this pull request
Feb 22, 2024
What's changed * Various CVE fixes: CVE-2023-40546 mok: fix LogError() invocation CVE-2023-40547 - avoid incorrectly trusting HTTP headers CVE-2023-40548 Fix integer overflow on SBAT section size on 32-bit system CVE-2023-40549 Authenticode: verify that the signature header is in bounds. CVE-2023-40550 pe: Fix an out-of-bound read in verify_buffer_sbat() CVE-2023-40551: pe-relocate: Fix bounds check for MZ binaries * Add make infrastructure to set the NX_COMPAT flag by @vathpela in rhboot#530 * Make sbat_var.S parse right with buggy gcc/binutils by @vathpela in rhboot#535 * Drop invalid calls to CRYPTO_set_mem_functions by @nicholasbishop in rhboot#537 * pe: Align section size up to page size for mem attrs by @nicholasbishop in rhboot#539 * test-sbat: Fix exit code by @vathpela in rhboot#540 * pe: Add IS_PAGE_ALIGNED macro by @nicholasbishop in rhboot#541 * CryptoPkg/BaseCryptLib: Fix buffer overflow issue in realloc wrapper by @nicholasbishop in rhboot#546 * Don't loop forever in load_certs() with buggy firmware by @rmetrich in rhboot#547 * Block Debian grub binaries with SBAT < 4 by @steve-mcintyre in rhboot#550 * Shim unable to locate grubx64 in PXE boot mode when grubx64 is stored in a different file path by @Alberto-Perez-Guevara in rhboot#551 * Further improve load_certs() for non-compliant drivers/firmwares by @pbatard in rhboot#560 * pe: only process RelocDir->Size of reloc section by @mikebeaton in rhboot#562 * Rename 'msecs' to 'usecs' to avoid potential confusion by @aronowski in rhboot#563 * Optionally allow to keep shim protocol installed by @bluca in rhboot#565 * SBAT-related documents formatting and spelling by @aronowski in rhboot#566 * Add SbatLevel_Variable.txt to document the various revocations by @jsetje in rhboot#569 * Add a security contact email address in README.md by @vathpela in rhboot#572 * Use -Wno-unused-but-set-variable for Cryptlib and OpenSSL by @vathpela in rhboot#576 * mok: fix LogError() invocation by @vathpela in rhboot#577 * Minor housekeeping by @vathpela in rhboot#578 * Test ImageAddress() by @vathpela in rhboot#579 * FreePages() is used to return memory allocated by AllocatePages() by @dennis-tseng99 in rhboot#580 * Size should minus 1 when calculating 'RelocBaseEnd' by @jsetje in rhboot#581 * Verify signature before verifying sbat levels by @jsetje in rhboot#583 * Add libFuzzer support for csv.c and sbat.c by @vathpela in rhboot#584 * mok: Avoid underflow in maximum variable size calculation by @alpernebbi in rhboot#587 * Housekeeping by @vathpela in rhboot#605 Signed-off-by: Peter Jones <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.