From f2003ed68c2fecf679d0b04146427258d39369ea Mon Sep 17 00:00:00 2001 From: Cheyenne Wills Date: Mon, 3 Jul 2023 13:14:52 -0600 Subject: [PATCH] gcc: Avoid false positive use-after-free in crypto MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Due to a bug in gcc-12 and gcc-13, several warnings are generated for a use-after-free in crypto.c, which leads to a build failure with --enable-checking: src/external/heimdal/krb5/crypto.c:1157:9: error: pointer ‘p’ may be used after ‘realloc’ [-Werror=use-after-free] 1157 | free(p); | ^~~~~~~ src/external/heimdal/krb5/crypto.c:1155:20: note: call to ‘realloc’ here 1155 | result->data = realloc(p, sz); | ^~~~~~~~~~~~~~ However, reviewing the code around these warnings shows that the use-after-free warnings are incorrectly generated (false positive). The documentation for realloc states that realloc will return a NULL and not alter the storage passed if there was an error allocating and the size passed is non-zero. There is a possible work-around for the false positive. One can use a variable that is not a member of a structure to hold and test the value returned from realloc, then update the structure member from that variable. However, the code that is producing the message is in a heimdal external file, so we cannot modify the source. So just use the compiler flag -Wno-use-after-free to avoid the warning/error. Update configure to add tests for the -Wno-use-after-free flag, update the Makefile to add the flag for CFLAGS.crypto.lo, and update CODING for the new exception. Because this is an important check, only disable the warning if the compiler exhibits this specific bug. We do this by adding specific configure tests for the compiler bug and conditionally set a CFLAG variable if the bug is present. NOTE: The false positive and work-around can be demonstrated with the following code using gcc-12 (with -O0) or gcc-13 (not sensitive to the optimization level): somestruct->somepointer = realloc(ptr, somesize); if (somestruct->somepointer == NULL && somesize != 0) { free(ptr); << gets flagged as use-after-free handle enomem... } However the following doesn't get flagged: char *tmpptr = realloc(ptr, somesize); if (tmpptr == NULL && somesize != 0) { free(ptr); handle enomem... } somestruct->somepointer = tmpptr; The GCC ticket https://gcc.gnu.org/bugzilla/show_bug.cgi?id=110501 has been marked as confirmed. Change-Id: I28566354da4199389964210f954cda1213098088 Reviewed-on: https://gerrit.openafs.org/15471 Reviewed-by: Benjamin Kaduk Tested-by: BuildBot Reviewed-by: Andrew Deason --- CODING | 2 ++ src/cf/gcc.m4 | 51 ++++++++++++++++++++++++++++++++++ src/cf/osconf.m4 | 1 + src/crypto/rfc3961/Makefile.in | 1 + 4 files changed, 55 insertions(+) create mode 100644 src/cf/gcc.m4 diff --git a/CODING b/CODING index 1bd62baf08..aff53f91e7 100644 --- a/CODING +++ b/CODING @@ -339,6 +339,8 @@ external/heimdal/hcrypto/evp.c: cast-function-type : Linux kernel build uses -Wcast-function-type external/heimdal/hcrypto/evp-algs.c: cast-function-type : Linux kernel build uses -Wcast-function-type +external/heimdal/krb5/crypto.c: use-after-free : False postive on certain GCC + compilers kauth/admin_tools.c : strict-proto : ubik_Call kauth/authclient.c : strict-proto : ubik_Call nonsense libadmin/kas/afs_kasAdmin.c: strict-proto : ubik_Call nonsense diff --git a/src/cf/gcc.m4 b/src/cf/gcc.m4 new file mode 100644 index 0000000000..7747807893 --- /dev/null +++ b/src/cf/gcc.m4 @@ -0,0 +1,51 @@ +dnl Checks for specific gcc behavior + +dnl Helper to test for UAF warning message +dnl _OPENAFS_UAF_COMPILE_IFELSE([success], [fail]) +AC_DEFUN([_OPENAFS_UAF_COMPILE_IFELSE],[ + AC_COMPILE_IFELSE( + [AC_LANG_PROGRAM([[ + #include + struct gcc_check { + char *ptr; + }; + void test(struct gcc_check *p, char *cp, int size) + { + p->ptr = realloc(cp, size); + if (p->ptr == NULL && size != 0) { + free(cp); /* If compiler has UAF bug this will be flagged */ + } + } + ]] + )], + [$1], + [$2] + ) +]) + +dnl Check to see if the GCC compiler incorrectly flags use-after-free (UAF). +dnl This false positive has been observed with gcc 12 when +dnl optimization is disabled (-O0) and gcc 13. +AC_DEFUN([OPENAFS_GCC_UAF_BUG_CHECK],[ + CFLAGS_USE_AFTER_FREE_GCCBUG= + AS_IF([test "x$GCC" = "xyes"], [ + AC_MSG_CHECKING([gcc use-after-free warning bug]) + ac_save_CFLAGS="$CFLAGS" + CFLAGS="$CFLAGS -Wall -Werror -O0" + _OPENAFS_UAF_COMPILE_IFELSE( + [AC_MSG_RESULT(no)], + [ + dnl Compiler flagged an error. Run one more check to ensure + dnl the error was only the false positive for a UAF. + AX_APPEND_COMPILE_FLAGS([-Wno-use-after-free], + [CFLAGS_USE_AFTER_FREE_GCCBUG], [-Werror]) + CFLAGS=" $CFLAGS $CFLAGS_USE_AFTER_FREE_GCCBUG" + _OPENAFS_UAF_COMPILE_IFELSE( + [AC_MSG_RESULT(yes)], + [AC_MSG_ERROR([Unexpected compiler error while testing for gcc use-after-free bug])]) + ] + ) + CFLAGS="$ac_save_CFLAGS" + ]) + AC_SUBST([CFLAGS_USE_AFTER_FREE_GCCBUG]) +]) diff --git a/src/cf/osconf.m4 b/src/cf/osconf.m4 index d08f45f7c2..c4bbe281cb 100644 --- a/src/cf/osconf.m4 +++ b/src/cf/osconf.m4 @@ -644,6 +644,7 @@ if test "x$GCC" = "xyes"; then [CFLAGS_NOIMPLICIT_FALLTHROUGH], [-Werror]) AX_APPEND_COMPILE_FLAGS([-Wno-dangling-pointer], [CFLAGS_NODANGLING_POINTER], [-Werror]) + OPENAFS_GCC_UAF_BUG_CHECK AC_DEFINE(IGNORE_SOME_GCC_WARNINGS, 1, [define to disable some gcc warnings in warnings-as-errors mode]) else CFLAGS_NOSTRICT= diff --git a/src/crypto/rfc3961/Makefile.in b/src/crypto/rfc3961/Makefile.in index 6fce447ad9..e389048716 100644 --- a/src/crypto/rfc3961/Makefile.in +++ b/src/crypto/rfc3961/Makefile.in @@ -48,6 +48,7 @@ ${TOP_LIBDIR}/libafsrfc3961.a: libafsrfc3961.a CFLAGS_crypto-arcfour.lo=@CFLAGS_NOERROR@ CFLAGS_crypto-des-common.lo=@CFLAGS_NOERROR@ +CFLAGS_crypto.lo=@CFLAGS_USE_AFTER_FREE_GCCBUG@ context.lo: context.c ${HEADERS} copy.lo: copy.c ${HEADERS}