gcc8 is more aggressive about parsing format strings and computing bounds
on the generated text from functions like snprintf. In this case it seems best
to detect cases of truncation and error out, rather than trying to increase
stack buffer sizes or switch to asprintf. These paths should be well-behaved
since they are local to the fileserver, so this is mostly about appeasing the
compiler's -Wformat-truncation checks to allow us to build with --enable-checking.
Change-Id: Id3f15e450c0f03143c0cc7e40186d5944a8fa3b4
Reviewed-on: https://gerrit.openafs.org/13463
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
clang thinks that 'time' can be used uninitialized:
bos.c:1472:9: error: variable 'time' is used uninitialized whenever 'if' condition is
false [-Werror,-Wsometimes-uninitialized]
if (as->parms[TIME].items) {
^~~~~~~~~~~~~~~~~~~~~
bos.c:1478:57: note: uninitialized use occurs here
if (!bos_ExecutableRestartTimeSet(bos_server, type, time, &st)) {
^~~~
bos.c:1472:5: note: remove the 'if' if its condition is always true
if (as->parms[TIME].items) {
^~~~~~~~~~~~~~~~~~~~~~~~~~~
bos.c:1445:5: note: variable 'time' is declared here
bos_RestartTime_t time;
^
but in this command description, the TIME argument is required.
Add a never-triggered error exit to appease the compiler when
--enable-checking is activated.
Change-Id: I38fac64fc5aba071f84f2f9e1b497df22df76f09
Reviewed-on: https://gerrit.openafs.org/13476
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
When char is signed, assigning 255 to a variable of type char changes
the value, which causes clang to emit a warning and fail the
--enable-checking build.
Change-Id: Id02e2526a9a9dd6657dee55b9dc22da03d102d8c
Reviewed-on: https://gerrit.openafs.org/13475
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
clang 7 notes that ~0 is signed and that left-shifting into the sign
bit is undefined behvaior. Use a new construction to clear the low
byte of tv_usec with only bitwise operations that are independent of
the width of tv_usec and stay within the realm of C's defined behavior.
Change-Id: I3e4f0fa4a8b8b72df23ef0c8ad7c4a229ac942f3
Reviewed-on: https://gerrit.openafs.org/13474
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
clang complains that these casts contain an incomplete function type
(since the function argument is omitted rather than declared to be
void). Since we just need the cast to pointer type, let the compiler
do it implicitly and pass stock NULL, rather than trying to force a
cast to function-pointer type.
Change-Id: I7f19f2936fe5425573c68fdd727ea90de02defd7
Reviewed-on: https://gerrit.openafs.org/13473
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
gcc does not benefit from our external knowledge that tm_year is
tightly bounded, and thinks it could still be in the range
[-2147481748, 2147483647], which would overflow our string buffer.
The function in question does not have error handling in place, so
rather than adding some or trying to assert the proper bounds, just
use a slightly larger buffer for safety.
Change-Id: Iafcba5588b805347ddcc0102969bd0e2a3173dd0
Reviewed-on: https://gerrit.openafs.org/13472
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Interestingly, even before this commit, the buffer size was larger
than what the kernel would accept. Since the kernel does its own
length checking, it's simplest to just allow slightly larger requests
here and have them fail later.
Change-Id: I9ed636e4ad025240cb27b3cc066a8f2a72959396
Reviewed-on: https://gerrit.openafs.org/13471
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Increase some buffer sizes to appease gcc8. While here, use snprintf
instead of plain sprintf(!).
Change-Id: I39d29522b92070ce2845ba3d392aaf2d97fc7b6e
Reviewed-on: https://gerrit.openafs.org/13468
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The "[dotted-quad] rxkad:name.inst@cell" construct can be as large as
(3*4+3)+7+3*64+2+1 == 217 characters (including trailing NUL); size
our buffer accordingly to avoid the risk of truncation.
Change-Id: Iee635aa66f5f639dfb0572c559a87b5313c305a9
Reviewed-on: https://gerrit.openafs.org/13466
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
When we're using old format compatibility, use OMAXNSERVERS for the
array lengths instead of MAXNSERVERS. Otherwise we'll try to copy more
data than we've read.
Detected by gcc8 as:
vlutils.c:183:2: error: ‘memcpy’ forming offset [149, 151] is out of the bounds [0, 148] of object ‘tentry’ with type ‘struct vlentry’ [-Werror=array-bounds]
memcpy(nbufp->serverFlags, oep->serverFlags, NMAXNSERVERS);
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vlutils.c:141:26: note: ‘tentry’ declared here
struct vlentry *oep, tentry;
^~~~~~
Change-Id: Ie720ca037c5a8bd6aaff5b6d5348161e0175b23b
Reviewed-on: https://gerrit.openafs.org/13465
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Make some formerly-64-character buffers VMAXPATHLEN (plus a smidgeon)
to give them space to hold the composed paths.
Change-Id: I403c822a8b7376d08fb29f0127315ec439a5cf0d
Reviewed-on: https://gerrit.openafs.org/13464
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Routines for constructing tokens (both regular and printed), extracting
and decrypting tokens, and helpers therein.
Provide the ability to print a token using a given session key and
using a random session key; the former is useful for certain variants
of localauth wherein a dummy GSS negotiation is performed with the
same identity acting as initiator and acceptor.
Include a paranoid sanity-check that only the routines intended to
produce printed tokens can produce tokens with a zero-length identities
list.
Change-Id: I0cde7fd0cdf9a27777523cd502b21bdccef41dcc
Reviewed-on: https://gerrit.openafs.org/10567
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
ptclient has been essentially disabled for pthreads since the ibm-1.0
release. Remove the conditionals to make a functional pthreaded
ptclient.
Change-Id: Ib0f60b3ab395827b73e5646b014e28ab09607e0e
Reviewed-on: https://gerrit.openafs.org/13500
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Move code to check the AFSCONF environment variable and read the
.AFSCONF files to separate functions.
Rename the internal functions afsconf_OpenInternal and
afsconf_CloseInternal to the more aptly named LoadConfig and
UnloadConfig in preparation for other changes. Add doxygen comments for
these functions.
Change-Id: Ie3361036c59c9e6ef99801891fff9fad63840344
Reviewed-on: https://gerrit.openafs.org/13397
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Newer updates to Solaris 11 have been including several changes to the
vnode struct. Since we embed a vnode in our struct vcache, our kernel
module must be recompiled for any such change in order for the openafs
client to work at all.
To avoid the need for this, switch Solaris to using a non-embedded
vnode in our struct vcache. Follow a similar technique as is used in
DARWIN and XBSD, where we allocate a vnode in osi_AttachVnode, and
free it in afs_FlushVCache.
Change-Id: I85fd5d084a13bdea4353b5ad9840fddbc45ce8c0
Reviewed-on: https://gerrit.openafs.org/12696
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
A few places were using vnodes and vcaches interchangeably. This is
incorrect, since they may not always be the same thing if we stop
embedding vnodes directly in vcaches Fix these to properly go through
AFSTOV/VTOAFS to convert between vcaches and vnodes.
Change-Id: I8a2e42d7b83a5374d2b16b19c47417e7f44d4f27
Reviewed-on: https://gerrit.openafs.org/12695
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Currently, our vnode op callbacks look like this:
int gafs_fsync(struct vcache *avc, afs_ucred_t *acred);
And a pointer to gafs_fsync is given directly to Solaris. This cannot
be correct, since 'struct vcache' is an OpenAFS type, so Solaris
cannot possibly give us a 'struct vcache'. The actual correct
signature for such a function is something like this:
int gafs_fsync(struct vnode *vp, afs_ucred_t *acred);
And then the 'gafs_fsync' function is supposed to translate 'vp' into
a vcache.
This works on Solaris right now because we embed the vnode as the
first member in our vcache, and so a pointer to a vnode is also a
pointer to a vcache. However, this would break if we ever change
Solaris vcaches to use a non-embedded vnode (like on some other
platforms). And even now, this causes a lot of warnings in
osi_vnodeops.c, since the function signatures are wrong for our vnode
callbacks.
So to fix this, change all of these functions to accept a 'struct
vnode', and translate to/from vnodes and vcaches appropriately.
Change-Id: Ic1c4bfdb7675037d947273ed987cacd05eddfc92
Reviewed-on: https://gerrit.openafs.org/12694
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Currently, many of the functions for our vnode ops are
forward-declared, right before they are referenced in the relevant
vnop template array. Move the function definitions to before the
references, so we can simply get rid of the forward declarations.
These functions are also all only referenced in this file, so declare
them 'static'.
Change-Id: Icd82b6d6176342e2576ce333b40c4b79e8c692c1
Reviewed-on: https://gerrit.openafs.org/12693
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Currently, the Solaris osi_vnodeops.c file forward-declares many of
its function definitions, but doesn't declare the arguments. For
example:
int afs_nfsrdwr();
This avoids type-checking for a few functions that are called before
they are defined in this file. Furthermore, many of these functions
are only used within this file, but are not declared 'static'.
To fix this weirdness, remove most of the forward declarations (most
are not referenced until the function is defined), and fully declare
the rest. Declare functions 'static' that are not referenced outside
of this file.
This commit only changes functions up to the 'afs_getsecattr'
definition. The rest of the file will be fixed in a future commit.
Change-Id: I3f58b9ad8e9c3ea8b3fe3dffacd5118eee0a7ff2
Reviewed-on: https://gerrit.openafs.org/12692
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
A comment in GenericAuth() refers to a "magic number" which used to be
present as:
*aindex = 2;
Commit d5622d03196762bd8a60404fea98b4bb044e076d made this a proper enum:
*aindex = RX_SECIDX_KAD;
Update the comment to remove mention of a "magic number".
No functional change is incurred by this commit.
Change-Id: I1d4770211fe4f88822426a9fe19db77bbb0d7738
Reviewed-on: https://gerrit.openafs.org/13490
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Bail with an error message to stderr if we are unable to format the
command syntax due to a string allocation error.
Found via scan-build.
[mmeffie: updated commit]
Change-Id: Ib3bc7f53c295d8dde6c07b9c4990cd1b3bcee58c
Reviewed-on: https://gerrit.openafs.org/13335
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In afs_linux_readdir, if we detect an error code from BlobScan,
currently we 'break' out of the current while() loop. But right after
this loop, we reset 'code' to 0, ignoring the error we just got from
BlobScan, and acting like we just reached the end of the directory.
This means that if BlobScan could not process the given directory at
all, we'll just fail to iterate through some of the entries in the
given directory, and not report an error.
To fix this, process errors from BlobScan like we do for
afs_dir_GetVerifiedBlob, and return an error code and log a message
about the corrupted dir.
Change-Id: I8bd628624ffc04fc55fd6a0820c73018bd9e4a18
Reviewed-on: https://gerrit.openafs.org/13430
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, all prdb write operations, except for SPR_Delete, will fail
with PRPERM if called by a non-system:administrators caller while
restricted mode is active. SPR_Delete is missing this check, and so
is not affected by the -restricted option.
Fix this by inserting the same check for -restricted as all other code
paths that check for -restricted.
Change-Id: I35f19d0b715423cd91769e6de845efa330368e50
Reviewed-on: https://gerrit.openafs.org/13374
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
We need one more byte for the trailing NUL.
Change-Id: I1379e958e3b5ec92802060c4541f419599e49311
Reviewed-on: https://gerrit.openafs.org/13462
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
According to the documentation, as well as other code paths that check
for -restricted, the -restricted option does not affect members of
system:administrators. Currently, though, AccessOK only bypasses the
-restricted check if the caller is SYSADMINID itself (i.e. localauth).
Fix AccessOK to only do the -restricted checks if the caller is not in
system:administrators, to match the documentation as well as other
ptserver operations.
Change-Id: I3074d4537845f1f4deb7f4b72cdb819391b617e3
Reviewed-on: https://gerrit.openafs.org/13373
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The function AccessOK is used by all of ptserver RPC handlers that
need to do an authorization check, and the last two arguments are set
as such:
- When adding a member to a group, 'mem' is PRP_ADD_MEM and 'any' is
PRP_ADD_ANY
- When removing a member from a group, 'mem' is PRP_REMOVE_MEM and
'any' is 0
- When modifying an entry (setFieldsEntry) or modifying some global
database fields, 'mem' and 'any' are both set to 0
- When reading an entry and not modifying it, 'mem' and/or 'any' are
set to other values (depending on if we're checking membership,
examining the entry itself, etc)
Commit 93ece98c (ptserver-restricted-mode-20050415) added a check to
AccessOK to make it return false for -restricted mode when we are
adding a member to a group, or when 'mem' and 'any' are both 0. This
didn't catch the case when we are removing a member from a group,
though, when 'mem' is PRP_REMOVE_MEM.
It looks like commit a614a8d9 (ptutils-restricted-accessok-20081025)
tried to fix this by adding a check for PRP_REMOVE_MEM, but it also
required 'any' to be set to 0 for the conditional to succeed. This is
true when removing a member from a group, but when adding a member to
a group, 'any' is PRP_ADD_ANY, and so this check fails.
This means that currently, when restricted mode is turned on,
non-admins can still run addToGroup and setFieldsEntry successfully.
Fix this by checking for PRP_ADD_MEM/PRP_REMOVE_MEM separately from
checking if 'mem'/'any' are set to 0. Break up this conditional into
separate if() statements with comments to try to make the checks
more clear.
Change-Id: I7e647865b772c42e70014f48ce9cd53ef511cd5b
Reviewed-on: https://gerrit.openafs.org/13370
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
dkms invokes a make clean command before and after building the kernel
module. The make clean that is issued at the start of building results
in a nuisance error message because the Makefile doesn't yet exist
Building module:
cleaning build area...(bad exit status: 2)
In the dkms.conf file, built from within the openafs.spec, change the
command defined in the CLEAN statement to test for the existence of the
Makefile prior to running the actual make clean
Change-Id: Ifc0d5eed6ef0cbc3ddfd193d27bbcb8a7cf52f2a
Reviewed-on: https://gerrit.openafs.org/13460
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Several places in the code currently call krb5_free_context(ctx) in a
cleanup code path, where 'ctx' may or may not be NULL. This is not
guaranteed to be okay, so check for NULL to make sure we don't cause
issues in these code paths.
While we are here cleaning up krb5_free_context() calls, also fix a
few call sites in afscp_util.c that were not calling krb5_free_context
in all error paths.
Change-Id: I881f01bdf94f00079f84c4bd4bcfa58998e51ac9
Reviewed-on: https://gerrit.openafs.org/13461
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
DRead currently returns ENOENT if we try to read a page beyond the end
of the given dir blob. We do this to indicate we've hit EOF, but we do
this even if the dir blob is completely empty (which is not a valid
dir blob).
If a dir blob in the cache is truncated due to cache corruption
issues, that means we'll indicate a normal EOF condition in that
directory for most code paths. If someone is trying to list the
directory's entries, for instance, we'll just return that there are no
entries in the dir, even though the dir itself is just invalid.
To avoid this for at least some cases, return an EIO error instead if
the dir blob is completely empty.
Change-Id: I8544e125ad12632523d7c514fe63ff9d87e1cd8f
Reviewed-on: https://gerrit.openafs.org/13429
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
SAFSVolRestore has a 'flags' argument, which the volserver passes on
to various internal functions, but the value of the flags never
actually changes any behavior. Remove the 'aflags' argument (and the
derived 'incremental' arg) from a few of our internal functions.
The relevant arguments have been unused since OpenAFS 1.0.
Change-Id: Ib6ba3d5d9aa3e29d720921cb32fe45c871cd803e
Reviewed-on: https://gerrit.openafs.org/13458
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Fix implicit declaration of function opr_softsig_Init() in scout.
Change-Id: I2bb9eb5240b053b2f16ef1f37035b01dbc42fb84
Reviewed-on: https://gerrit.openafs.org/13454
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Michael Meffie <mmeffie@sinenomine.net>
The file server stats are maintained in global static structures, which
are zero-ed on program start. The full contents are memcpy-ed to
allocated buffers as rx output arguments, so no uninitialized data is
sent over the wire. However, this commit converts the output buffer
allocation from malloc to calloc to make this more clear from code
inspection and make the code more robust.
While here, clean up the comments in SRXAFS_GetXStats and remove the
commented out code for a collection type which was never implemented.
Remove the comments about overwriting spare xstat values, which seems to
be a remnant from an early version of the code.
For informational purposes, add a note at the top of SRXAFS_GetXStats to
make it clear the CallPremable() is intentionally avoided in this
implementation of the GetXStats RPC. Apparently, the CallPremable() is
omitted since the OpenAFS file server does not to send callbacks to
clients issuing only GetXStats RPCs, and so also avoids sending TMAY
requests to clients like xstat_fs_test.
Note that the presumably older GetStatistics and GetStatistics64 do
unfortunately invoke CallPreamble(), so programs such as scout, must be
able receive RXAFSCB RPCs from OpenAFS file servers.
Change-Id: I7b90c7c6c561c74961fb7f7694a9576e1bed44d6
Reviewed-on: https://gerrit.openafs.org/13204
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Convert the xstat, fsprobe, and gtx libraries and test programs to
pthreads. Build these libraries with libtool.
Build the scout and afsmonitor programs with pthreads instead of LWP.
Change-Id: Ie1737e71b4e57735bf7b6c7dc3177d717ea35ac6
Reviewed-on: https://gerrit.openafs.org/12753
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit c4a127d0578e521b97131c5dedf9da58f71b0242
(ubik-clone-support-20010212) added changes to support ubik clone sites.
This commit added the afsconf_GetExtendedCellInfo function, which
returns the info given by the original afsconf_GetCellInfo, plus an
array of booleans (as chars) to indicate which cell servers are ubik
clones.
Unfortunately, the afsconf_GetExtendedCellInfo function calls the
afsconf_OpenInternal function on an already opened configuration. It
does so to look for server entries which are marked as clone sites in
the CellServDB file. Opening the already opened configuration leaks at
least the cellName and local realms information, and is generally
confusing.
Instead, remember which sites are designated as clone sites when the
CellServDB is read when the configuration is opened, and return that
info to the callers of afsconf_GetExtendedCellInfo.
This commit adds the clone array to the afsconf_cell structure and
changes to afsconf_GetCellInfo() for this new server-related data.
As part of this change, remove the no longer needed cell and clones
arguments to the internal function afsconf_OpenInternal, which were
added by commit c4a127d0578e521b97131c5dedf9da58f71b0242.
Update the testcellconfig test program to output the new afsconf_cell
clone member.
This leak was found with valgrind.
Change-Id: I73db60b6a4a77e620e0511ca45cc3418503278a4
Reviewed-on: https://gerrit.openafs.org/13396
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
The function _afsconf_FreeRealms, called by afsconf_CloseInternal, leaks
two afsconf_realms structures.
The function _afsconf_LoadRealms also leaks those two structures when it
fails.
These memory leaks were discovered with valgrind.
Change-Id: I1436ce21609951bc3433b6c91221cc45e78881bc
Reviewed-on: https://gerrit.openafs.org/13395
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
The determination of the CellServDB pathname is platform-dependent.
However, error reporting in the current code base assumes the CellServDB
location is platform-independent.
Add the pathname of the CellServDB file to the configuration directory
structure and set the new cellservDB member when opening the
configuration. Use this value when checking if the CellServDB has
changed and update the callers to use the cellservDB member when
reporting errors about the CellServDB file.
Change-Id: I5a3393fb9d4ae3c637d5a0d773598115314bfe1c
Reviewed-on: https://gerrit.openafs.org/13408
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
afs_CacheFetchProc currently has a section of code that looks like
this pseudocode:
if (!code) do {
while (length > 0) {
code = read_from_rx();
if (code) {
break;
}
code = write_to_cache();
if (code) {
break;
}
}
code = 0;
} while (moredata);
return code;
When we encounter an error when reading from rx or writing to the
cache, we break out of the current loop to stop processing and return
an error. But there are _two_ loops in this section of the code, so
what we actually do is break out of the inner loop, set 'code' to 0,
and then usually return (since 'moredata' is usually never set).
This means that when we encounter an unexpected error either from the
net or disk (or the memcache layer), we ignore the error and return
success. This means that we'll store a subset of the relevant chunk's
data to disk, and flag that chunk as complete and valid for the
relevant DV. If the error occurred before we wrote anything to disk,
this means we'll store an empty chunk and flag it as valid. The chunk
will be flagged as valid forever, serving invalid data, until the
cache chunk is evicted or manually kicked out. This can result in
files and directories appearing blank or truncated to applications
until the bad chunk is removed.
Possibly the most common way to encounter this issue is when using a
disk cache, and the underlying disk partition is full, resulting in an
unexpected ENOSPC error. Theoretically this can be seen from an
unexpected error from Rx, but we would have to see a short read from
Rx without the Rx call being aborted. If the call was aborted, we'd
get an error from the call to rx_EndCall() later on.
To fix this, change all of these 'break's into 'goto done's, to be
more explicit about where we are jumping to. Convert all of the
'break's in this function in the same way, to make the code flow more
consistent and easier to follow. Remove the 'if () do' on a single
line, since it makes it a little harder to see from a casual glance
that there are two nested loops here.
This problem appears to have been introduced in commit 61ae8792 (Unite
CacheFetchProcs and add abstraction calls), included in OpenAFS
1.5.62.
Change-Id: Ib965a526604e629dc5401fa0f1e335ce61b31b30
Reviewed-on: https://gerrit.openafs.org/13428
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In Kernel commit fb7fcc96a86cfaef0f6dcc0665516aa68611e736 the
current_kernel_time/current_kernel_time64 functions where renamed
and the calling was standardized.
According to the Linux Documentation/core-api/timekeeping.rst
ktime_get_coarse_real_ts64 is the direct replacement for
current_kernel_time64. Because of year 2038 issues, there is no
replacement for current_kernel_time.
Updated code that used current_kernel_time to use new name and calling
convention.
Updated autoconf test that sets IATTR_TAKES_64BIT_TIME as well.
Change-Id: I607bdcf6f023425975e5bb747e0e780b3d2a7ce5
Reviewed-on: https://gerrit.openafs.org/13434
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In Kernel commit e4b92b108c6cd6b311e4b6e85d6a87a34599a6e3 the
do_gettimeofday function was removed.
According to the Linux Documentation/core-api/timekeeping.rst
ktime_get_real_ts64 is the direct replacement for do_gettimeofday
Updated the macro osi_GetTime to use ktime_get_real_ts64 if it is
available.
Change-Id: I7fcd49958de83a6a040e40bd310a228247c481b2
Reviewed-on: https://gerrit.openafs.org/13433
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
The include for the ktime_get_coarse_real_ts64() autoconf test is
incorrect; ktime_get_coarse_real_ts64() has always been in linux/ktime.h
(via #include timekeeping.h), not linux/time.h.
This autoconf test still ran correctly because the OpenAFS build was
inadvertently picking up ktime.h via the default autoconf include path.
Therefore, this commit is needed only to provide documentation and
clarity to future maintainers.
Introduced as a cut-n-paste error (from the current_kernel_time test)
with commit 3c454b39d04f4886536267c211171dae30dc0344 for Linux 4.20.
Change-Id: I994b03a1700330756216c7feab0121c82d0f3ee4
Reviewed-on: https://gerrit.openafs.org/13437
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
In Kernel commit e262e32d6bde0f77fb0c95d977482fc872c51996
the mount flags (MS_) were moved from uapi/linux/fs.h to
uapi/linux/mount.h. This caused a compile failure in
src/afs/LINUX/osi_vfsops.c
The Linux documentation in uapi/linux/mount.h indicates that the MS_
(mount) flags should only be used when calling sys_mount and filesystems
should use the SB_ (super_block) equivalent.
src/afs/LINUX/osi_vfsops.c utilized the mount flag MS_NOATIME while
filling the super_block. Changed to use SB_NOATIME (which has the same
numeric value as MS_NOATIME) if available.
Change-Id: I2b2199de566fbadd45e857b37d24ce63002c7736
Reviewed-on: https://gerrit.openafs.org/13432
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The vos zap -force command does not remove the directories associated
with the volume in question (AFS_NAMEI_ENV). When the vos zap -force
command is executed, the volume server goes through the /vicep*/AFSIDat
directories and removes the files associated with the volume id received
as an argument. Unfortunately, the volume server does not remove the
directories associated with this volume. As a result, empty directories
are left behind.
To fix this problem, remove the empty directories left behind when vos
zap -force is executed.
Change-Id: I56fd52918223f87e424121bac6a086d7b0a46284
Reviewed-on: https://gerrit.openafs.org/12879
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
roken-post.h is a source file, not a generated file in the objdir.
Specify $(srcdir) so we can work with objdir builds.
Change-Id: I1d00ba1f28bea99770c2af56890fbf22ee764820
Reviewed-on: https://gerrit.openafs.org/13387
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
This fix corrects some annoying error and warning messages during
dkms install or uninstall.
Install:
DKMS: build completed.
openafs:
Running module version sanity check.
ERROR: modinfo: could not open /lib/modules/2.6.32-754.6.3.el6.x
86_64/weak-updates/openafs.ko: No such file or directory
- Original module
- No original module exists within this kernel
- Installation
- Installing to /lib/modules/2.6.32-754.6.3.el6.x86_64/extra/
Adding any weak-modules
WARNING: Can't read module /lib/modules/2.6.32-754.6.3.el6.x86_6
4/weak-updates/openafs.ko: No such file or directory
egrep: /lib/modules/2.6.32-754.6.3.el6.x86_64//weak-updates/open
afs.ko: No such file or directory
Remove
Status: Before uninstall, this module version was ACTIVE on this
kernel.
Removing any linked weak-modules
rmdir: failed to remove `.': Invalid argument
WARNING: Can't read module /lib/modules/2.6.32-754.6.3.el6.x86_6
4/weak-updates/openafs.ko: No such file or directory
egrep: /lib/modules/2.6.32-754.6.3.el6.x86_64//weak-updates/open
afs.ko: No such file or directory
openafs.ko:
- Uninstallation
- Deleting from:/lib/modules/2.6.32-754.6.3.el6.x86_64/extra/
- Original module
- No original module was found for this module on this kernel
- Use the dkms install command to reinstall any previous
module version.
Background:
Commit 1c96127e37c0ec41c7a30ea3e4aa68f3cc8a24f6 standardized the
location where the openafs.ko module is installed (from
/kernel/3rdparty to /extra/). The RPM Spec file was not updated to
build the dkms.conf file with the corrected location.
From the documentation for dkms
DEST_MODULE_LOCATION is ignored on Fedora Core 6 and higher, Red Hat
Enterprise Linux 5 and higher, Novell SuSE Linux Enterprise Server 10
and higher, Novell SuSE Linux 10.0 and higher, and Ubuntu. Instead,
the proper distribution-specific directory is used.
However the DEST_MODULE_LOCATION is still used saving and restoring old
copies of the module.
The NO_WEAK_MODULES parameter prevents dkms from creating a symlink into
weak-updates directory, which can lead to broken symlinks when
dkms-openafs is removed. The weak modules facility was designed to
eliminate the need to rebuild kernel modules when kernel upgrades occur
and relies on the symbols within the kABI. Openafs uses symbols that
are outside the kABI, and therefor is not a candidate for a weak module.
Change-Id: I52a332036056a359a57a3ab34d56781c896a2eea
Reviewed-on: https://gerrit.openafs.org/13404
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Modern versions `make` will not build the 'test' target since a
directory exists with the same name.
$ grep -C1 '^test:' Makefile
test:
cd test; $(MAKE)
$ make test
make: 'test' is up to date.
Declare these targets as .PHONY to force make to build the test programs
even when the 'test' directory is present. Also use '&&' to concatenate
commands instead ';' to avoid running the second command when the first
fails.
Change-Id: Id561d7610f80b87b59c632801fa0a4b216feb42d
Reviewed-on: https://gerrit.openafs.org/13419
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 55013a11 (lwp: Fix possible memory leak from scan-build) added
some free() calls to some otherwise-leaked memory. However, one of
these calls frees the 'stackmemory' pointer, which on AIX32 is not a
pointer from malloc/calloc, but calculated from reserveFromStack().
To avoid corrupting the heap, skip this free call on AIX32. This
commit adds another #ifdef to avoid this, which is unfortunate, but
this is also how the free is avoided in the existing code for
Free_PCB().
Change-Id: I6c4518f810e56c362ee744f250747fe8fc765b13
Reviewed-on: https://gerrit.openafs.org/13426
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The rx_atomic bitops were introduced with commit
1839cdbe268f4b19ac8e81ae78548f5c78e0c641 ("rx: atomic bit ops").
The last (only) reference to them was recently removed with commit
5ced6025b9f11fadbdf2e092bf40cc87499ed277 ("rx: Convert rxinit_status to
rx_IsRunning()").
Remove the now unreferenced bitops. This commit is comprised of partial
or complete reverts of the following commits:
ae4ad509d35 rx: fix rx_atomic warnings under Solaris (partial)
c16423ec4e6 rx: fix atomics on darwin (partial)
9dc6dd9858a rx: Fix AIX test_and_set_bit (complete)
1839cdbe268 rx: atomic bit ops (complete)
Note: The rx_atomic bitops for Linux systems are known to be broken due
to incorrect casting of rx_atomic_t into the unsigned long operand
expected by the native Linux bitops. The failure modes include silent
overruns on little-endian and incorrect results on big-endian. Do not
merely revert this commit in order to bring these bitops back into the
tree.
Change-Id: I6b63519f63d370ccc8df816b4388487909c17dcd
Reviewed-on: https://gerrit.openafs.org/13390
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, rx_GetStatistics assumes that struct rx_statistics and
rx_statisticsAtomic have the same size (we just memcpy between them).
However, this is never checked, and rx_statistics contains many 'int'
fields where rx_statisticsAtomic has rx_atomic_t fields.
If these are not the same size, our rx stats will silently break, so
add a static assert to make sure they are the same size.
Change-Id: I889867f4a85530c30dd15d32d1822144ea128a95
Reviewed-on: https://gerrit.openafs.org/13414
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
