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.
Reviewed-on: https://gerrit.openafs.org/15471
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
(cherry picked from commit f2003ed68c2fecf679d0b04146427258d39369ea)
Change-Id: Ib7ae86c66f0ef1fc12d4ff4b796b712dc97e2e13
Reviewed-on: https://gerrit.openafs.org/15508
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Every function should be explicitly declared before it can be called.
Since -Wimplicit-function-declaration is usually a warning and not an
error, calling undeclared functions does not prevent us from building
the code.
However, apple-clang 12 makes this an error by default, prohibiting the
build of the current version on macOS 11 (Big Sur).
To fix this problem, declare functions before calling them. Also,
include mach/thread_act.h into afs_call.c so the declaration of
thread_terminate() can be found on macOS. Last, given that the third
argument of PIOCTL() (if UKERNEL is defined) is a pointer, cast it to
'long'. Doing so, we can avoid another inhibited warning. Notice that
this PIOCTL definition is scoped to a single file (src/auth/ktc.c).
Reviewed-on: https://gerrit.openafs.org/14744
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit f9c96d0fd609e14fcb8ff7d9269024e026f742cb)
Change-Id: I70dadc44d40e1fc877f9e0490e28636907432faa
Reviewed-on: https://gerrit.openafs.org/15246
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Harish Sharma <Harish.Sharma1@ibm.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
In an effort to avoid the usage of undeclared functions, add wrappers
for ubik_Call_SingleServer() (_BUDB_GetVolumes(), _BUDB_DumpDB()) and
adjust its callers accordingly.
Also, make sure that ubik_Call_SingleServer() uses the same signature as
ubik_Call(). This change helps us to get rid of casting errors.
Reviewed-on: https://gerrit.openafs.org/14886
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 981b3d723ec56dad553be570147d1b2aa632f4e1)
Change-Id: I5893a939bc5cd64eec9dbbf1ce8a0dc5032c81f0
Reviewed-on: https://gerrit.openafs.org/15244
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Harish Sharma <Harish.Sharma1@ibm.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
In lwp/process.c the address of a stack variable is saved as part of
creating a new context. GCC-12 is flagging the statement with a
diagnostic:
./process.c:46:24: error: storing the address of local variable
‘stackvar’ in ‘*savearea.topstack’ [-Werror=dangling-pointer=]
46 | savearea->topstack = (char *)&stackvar;
| ~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
In this particular case, the code is meaning to save the address of the
stack in preparation of setting up a new context, which requires knowing
the address the current stack.
The diagnostic is changed from a warning to an error when configured
with --enable-checking.
Set the env variable CFLAGS_NODANGLING_POINTER to
'-Wno-dangling-pointer' if the compiler supports the option and update
the src/lwp/Makefile.in to use the flag when compiling process.c
Reviewed-on: https://gerrit.openafs.org/14957
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 748ae0fc25c51316d741afd9b9dfd479ffdf3250)
Change-Id: I03d2d4a77b4a391f023f40e9793968e03a50c241
Reviewed-on: https://gerrit.openafs.org/15062
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
The linux 5.17 commit:
"Makefile: Enable -Wcast-function-type" (552a23a0)
added the -Wcast-function-type compiler flag for kernel module builds.
This change catches a type mismatch in the external files obtained from
heimdal: hcrypto/evp.c and hcrypto/evp-algs.c and produces the following
type of compile time error messages.
src/libafs/MODLOAD-.../evp.c: In function ‘hc_EVP_md_null’:
src/libafs/MODLOAD-.../evp.c:501:2: error: cast between incompatible
function types from ‘void (*)(void *)’ to ‘int (*)(EVP_MD_CTX *)’
{aka ‘int (*)(struct hc_EVP_MD_CTX *)’}
[-Werror=cast-function-type]
501 | (hc_evp_md_init)null_Init,
| ^
Use AX_APPEND_COMPILE_FLAGS to create a CFLAGS_NOCAST_FUNCTION_TYPE
macro to disable this warning and update the CFLAGS for these 2 files
for the Linux libafs build.
Update the CODING documentation to add the new exceptions. In addition
add a brief description on how to set up autoconf to add a new build
macro to suppress compiler warnings.
Note: upstream heimdal has committed a fix for this in:
hcrypto: Fix return type for null_Init, null_Update and null_Final
(fc4b3ce49b)
Reviewed-on: https://gerrit.openafs.org/14881
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
(cherry picked from commit 6bdfa976731ce07f3236893ecf12abb9e169b882)
Change-Id: Ibd354f663d5876c421a8b4e89b8943c9e3d59ebc
Reviewed-on: https://gerrit.openafs.org/14946
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Clang-10 will not recognize '/* fall through */' as an indicator to
turn off the fallthrough warning due to the lack of a 'break' in a case
statement.
Code generated by flex uses the '/* fall through */' comments to turn
off compiler warnings for fallthroughs in case statements.
For code generated by flex, ignore the implicit-fallthrough via pragma
or disable the warning via a compile time flag.
Add new env variable "CFLAGS_NOIMPLICIT_FALLTHROUGH" to selectively
disable the compile check in Makefiles when checking is enabled.
Reviewed-on: https://gerrit.openafs.org/14275
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit e5f44f6e9af643cab3a66216dff901e0a4c5eda8)
Change-Id: Ibe1b95e6784ca8b422378cf2896bdc7f1a6d8e61
Reviewed-on: https://gerrit.openafs.org/14990
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Clang-10 will not recognize '/* fallthrough */' as an indicator to
turn off the fallthrough diagnostic due to the lack of a 'break' in a
case statement. Clang-10 requires the '__attribute__((fallthrough))'
statement to disable the diagnostic.
In addition clang-10 is finding additional locations where fall throughs
occur.
Determine if the compiler supports '__attribute__((fallthrough))' to
disable the implicit fallthrough diagnostic.
Define a new macro 'AFS_FALLTHROUGH' that will disable the fallthrough
diagnostic. Set it as a wrapper for the Linux kernel's 'fallthrough'
macro if available, otherwise set it as a wrapper macro for
'__attribute__((fallthrough))' if the compiler supports it.
Update CODING to document the use of AFS_FALLTHROUGH when needing to
fallthrough between case statements.
Replace the '/* fallthrough */' comments with AFS_FALLTHROUGH, and add
AFS_FALLTHROUGH as needed.
Replace some fallthroughs with a break (or goto) if the flow was was
just to a break (or goto).
e.g. case x: case x:
somestmt; somestmt;
break;
case y: case y:
break; break;
Correct a mis-indented brace '}' in src/WINNT/afsd/smb3.c
Note, the clang maintainers have rejected the use of comments as a flag
to turn off the fall through warnings.
Reviewed-on: https://gerrit.openafs.org/14274
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 16f1b2f894c28614df0f096be8232b1176e87c70)
[cwills@sinenomine.net Note the fallthrough case in asetkey.c that
exists in the original master commit is not present 1.8.x]
Change-Id: I4d92d519bd168ac111f46d37bcf7dca7021e5463
Reviewed-on: https://gerrit.openafs.org/14970
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
GCC 7 is producing new warnings due to better compile time analysis.
With --enable-checking v5der.c is failing with 2 errors due to possible
format-truncation in some snprintf calls. The format strings are being
used to format a date and time values from a tm structure.
The actual warnings/errors are being triggered from arithmetic being
performed on the year and month members of the structure. The resulting
values should not exceed the format lengths, but the compilers are still
flagging the statements.
v5der.c is part of the heimdal package that is pulled into the openafs
source tree. v5der.c is not compiled directly but is #included in
ticket5.c
Update ticket5.c to change the severity of the format-truncation
diagnostic to a warning if using GCC 7 (or higher).
Note: since v5der.c is pulled from an external source (heimdal), any
changes to update v5der.c directly would need to be performed upstream.
Reviewed-on: https://gerrit.openafs.org/13661
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
(cherry picked from commit 98ca332c4a5ac9e5687fb4fe21b350134bc74d1b)
Change-Id: I1a808060b302549887e529e74bc3805d9431c499
Reviewed-on: https://gerrit.openafs.org/13727
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Update the style guide with a declaration of the prevailing and
preferred brace style for one-line if statements and loops. Provide an
example and counter-example.
Change-Id: Iafeea977203b76c0e67385779fb4ed57f3c6699a
Reviewed-on: https://gerrit.openafs.org/12370
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Document the preferred style for multiple line comment blocks and give an
example.
Change-Id: I73d6183da9014a943316e5aea1d43be2acc81ad7
Reviewed-on: https://gerrit.openafs.org/12361
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Resolves these warnings:
ticket5.c: In function ‘tkt_MakeTicket5’:
ticket5.c:574:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncTicketPart’ differ in signedness [-Wpointer-sign]
code = encode_EncTicketPart(encodebuf, allocsiz, &data, &encodelen);
^
In file included from ticket5.c:80:0:
v5gen-rewrite.h:43:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
#define encode_EncTicketPart _rxkad_v5_encode_EncTicketPart
^
v5gen.c:1889:1: note: in expansion of macro ‘encode_EncTicketPart’
encode_EncTicketPart(unsigned char *p, size_t len, const EncTicketPart * data, size_t * size)
^
ticket5.c:602:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncryptedData’ differ in signedness [-Wpointer-sign]
code = encode_EncryptedData(ticket + *ticketLen - 1, *ticketLen, &encdata, &tl);
^
In file included from ticket5.c:80:0:
v5gen-rewrite.h:16:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
#define encode_EncryptedData _rxkad_v5_encode_EncryptedData
^
v5gen.c:690:1: note: in expansion of macro ‘encode_EncryptedData’
encode_EncryptedData(unsigned char *p, size_t len, const EncryptedData * data, size_t * size)
^
ticket5.c: In function ‘tkt_DecodeTicket5’:
ticket5.c:320:10: warning: ‘plainsiz’ may be used uninitialized in this function [-Wmaybe-uninitialized]
code = decode_EncTicketPart((unsigned char *)plain, plainsiz, &decr_part, &siz);
^
Change-Id: Ic1b878f01cf82222dc258847747ce192ee5948fc
Reviewed-on: http://gerrit.openafs.org/11955
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Resolves this warning:
admin_tools.c: In function ‘SetFields’:
admin_tools.c:611:30: warning: pointer targets in passing argument 2 of ‘ktime_DateToInt32’ differ in signedness [-Wpointer-sign]
code = ktime_DateToInt32(s, &expiration);
^
In file included from /home/anders/wd/openafs/include/afs/afsutil.h:84:0,
from admin_tools.c:39:
/home/anders/wd/openafs/include/afs/afsutil_prototypes.h:101:18: note: expected ‘afs_int32 *’ but argument is of type ‘afs_uint32 *’
extern afs_int32 ktime_DateToInt32(char *adate, afs_int32 * aint32);
^
Change-Id: Id24e7a6cd1ab2291c0c05d3835f4ad7fddfec8d7
Reviewed-on: http://gerrit.openafs.org/11956
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Replaces this warning:
afs_kasAdmin.c: In function ‘GetPrincipalLockStatus’:
afs_kasAdmin.c:710:6: warning: implicit declaration of function ‘ubik_CallIter’ [-Wimplicit-function-declaration]
ubik_CallIter(KAM_LockStatus, kaserver->servers, UPUBIKONLY,
^
with these marginally less alarming warnings:
In file included from ../adminutil/afs_AdminInternal.h:17:0,
from afs_kasAdmin.c:21:
/home/anders/wd/openafs/include/ubik.h:627:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
extern afs_int32 ubik_CallIter(int (*aproc) (), struct ubik_client *aclient,
^
/home/anders/wd/openafs/include/ubik.h:632:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
extern afs_int32 ubik_Call_New(int (*aproc) (), struct ubik_client
^
Change-Id: I49dbc5f6bb9199764c73c6ee8449d62518f377e6
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Reviewed-on: http://gerrit.openafs.org/11954
Reviewed-by: Perry Ruiter <pruiter@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
Move the information about compiler warnings to the CODING
readme file.
Change-Id: I0be752c76ddee809fe80bd1f97048953eeee89ee
Reviewed-on: http://gerrit.openafs.org/10975
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: D Brashear <shadow@your-file-system.com>
Move the REAME.GIT information to the CODING readme file.
Change-Id: I3013e03ebfe003dce23f0e2d808ab6905dd2b452
Reviewed-on: http://gerrit.openafs.org/10974
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: D Brashear <shadow@your-file-system.com>
Rename the developer's readme file to CODING as a home
for developer related information.
Change-Id: I8c2cf70258671387b926ef3d666f6476056ef06e
Reviewed-on: http://gerrit.openafs.org/10973
Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: D Brashear <shadow@your-file-system.com>
