Currently, the IsDCacheSizeOK check can trigger a false positive for a
dcache, if the data in the dcache was populated by a local write to a
file that was later extended with sparse data.
For example: say a client opens a new file, and writes 4 bytes to
offset 0, and then writes 4 bytes to offset 0x400000. After the first
write, the first chunk for the file will contain just 4 bytes, and
after the second write, the first chunk is unchanged (since we're
writing to a different area of the file), but the file is now 0x400004
bytes long. The sparse area of the file will be correctly filled with
zeroes for local reads and on the fileserver, but the 4-byte chunk
causes IsDCacheSizeOK to complain and mark the dcache as invalid.
Even though nothing is wrong, this causes the following scary messages
to potentially appear in the kernel log, and the relevant dcache to be
invalidated:
afs: Detected corrupt dcache for file 1.536870913.2.2: chunk 0 (offset 0) has 4 bytes, but it should have 131072 bytes
afs: (dcache 0xfffffdeadbeefb4d, file length 4194308, DV 1, dcache mtime 1575049956, index 996, dflags 0x2, mflags 0x0, states 0x4, vcache states 0x1)
afs: Ignoring the dcache for now, but this may indicate corruption in the AFS cache, or a bug.
It's probably difficult or impossible to detect if this specific case
is happening, so to avoid this scenario, just avoid doing the size
check at all for RW data from the cache.
Change-Id: Ia40ec838c525d9abc13a03be39028e4ca04a9457
Reviewed-on: https://gerrit.openafs.org/13969
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, a fileserver can be initialized as readonly. In this mode,
writes on this server should not be allowed. Unfortunately, updates on
files stored by readonly fileservers are not completely prevented. In
some situations, the check for RO server is omitted (e.g. if the user is
the owner of the file to be updated). In other situations, the same
check is redundant.
To fix these problems, consolidate this check in one place.
Change-Id: Id53e15216404dfe691a87c7b4964ff08924c262c
Reviewed-on: https://gerrit.openafs.org/13934
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
The variable errno might be set by some system calls to indicate the
reason why the system call in question did not work as expected. If the
setpag system call is interrupted by a signal, the value of errno will
be EINTR. This value means that setpag did not succeed because it was
interrupted.
If lsetpag did not succeed and errno is equal to EINTR, try again.
Change-Id: Ibf306d62fc8d2fa9ccb0692f9031c5aa659b2bfe
Reviewed-on: https://gerrit.openafs.org/12295
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
To enforce a maximum average rate of one PAG allocation per second,
afs_pag_wait(), called by afs_setpag*(), sleeps until the difference
between the current time and pag_epoch gets greater than pagCounter.
Unfortunately, this function ignores the code returned by afs_osi_Wait().
As a result, it is not possible to kill the process that requested the
new pag while afs_pag_wait() is sleeping.
To fix this problem, do not ignore the code returned by afs_osi_Wait().
Change-Id: I6be11a569edcafa6ecdf716e5315fc75f5a128e8
Reviewed-on: https://gerrit.openafs.org/12260
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, in afs_write(), we set CDirty on the given vcache, and then
write the given data into various dcaches. When writing to a dcache,
we call afs_DoPartialWrite, which may cause us to flush the dirty data
to the fileserver and clear the CDirty bit.
If we were given more than 1 chunk of data to write, we will then go
through another iteration of the loop, writing more dirty data into
dcaches, but CDirty will not be set. This can cause issues with, for
example, afs_SimpleVStat() or afs_ProcessFS(), which use CDirty to
determine whether or not to merge in FetchStatus info from the
fileserver into our local cache. This can cause our local cache to
incorrectly reflect the state of the file on the fileserver, instead
of the state of the locally-modified file in our cache.
A more detailed example is as follows. Consider a small C program that
copies a file, fchmod()ing the destination before closing it:
void
do_copy(char *src_name, char *dest_name)
{
/* error checking elided */
src_fd = open(src_name, O_RDONLY);
dest_fd = open(dest_name, O_WRONLY|O_CREAT|O_TRUNC, 0755);
fstat(src_fd, &st);
src_buf = mmap(NULL, st.st_size, PROT_READ, MAP_SHARED, src_fd, 0);
write(dest_fd, src_buf, st.st_size);
munmap(src_buf, st.st_size);
close(src_fd);
fchmod(dest_fd, 0100644);
close(dest_fd);
}
Currently, on FBSD, using this to copy a 7862648-byte file, using a
smallish cache (10000 blocks) will cause the destination to appear to
be truncated, because avc->f.m.Length will be incorrect, even though
all of the relevant data was written to the fileserver.
On most other platforms such as SOLARIS and LINUX, this is not a
problem, since currently they only write one page of data at a time to
afs_write(), and so they never hit multiple iterations of the while()
loop inside afs_write().
To fix this, just set CDirty on every iteration of the while() loop in
afs_write(). In general, we need to set CDirty after calling
afs_DoPartialStore() anywhere if the caller continues to write more
data. But all callers already do this, except for this one instance in
afs_write().
Thanks to tcreech@tcreech.com for helping find occurrences of the
relevant issue.
FIXES 135041
Change-Id: I0f7a324ea2d6987a576786292be2d06487359aa6
Reviewed-on: https://gerrit.openafs.org/13948
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Commit 75e3a589 (libafs: afs_InitVolSlot function) split out a bit of
our code that initializes a struct volume into the afs_InitVolSlot
function. However, it caused us to almost always pass a non-NULL 'tf'
to afs_InitVolSlot, even if the target volume was not found.
That is, before that commit, our code roughly did this:
for (...; j != 0; j = tf->next) {
...;
tf = &staticVolume;
if (tf->volume == volid)
break;
}
if (tf && j != 0) {
use_tf_data();
} else {
use_blank_data();
}
The reason for the extra 'j != 0' check after the loop is to see if we
hit the end of the volume hash chain, or if we actually found a
matching 'tf' in the loop.
And after that commit, the code did this:
for (...; j != 0; j = tf->next) {
...;
if (j != 0) {
tf = &staticVolume;
if (tf->volume == volid)
break;
}
}
if (tf) {
use_tf_data();
} else {
use_blank_data();
}
The check for 'j != 0' was moved to inside the for loop, but 'j' is
always nonzero in the loop (otherwise, the for() would exit the loop).
This means that if we didn't find a matching 'tf' in the loop, our
'tf' would be non-NULL anyway, and so we'd initialize our volume slot
from just the last entry in the hash chain.
This means that for volumes that are not found in the VolumeItems
file, our struct volume will probably be initialized with arbitrary
data from another volume, instead of being initialized to the normal
defaults (the 'else' clause in afs_InitVolSlot).
This means that the 'dotdot' entry for the volume may be wrong, and so
we may report the wrong parent dir for the root of a volume. However,
the 'dotdot' entry should be fixed when the volume root is accessed
via a mountpoint, so any such issue should be temporary. And of
course, on some platforms (LINUX) we don't ever use the 'dotdot'
information for a volume, and even on other platforms, often resolving
the '..' entry is handled by other means (e.g. shells often calculate
it themselves). But some 'pwd' calculations and other '..' corner
cases may be affected.
To fix this, change the relevant loop so that we only set 'tf' to
non-NULL when we actually find a matching entry.
Change-Id: I53118960462c0057725e749cbf588e98024217c3
Reviewed-on: https://gerrit.openafs.org/13933
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit c6b61a45 (afs: Verify osi_UFSOpen worked) added various checks
to return an error if a given osi_UFSOpen failed. However, two of
these checks (in afs_UFSReadUIO and afs_UFSWriteUIO) result in us
returning -1 on error, in functions that otherwise return errno codes
(e.g. ENOSPC). An error code of -1 might get interpreted as
RX_CALL_DEAD, which would be rather confusing, so use EIO as a generic
error instead.
Change-Id: I23b9a73b82d999d8ee4670b5e7ec39b9d820fb0f
Reviewed-on: https://gerrit.openafs.org/13931
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In this example, krbtgt.Example.COM clearly refers to the principal
name converted from krbtgt/Example.COM, and so by convention the realm
name would be in all caps. Fix this example to use the all-caps realm
name, for consistency.
This mistake was introduced by commit 1cc8feb6 (doc: replace hostnames
with IETF example hostnames), the realm was in all caps before that
commit.
Mistake spotted by Chas Williams.
Change-Id: Icaf4931868752064c4617c8ad778122e076ae3cb
Reviewed-on: https://gerrit.openafs.org/13930
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, SVOTE_Debug/SVOTE_DebugOld examine some ubik internal state
without any locks, because the speed of these functions is more
important than accuracy.
However, one of the pieces of data we examine is ubik_currentTrans,
which we dereference to get ubik_currentTrans->type. ubik_currentTrans
could be set to NULL while this code is running, so there is a small
chance of this code causing a segfault, if SVOTE_Debug() is running
when the current transaction ends.
We only ever initialize ubik_currentTrans as a write transation (via
SDISK_Begin), so this check is pointless anyway. Accordingly, skip the
type check, and always assume that any active transaction is a write
transaction. This means we only ever access ubik_currentTrans once,
avoiding any risk of the value changing between accesses (and we no
longer need to dereference it, anyway).
Note that, since ubik_currentTrans is not marked as 'volatile', some C
compilers, with certain options, can and do assume that its value will
not change between accesses, and thus only fetch the pointer value once.
This avoids the risk of NULL dereference (and thus, crash, if pointer
stores/loads are atomic), but the value pointed to by ubik_currentTrans->type
would be incorrect when the transaction ends during the execution of
SVOTE_Debug().
Change-Id: Ia36c58e5906f5e8df59936f845ae11e886e8ec38
Reviewed-on: https://gerrit.openafs.org/13915
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, our server-side RPC argument-handling code generated from
rxgen initializes complex arguments like so (for example, in
_RXAFS_BulkStatus):
AFSCBFids FidsArray;
AFSBulkStats StatArray;
AFSCBs CBArray;
AFSVolSync Sync;
FidsArray.AFSCBFids_val = 0;
FidsArray.AFSCBFids_len = 0;
CBArray.AFSCBs_val = 0;
CBArray.AFSCBs_len = 0;
StatArray.AFSBulkStats_val = 0;
StatArray.AFSBulkStats_len = 0;
This is done for any input or output arguments, but only for types we
need to free afterwards (arrays, usually). We do not do this for
simple types, like single flat structs. In the above example, we do
this for the arrays FidsArray, StatArray, and CBArray, but 'Sync' is
not initialized to anything.
If some server RPC handlers never set a value for an output argument,
this means we'll send uninitialized stack memory to our peer.
Currently this can happen in, for example,
MRXSTATS_RetrieveProcessRPCStats if 'rxi_monitor_processStats' is
unset (specifically, the 'clock_sec' and 'clock_usec' arguments are
never set when rx_enableProcessRPCStats() has not been called).
To make sure we cannot send uninitialized data to our peer, change
rxgen to instead 'memset(&arg, 0, sizeof(arg));' for every single
parameter. Using memset in this way just makes this a little simpler
inside rxgen, since all we need to do this is the name of the
argument.
With this commit, the rxgen-generated code for the above example now
looks like this:
AFSCBFids FidsArray;
AFSBulkStats StatArray;
AFSCBs CBArray;
AFSVolSync Sync;
memset(&FidsArray, 0, sizeof(FidsArray));
memset(&CBArray, 0, sizeof(CBArray));
memset(&StatArray, 0, sizeof(StatsArray));
memset(&Sync, 0, sizeof(Sync));
Change-Id: Iedccc25e50ee32bd1144e652b951496cb7dde5d2
Reviewed-on: https://gerrit.openafs.org/13914
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, part of our server-side RPC argument-handling code that's
generated from rxgen looks like this (for example):
z_result = SRXAFS_BulkStatus(z_call, &FidsArray, &StatArray, &CBArray, &Sync);
z_xdrs->x_op = XDR_ENCODE;
if ((!xdr_AFSBulkStats(z_xdrs, &StatArray))
|| (!xdr_AFSCBs(z_xdrs, &CBArray))
|| (!xdr_AFSVolSync(z_xdrs, &Sync)))
z_result = RXGEN_SS_MARSHAL;
fail:
[...]
return z_result;
When the server routine for implementing the RPC results a non-zero
value into z_result, the call will be aborted. However, before we
abort the call, we still call the xdr_* routines with XDR_ENCODE for
all of our output arguments. If the call has not already been aborted
for other reasons, we'll serialize the output argument data into the
Rx call. If we push more data than can fit in a single Rx packet for
the call, then we'll also send that data to the client. Many server
routines for implementing RPCs do not initialize the memory inside
their output arguments during certain errors, and so the memory may be
leaked to the peer.
To avoid this, just jump to the 'fail' label when a nonzero 'z_result'
is returned. This means we skip sending the output argument data to
the peer, but we still free any argument data that needs freeing, and
record the stats for the call (if needed). This makes the above
example now look like this:
z_result = SRXAFS_BulkStatus(z_call, &FidsArray, &StatArray, &CBArray, &Sync);
if (z_result)
goto fail;
z_xdrs->x_op = XDR_ENCODE;
if ((!xdr_AFSBulkStats(z_xdrs, &StatArray))
|| (!xdr_AFSCBs(z_xdrs, &CBArray))
|| (!xdr_AFSVolSync(z_xdrs, &Sync)))
z_result = RXGEN_SS_MARSHAL;
fail:
[...]
return z_result;
Change-Id: I2bdea2e808bb215720492b0ba6ac1a88da61b954
Reviewed-on: https://gerrit.openafs.org/13913
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
With commit 6e0f1c3b45 (LINUX: Honor
--enable-checking for libafs) building libafs against a linux 5.3
kernel compiles with errors due to fall through in case statements when
--enable-checking / --enable-warning is used.
e.g.
src/opr/jhash.h:82:17: error: this statement may fall through
[-Werror=implicit-fallthrough=]
case 3 : c+=k[2];
~^~~~~~
The GCC compiler will disable the implicit-fallthrough check for case
statements that contain a "special" comment ( /* fall through */ ).
Add the 'fall through' comment to indicate where fall throughs are
acceptable.
This commit only adds comments and does not alter any executable code.
The -Wimplicit-fallthrough flag was enabled globally in the linux kernel
build in 5.3-rc2 (commit: a035d552a93bb9ef6048733bb9f2a0dc857ff869
Makefile: Globally enable fall-through warning)
Change-Id: Ie6ca425e04b53a22d07b415cb8afd172af7e8081
Reviewed-on: https://gerrit.openafs.org/13881
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In the VLDB, there's only one logical entry for a volume and its
associated clones; there are not separate entries for the RW volume
"avol", the RO volume "avol.readonly", and the BK volume
"avol.backup". And so, when looking up a volume in the VLDB by name,
the vlserver ignores any trailing ".readonly" or ".backup" in the
given name. More concretely, the result of calling
VL_GetEntryByName*("avol") is identical to that from calling
VL_GetEntryByName*("avol.readonly").
Accordingly, if afs_GetVolumeByName(name) failed because the volume
was not found in the VLDB, afs_GetVolumeByName(name.readonly) will
fail as well (barring a change in external circumstances, such as the
volume being created or a network connection coming back up).
Therefore, the extra call in EvalMountData() is not necessary and can
be removed.
Remove the extra call, to slightly improve the response time of the
client if the volume in question does not exist, and to reduce
vlserver load when patched clients are looking up nonexistent volumes.
Change-Id: I4f2f668107281565ae72a563a263121bd9bb7e3c
Reviewed-on: https://gerrit.openafs.org/13334
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Install libadmin rxstat_* sample programs with 'make install'/'make
dest'. Include these programs in the openafs rpm package.
Change-Id: I81b965cf440c869072cce0065a3c74c4c699b8b8
Reviewed-on: https://gerrit.openafs.org/13883
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 2f2c2ce62a
'Remove automake autoconf vars' replaced the automake variable @VERSION@
with the autoconf variable @PACKAGE_VERSION@. (Gerrit #13357)
The RedHat openafs.spec.in is not processed using autoconf, but
by 'makesrpm.pl', which was not updated to use @PACKAGE_VERSION@.
Update makesprm.pl to use @PACKAGE_VERSION@ instead of @VERSION@
Change-Id: I74d1d61e40e660459942ec68cfdedfe569a6abeb
Reviewed-on: https://gerrit.openafs.org/13887
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 6ad3d646 (rx: Correctly test for end of call queue) fixed a
broken end-of-queue check in rx_GetCall, but it only fixed the
RX_ENABLE_LOCKS version of rx_GetCall. The non-locks version (i.e. the
LWP version) still had this bug. Fix it for the LWP case, to avoid
some rare cases where an Rx call can get stuck in the incoming queue.
Also remove the comment added by commit 170dbb3c (rx: Use opr queues),
since we're fixing the mentioned problem.
Change-Id: I5b96d97d9aba7bc4b383133b2136f949f3ed22bc
Reviewed-on: https://gerrit.openafs.org/13880
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
From time to time, the fileserver may issue potentially long-running
RXAFSCB_* RPCs back to a host (client). If these are holding h_Lock_r
(host->lock) while running, they may cause other service threads for the
same host (client) to block.
In order to prevent a given host from tying up too many service threads
in this way, the fileserver enforces a quota limiting how many threads
can be waiting for h_Lock_r on a particular host while waiting for one
of the following RPCs to complete:
- RXAFSCB_TellMeABoutYourself (TMAY)
- RXAFSCB_WhoAreYou
- RXAFSCB_ProbeUuid
- RXAFSCB_InitCallBackState (ICBS)
- RXAFSCB_InitCallBackState3 (ICBS3)
Note: Although some of these RPCs are relatively lightweight, they may
still experience network delays.
This quota is enforced by calling h_threadquota() in h_Lookup_r and
h_GetHost_r. The quota check is enabled for a given host by turning on
host->hostFlags HWHO_INPROGRESS for the duration of the RXAFSCB_* RPC.
The quota check is only needed, and should only be enabled, when the RPC
is issued while h_Lock_r is held.
However, there are a few paths to ICBS(3) where h_Lock_r is held but
HWHO_INPROGRESS is not set. A delay in those paths may allow a host to
consume an unlimited number of fileserver threads. One such path
observed in a field report was SRXAFS_FetchStatus -> CallPreamble ->
BreakDelayedCallBacks_r -> RXAFSCB_ICBS3.
Instead, enable host thread quotas for all remaining unregulated ICBS(3)
RPCs.
Change-Id: I70b96055ff80d8650bdbaec0302b7d18a8f22d56
Reviewed-on: https://gerrit.openafs.org/13873
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Originally '%x' was commonly used as the printf specifier for formatting
pointer values.
Commit 37fc3b0144 introduced the
AFS_PTR_FMT macro to support platform-dependent printf format specifiers
for pointer representation. This macro defined the format specifier as
'%p' for Windows, and '%x' for non-Windows platforms.
Commit 2cf12c43c6 changed the printf
pointer format specifier from '%x' to '%p' on non-Windows platforms as
well, so at this point '%p' is the printf pointer format specifier for
all supported platforms.
Since the AFS_PRT_FMT macro is no longer platform-dependent, and all C89
compilers support the '%p' specifier, retire the macro to simplify the
printf format strings.
Change-Id: I0cb13cccbe6a8d0000edd162b623ddcdb74c1cf7
Reviewed-on: https://gerrit.openafs.org/13830
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Currently, we use $(LT_LDLIB_shlib) to build most of our shared
libraries. This invokes libtool, passing our various flags like
PTH_LDFLAGS and PTH_CFLAGS (since all of our shared-library code is for
pthreads). Notably, we do NOT pass the -shared flag; the -shared flag
tells libtool to only build a shared library, and to not also build a
static library (on systems where libtool supports building shared and
static libraries simultaneously). Because of this, our LT_LDLIB_shlib
invocations build both, which is reasonably correct for our per-module
convenience libraries (that end up getting linked statically into the
binaries that we install), but is not entirely correct for the public
libraries that we install. Specifically, for ABI compatibility
purposes, we must provide both shared and static libraries of the public
libraries that we install, and since libtool on AIX does not build (or
install) a static library at all with --mode-link unless -static is
passed, we have separate rules to build the shared and static libraries
for final installation.
This can cause install errors with parallel make (on non-AIX systems),
and possibly other errors, when we go to install the relevant library
into TOP_LIBDIR. For example, in src/kopenafs, we have the following
rules:
${TOP_LIBDIR}/libkopenafs.${SHLIB_SUFFIX}: libkopenafs.la
${LT_INSTALL_DATA} libkopenafs.la ${TOP_LIBDIR}/libkopenafs.la
${RM} ${TOP_LIBDIR}/libkopenafs.la
${TOP_LIBDIR}/libkopenafs.a: libkopenafs.a
${INSTALL_DATA} libkopenafs.a $@
The rule to install libkopenafs.so will invoke libtool to do the
install, which will install libkopenafs.so, libkopenafs.so.X.Y, and
libkopenafs.a (from .libs/libkopenafs.a, not the libkopenafs.a we
built separately). If we are running the rule to install libkopenafs.a
in parallel, it may fail with an error like so:
/usr/bin/install -c -m 644 libkopenafs.a /home/buildbot/openafs/fedora26-x86_64/build/lib/libkopenafs.a
/usr/bin/install: cannot create regular file '/home/buildbot/openafs/fedora26-x86_64/build/lib/libkopenafs.a': File exists
make[3]: *** [Makefile:35: /home/buildbot/openafs/fedora26-x86_64/build/lib/libkopenafs.a] Error 1
Even without that error, this confusion means that the libkopenafs.a
installed into TOP_LIBDIR may be the one from
src/kopenafs/libkopenafs.a, or the one from libtool's
src/kopenafs/.libs/libkopenafs.a; it depends on what order the rules
are run. If those libraries are different, that could potentially
cause all sorts of other problems.
To avoid this, we can pass -shared to libtool when building our shared
libraries. We used to pass -shared when building shared libraries,
since -shared is almost always one our SHLIB_LDFLAGS set in
src/osconf.m4. However, ever since commit 2c3a517e (Retire
Makefile.shared), SHD_CFLAGS, SHD_LDFLAGS, and SHD_CCRULE have all
been unused, and SHD_LDFLAGS was the only place where we used
SHLIB_LDFLAGS. As a result, we never use SHLIB_LDFLAGS anywhere, and
so we never pass -shared to anything.
However, we cannot pass -shared to libtool when building all of our
shared libraries, since we do need the static library for our per-module
convenience libraries. For example, liboafs_rx.la has no
separately-built static library (librx.a is for LWP, liboafs_rx.{so,a}
is for pthreads), but liboafs_rx needs to be linked statically into all
of our command-line tools.
So to fix this, introduce a new linking rule, called
LT_LDLIB_shlib_only, which causes the given library to be built only
as a shared library (by giving -shared to libtool), and not as a
static library. Update the build rules to use this new linking rule
for the libraries that need it, and leave the others alone. Since the
only use of LT_LDLIB_shlib_missing is also used for a public library
(afshcrypto), also pass -shared in that rule.
Also remove SHD_* and SHLIB_LDFLAGS variables, since they are unused.
Change-Id: Ia9e040afa3819f1ff70d050a400fecb9624bb9ba
Reviewed-on: https://gerrit.openafs.org/13786
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently we get tokens for infinite lifetime using aklog impersonate
feature. Based on inputs from Ben, this was done for server to server
tickets to be valid forever. However on 1.8.x we have other
mechanisms that were usable for server-to-server authentication with
strong enctypes, so we do not need to provide user level akimpersonate
to generate tokens for infinite lifetime. For this we have added new
option -token-lifetime <hrs>, this can take values from 0 to 720
hours. If 0 is specified it means tokens will have infinite lifetime.
By default 10 hours will be token lifetime for akimpersonate tokens.
Change-Id: I8190be81771b34682cc000ac051888561dc63c2f
Reviewed-on: https://gerrit.openafs.org/13828
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
In all other places where we remove an rx_call from a queue, we also
CLEAR_CALL_QUEUE_LOCK. This isn't necessary in the LWP
(non-RX_ENABLE_LOCKS) version of rx_GetCall because rx_call does not
have member call_queue_lock for LWP. However, for the sake of
consistency for future maintainers, add a CLEAR_CALL_QUEUE_LOCK here as
well; it is a no-op for LWP.
No functional change is incurred by this commit.
Change-Id: Ibbb005fa15dd517fc5282574d0d4abd74e937e02
Reviewed-on: https://gerrit.openafs.org/13695
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Add the canonical install path for Studio 12.6 to the autoconfig test.
Change-Id: Id90ae1816845ed8aaa80be7b3d57846059084339
Reviewed-on: https://gerrit.openafs.org/13867
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The call_queue_lock is set to either rx_serverPool_lock or
rx_freeCallQueue_lock, depending on whether an rx_call resides in the
rx_incomingCallQueue or the rx_freeCallQueue, respectively. This value
is used by rxi_ResetCall to lock the appropriate queue before removing a
call. Therefore, the call_queue_lock should be cleared after a call is
removed from a queue.
This issue has no known external symptoms; however, repairing this is
helpful to developers examining core files.
Repair two instances where the call_queue_lock is not cleared.
Change-Id: Id1d9ac8454c1e07c10766dffb2a2beac7122bf3e
Reviewed-on: https://gerrit.openafs.org/13641
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, afs_InvalidateAllSegments panics when afs_GetValidDSlot
fails. We panic in these cases because afs_InvalidateAllSegments
cannot simply return an error to its callers; we must invalidate all
segments for the given vcache, or we risk serving incorrect data to
userspace as explained in the comments.
Instead of panicing, though, we could simply sleep and retry the
operation until it succeeds. Implement this, retrying every 10
seconds, and logging a message every hour that we're stuck (in case
we're stuck for a long time).
When we retry the operation, do so in a background request, to avoid a
somewhat common situation on Linux where we always get I/O errors from
the cache when the calling process has a SIGKILL pending. Create a new
background op for this, BOP_INVALIDATE_SEGMENTS.
With this, the relevant vcache will be effectively unusable for the
entire time we're stuck in this situation (avc->lock will be
write-locked), but this is at least better than panicing the whole
machine.
Change-Id: Icdc58a94f0cd5857903836d94e5cf7814ce7e088
Reviewed-on: https://gerrit.openafs.org/13677
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Commit f0a3d477d6 restructured the
operation on tv_usec to avoid using undefined behavior, but in
the process introduced a behavior change. Historically (at least as
far back as AFS-3.3), we masked off the low nybble (four bits) of
tv_usec before adding the low byte (eight bits) of the rxi_getaddr()
output. Why there was a desire to combine two sources of input for
the overlapping four bits remains unclear, but restore the historical
behavior for now, as the intent of commit
f0a3d477d6 was to not introduce any
behavior changes.
Change-Id: Icb8bc1edd34ca29c3094b976436177b18bfc8d1d
Reviewed-on: https://gerrit.openafs.org/13759
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
We currently always pass DES as the requested enctype to
get_credv5_akimpersonate, but this means we will fail to use our
service princ if we're using another enctype (say, AES) with rxkad-k5.
To allow this to work with any enctype, just don't pass any requested
enctypes, and just use the enctype inside the 'entry' returned to us
from krb5_kt_get_entry.
Remove all of the logic associated with the now-unused
"allowed_enctypes" argument. Also remove the logic handling the case
where "service_principal" is NULL (since no callers pass a NULL
service_principal), to make it easier to take out the allowed_enctypes
related code.
Change-Id: Id11514ead26e15a287791c40509a001a1861df97
Reviewed-on: https://gerrit.openafs.org/13827
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
If we're creating tokens with -keytab and our AFS service principal is
afs@<cellname>, we'll first try creating tokens with
afs/<cellname>@<cellname> and krb5_kt_get_entry will fail with
KRB5_KT_NOTFOUND. Since we do not retry for KRB5_KT_NOTFOUND error, we
will not get tokens. So in order to get tokens for principal
afs@<cellname> we should retry for KRB5_KT_NOTFOUND error. Thanks to
jpjanosi@us.ibm.com for finding this issue and suggesting a fix.
Change-Id: I8af9df9876973badc4631f509eebcda46d667cef
Reviewed-on: https://gerrit.openafs.org/13826
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Introduce a small wrapper around osi_NetSend, called rxi_NetSend. This
small wrapper allows future commits to change the code around our
osi_NetSend calls, without needing to change every single call site,
or every implementation of osi_NetSend.
Change most call sites to use rxi_NetSend, instead of osi_NetSend. Do
not change a few callers in the platform-specific kernel shutdown
sequence, since those call osi_NetSend for platform-specific reasons.
This commit on its own does not change any behavior with osi_NetSend;
it is just code reorganization.
Change-Id: I0a7eb39d85d4e542c2832bb40191ab49fb02d067
Reviewed-on: https://gerrit.openafs.org/13717
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
In get_credv5_akimpersonate we use HAVE_ENCODE_KRB5_ENC_TKT which is not
defined, due to this we always return -1 from this routine for non
Heimdal case. We have a another define i.e
HAVE_ENCODE_KRB5_ENC_TKT_PART which is defined if
encode_krb5_enc_tkt_part function is present. In current code
encode_krb5_enc_tkt_part is called from krb5_encrypt_tkt_part and
krb5_encrypt_tkt_part is called from get_credv5_akimpersonate for non
Heimdal case. So we should change HAVE_ENCODE_KRB5_ENC_TKT to
HAVE_ENCODE_KRB5_ENC_TKT_PART.
Also while we're here, add a declaration for the internal function
encode_krb5_ticket, so we can build this newly-enabled code without
warnings.
Change-Id: I8f740e319ad279e284efaa407e6f92d0dc7a1bf6
Reviewed-on: https://gerrit.openafs.org/13825
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
An implementation limit of those lists was introduced in commit
a0ffea098d to prevent using unlimited
amounts of memory in ptserver and the client. Subsequent reports
indicate that the chosen limits are small enough to restrict
functionality currently in use at some sites where membership lists
exceed the current limit. Since this is just an implementation-
defined limit and can freely change from release to release, increase
the threshold by an order of magnitude to preserve functionality for
existing deployments while still retaining some protection against
attacker-controlled excessive memory allocation.
Change-Id: I857bb3b697909668eb71224b631dfbb7e3c03d3c
Reviewed-on: https://gerrit.openafs.org/13838
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit cc7f942a (LINUX: Disable kernel fortuna large frame errors)
added -Wno-error=frame-larger-than= to the CFLAGS for a file, but
older gcc (like 4.3.4 from SLES 11.x) does not support this flag,
causing a compiler error.
To avoid this, add a configure check for
-Wno-error=frame-larger-than=, and only use it if the compiler
supports it.
Thanks to mvitale@sinenomine.net for discovering the error.
Change-Id: I5486d2d4711f2c301be1cb79f0aaad69a22e9d3a
Reviewed-on: https://gerrit.openafs.org/13762
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 7620bd3348 (vlserver: fix
vlentryread() for old vldb formats) leaves the tail end of the
serverNumber, serverParition and serverFlags arrays uninitialized since
it only copies OMAXNSERVERS elements into arrays that have NMAXNSERVERS
elements.
Initialize the elements in the nvlentry server arrays that were not
copied with BADSERVERID.
Change-Id: I9533e3a40922c76d4179e0ada393103c2aa533dd
Reviewed-on: https://gerrit.openafs.org/13755
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
On WINNT, procmgmt_softsig.h exists to implement our opr softsig
routines in terms of procmgmt routines. Any time we include
opr/softsig.h in cross-platform code, we currently must also include
afs/procmgmt_softsig.h so we can build on WINNT. We currently do not
do this in src/xstat, causing build failures on WINNT.
To avoid this, just make opr/softsig.h include procmgmt_softsig.h
itself, so all of the opr/softsig.h users don't have to remember to do
this. Link xstat_*_test against procmgmt, so linking will succeed for
those tools.
Change-Id: I2dc8226d438be25cdccbe96474220d7c81ae25b9
Reviewed-on: https://gerrit.openafs.org/13824
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Inside get_credv5, client_principal is static so the first time
get_credv5 runs we'll allocate memory for it, and on subsequent calls
we'll reuse the same value.
However, if we call get_credv5_akimpersonate, we'll free
client_principal and never change what client_principal points to. If we
need to call get_credv5 again (because we need to retry getting creds),
we'll reuse the old value for client_principal, but since it points to
free memory we'll segfault or cause other problems.
To avoid this, change get_credv5 so we allocate the client and server
principals on each invocation of get_credv5 and free them before
returning from get_credv5. Since we free the client and server
principals inside get_credv5, remove freeing the client and server
principals inside get_credv5_akimpersonate.
Change-Id: Ie263aa2c03efc75e818d9007347dca9e42380dd4
Reviewed-on: https://gerrit.openafs.org/13761
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, the shutdown_*() code paths for several subsystems only
free the memory for that subsystem for "cold" shutdowns, and not for
"warm" shutdowns. This means the memory gets leaked during a "warm"
shutdown, since we never free these resources anywhere else.
Specifically, this happens in shutdown_bufferpackage, shutdown_AFS,
and shutdown_osinet.
To avoid these leaks for warm shutdowns, just move the
afs_cold_shutdown check around a little, so we free the relevant items
in either codepath.
Change-Id: I748311784f512b3e2f25bdcaa6629108a5790212
Reviewed-on: https://gerrit.openafs.org/13716
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
rxkad_get_ticket allocates 'v5cred' which should be freed when we
return from rxkad_get_token.
Change-Id: I09b20781f0856ab8e230e0af271e9d0c58fee90c
Reviewed-on: https://gerrit.openafs.org/13760
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, rx_serverQueueEntry structs are placed on the
rx_FreeSQEList linked list instead of being freed directly, but
managing this list is done a bit oddly. The first field in struct
rx_FreeSQEList is an opr_queue, but we don't use the opr_queue_*
macros to manage the list. Instead, we just assume the first field in
a struct rx_serverQueueEntry is a pointer that we can use to link
entries together. This is currently true and works, but it's an odd
way of maintaining such a list, and of course would break if we ever
moved the fields around in struct rx_serverQueueEntry.
Make this code more closely follow the normal way of managing
opr_queue lists, by using opr_queue_* macros, and changing
rx_FreeSQEList to be an opr_queue itself. Change the name to
rx_freeServerQueue to ensure all callers are changed, and to match the
naming convention for the other linked lists for rx_serverQueueEntry
structs. Also move rx_freeServerQueue and its associated lock
freeSQEList_lock to be declared static inside rx.c, since neither are
referenced outside of rx.c.
The general idea for this commit suggested by kaduk@mit.edu.
Change-Id: I2ea15af1ad3228fa5fdf9f323e9394838fba4bac
Reviewed-on: https://gerrit.openafs.org/13811
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 99418024 (libafs: Create $(DESTDIR)$(KMODDIR) on FBSD inst)
made it so we create the kmod installation dir before copying our
module into it. However, if we build a 'debug' variant of our module,
the FreeBSD build process also installs debug symbols in a different
directory, ${DESTDIR}${KERN_DEBUGDIR}${KMODDIR}, which may not exist.
So do the same thing for that dir too, if --enable-debug-kernel is
turned on, so the build still works.
To do this, introduce the LIBAFS_REQ_DIRS var, to make it easier to
keep track of which dirs we may need to create.
Change-Id: Id1ad72f6c19d5949d38ee97334b4014ae6ef16ad
Reviewed-on: https://gerrit.openafs.org/13690
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Commit 6b67cac4 (convert xstat and friends to pthreads) converted the
xstat utilities to pthreads, but we still need to explicitly pass
AFS_PTHREAD_ENV on WINNT to enable various pthread-specific code
paths. So give -DAFS_PTHREAD_ENV for our objects in this dir.
Change-Id: I222b99399a5fad3df528be2bc31823eb8bc52c62
Reviewed-on: https://gerrit.openafs.org/13823
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
tbutc uses pthreads, not LWP, so link it against mtafsutil.lib (a
pthread library), and not afsutil.lib (an LWP library).
Change-Id: Id29888d88bfdd9585e017217a9951eb645c65336
Reviewed-on: https://gerrit.openafs.org/13822
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 59d3a8b8 (vos: restore status information to 'vos status')
added the function rx_GetCallStatus to Rx, and used it in the
volserver, but didn't add the function to our .sym and .exp files,
causing a linker error on at least WINNT.
Add the function to the relevant .sym/.exp files, so we can link on
all platforms.
Change-Id: I859ac6d04d8a21eb6f8b4ba3f3720ca318e91334
Reviewed-on: https://gerrit.openafs.org/13820
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
ptclient.c contains a stub definition for osi_audit, but audit.c
already contains a real definition for osi_audit. libafsauthent
doesn't seem to actually need anything from ptclient (and the Unix
libafsauthent doesn't appear to use it), so just don't include
ptclient when linking libafsauthent.
Change-Id: I4172b80138e5ea121fc3ae2689cf4ed23c81e35b
Reviewed-on: https://gerrit.openafs.org/13819
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Since commit c43169fd (OPENAFS-SA-2018-001 Add auditing to butc server
RPC implementations), butc references symbols from audit. So add audit
to our libraries to link against, so we can link butc on WINNT.
Change-Id: I65f4d87085a8917c9b11d7c27b8e3902cd2a1c1c
Reviewed-on: https://gerrit.openafs.org/13818
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
We don't supply an implementation for opr_threadname_set for WINNT;
don't pretend that we do.
Change-Id: Ifa8042253d0aa10f365356d93cea3fad4686371a
Reviewed-on: https://gerrit.openafs.org/13817
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The current method of importing our ticket5 code from Heimdal has a
few issues:
- The der-protos.h file we generate contains numerous function
prototype declarations that looks like this:
ret-type func(parm-list, type */* comment */);
which cause numerous warnings on WINNT, because the '*/*' sequence
looks like the end of a nonexistent comment. This was previously
fixed manually in commit 8b5d3a73 (rxkad: remove warnings from
der-protos.h), but each time we regenerated our ticket5 code, the
same thing would happen.
- We manually insert an include for "asn1_err.h" in our v5der.c, and
the v5gen.c we pull in has an include for <asn1_err.h> inside it.
During a WINNT build, these can pull in different asn1_err.h files
(one from us, and one from the "Heimdal compatibility layer SDK" or
anything else in our include paths). Since the asn1_err.h in our
tree doesn't have an include guard, the code for both gets included,
which can cause various problems.
- Our current asn1_err.h file that we include is ultimately generated
by the awk-based compile_et from e2fsprogs, not the C-based
compile_et from Heimdal. This likely happened by accident because
the Heimdal build system uses the system compile_et by default. This
flavor of compile_et generates arguably inferior comerr-based header
files (they lack include guards, and they use #define constants
instead of enums).
Fix these issues with some edits to our README.v5 script:
- Apply a simple sed filter when we pull in der-protos.h to change
'*/*' into '* /*', to remove the relevant warnings.
- Instead of inserting an include for asn1_err.h into v5der.c in our
import script, just put it in ticket5.c, making it easier to see and
edit. Change this to <asn1_err.h> so it uses the same asn1_err.h as
in v5gen.c.
- Add a note to run the Heimdal build with COMPILE_ET=no, so the
Heimdal build system uses the in-tree compile_et, instead of
whatever is on the relevant system.
With these changes, redo the Heimdal import from the same version of
the Heimdal codebase.
Change-Id: I01e06f2799f1c828b8224c3425079b313ffb5b6b
Reviewed-on: https://gerrit.openafs.org/13816
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit b604ee7a (OPENAFS-SA-2018-002 kaserver: prevent KAM_ListEntry
information leak) added a memset in kamListEntry before COUNT_REQ, but
COUNT_REQ declares a local variable. This breaks the WINNT build,
because we must declare variables at the beginning of a block.
To fix this, just swap the two lines.
Change-Id: I47eb61e6f95c2e38c619e90c8f093de325892c63
Reviewed-on: https://gerrit.openafs.org/13815
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 83eec909 (Implement afsconf_GetRXGKKey) added a reference to
rx/rxgk_types.h inside cellconfig.p.h. Nothing ever added src/rxgk
WINNT makefiles, so that include file is never installed into
place, breaking the WINNT build when code tries to include
cellconfig.h.
To fix this and other code that needs rxgk header files, create an
NTMakefile for src/rxgk, which just exists to install headers into
place. Call it from the top-level NTMakefile right before copying in
the auth headers.
Change-Id: Id111479f55b4c330640e80d167a8af664fe3622e
Reviewed-on: https://gerrit.openafs.org/13814
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, in rx_GetCall when building for the kernel, if we notice
that we're shutting down (that is, if afs_termState has reached
AFSOP_STOP_RXCALLBACK), we return immediately. However, 'sq' may have
been allocated much earlier in this function, and if we return here,
we never free 'sq' or set it on any list.
Returning immediately is also unnecessary here; if we just 'break' out
of our wait loop, 'call' will still be NULL, and we'll break out of
the outer loop, and go through the rest of the function like normal.
The only difference is, if we 'break' instead of 'return'ing, we'll
put 'sq' on the free list before returning.
So, just 'break' out of the loop instead of returning, so we put 'sq'
on the free list and avoid leaking its memory.
Change-Id: Ibb2f4e697a586392f76ccdbbefdae8d75740f6fe
Reviewed-on: https://gerrit.openafs.org/13715
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 9ebff4c6 (OPENAFS-SA-2018-001 audit: support butc types) made
src/audit require the butc.h header, and updated Makefile.in to
reflect this. However, this dir is also built on WINNT, and the
NTMakefile was not updated to reflect this dependency. As a result, we
might fail to build src/audit on WINNT, since butc.h may not exist
yet, and we get an error like:
cl [...] /c audit.c
audit.c
cl : Command line warning D9025 : overriding '/W4' with '/W3'
audit.c(27) : fatal error C1083: Cannot open include file: 'afs/butc.h': No such file or directory
NMAKE : fatal error U1077: 'C:\PROGRA~2\MICROS~1.0\VC\bin\amd64\cl.EXE' : return code '0x2'
To fix this, move 'bubasics' to be made before 'audit' in NTMakefile,
so butc.h is available when we build 'audit'.
Change-Id: I2053db7cd95353cf6b703b4033239810338890aa
Reviewed-on: https://gerrit.openafs.org/13813
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Change afs_FreeOneToken to unlink the given token from its container,
instead of requiring its caller to do so. Rename the function to
afs_FreeFirstToken, to help indicate the change in behavior.
Also, while we are changing afs_FreeTokens to accommodate this change,
simplify afs_FreeTokens a little, making it resemble
afs_DiscardExpiredTokens a bit more.
[kaduk@mit.edu: add note about dead store elimination]
Change-Id: I0cf9d8b94236c736001a38cccfa7fdfff9f3e609
Reviewed-on: https://gerrit.openafs.org/13807
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
