Since the original IBM code import, there have been two versions of the
lock macros: one with INSTRUMENT_LOCKS (the default) and one without.
Only the former is ever enabled. Therefore, remove the unused latter
versions, and all INSTRUMENT_LOCKS conditional logic.
No functional change is incurred by this commit.
Change-Id: I154cd07e81ce7e9a2cc1bb4f0f93615de921e199
Reviewed-on: https://gerrit.openafs.org/14716
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Since OpenAFS 1.0, LockWait has been commented out, and the following
routines have been unreferenced:
- ObtainLock
- ReleaseLock
- afs_osi_SleepR
- afs_osi_SleepW
- afs_osi_SleepS
Remove them from the code and comments.
No functional change is incurred by this commit.
Change-Id: I480d023282878243f9a92f432d7bbec7028af70c
Reviewed-on: https://gerrit.openafs.org/14407
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Use a real call to File::Temp::tempfile to generate our temporary
filename, instead of creating a somewhat predictable filename based on
our pid.
Change-Id: Icb585c04e088b1fa37dffb6d92fc41cdb14874c7
Reviewed-on: https://gerrit.openafs.org/14799
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Our VOP_WRITE implementation causes the given data to be written to
the libafs cache, and potentially to the fileserver, but does not
update pages mapped to our vnode. This can cause file data to appear
out-of-date if otherwise valid pages exist for that range.
As a practical example of this, when running 'make buildworld' in
/afs, we run:
cc -nostdlib -Wl,-dc -r -o ipf.lo ipf_stub.o [...]
crunchide -k _crunched_ipf_stub ipf.lo
The first 'cc' command generates ipf.lo by writing to an mmap'd
region, and the second 'crunchide' command rewrites that file using
write() syscalls.
Afterwards, anything that reads from ipf.lo using mmap'd memory will
see the ipf.lo that 'cc' generated; anything that reads using read()
syscalls will see the updated version from 'crunchide'. And of course
if the pages are evicted for any other reason (such as memory
pressure), everything will see the updated version from 'crunchide'.
The eventual error seen during 'make buildworld' involves duplicate
symbols during linking, since 'crunchide' modifies most symbols to be
hidden. For example:
cc [...] -static -o rescue rescue.o cat.lo [...] ipf.lo [...]
ld: error: duplicate symbol: main
>>> defined at rescue.c
>>> rescue.o:(main)
>>> defined at count4bits.c
>>> ipf.lo:(.text+0x10)
ld: error: duplicate symbol: main
>>> defined at rescue.c
>>> rescue.o:(main)
>>> defined at trace.c
>>> routed.lo:(.text+0x3DE0)
[...]
To fix this, call vn_pages_remove() to invalidate the pages in the
given range after the write has gone through (successfully or not, in
case of partial writes or other edge cases). We don't do this lower in
afs_write(), since that is also called from our VOP_PUTPAGES()
implementation, and we'd be invalidating pages that we were just given
to write out.
Change-Id: I67708ae994da4a4c26edf32e545606a5238da4d0
Reviewed-on: https://gerrit.openafs.org/14166
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Although the volname string passed to FindByName() is currently always
limited 65 characters (including the terminating nul), to be on the safe
side, use the bounded strlcpy() function when coping the volname to the
temporary tname local variable to avoid the possibility of overwriting
the stack with an unbounded strcpy().
Change-Id: I12a8ca2901147c7dd88e63339d0d11c3c89bf94a
Reviewed-on: https://gerrit.openafs.org/14763
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>
When defining an xdr union with a 'default' arm, rxgen generates xdr
routines like this:
bool_t
xdr_myunion(...)
{
switch (objp->op) {
case FOO:
xdr_foo(...);
break;
default:
xdr_default(...);
break;
}
}
However, if the default arm of the union is just 'void;', we just
don't generate a 'default:' arm in the switch statment in the
generated routines. If there are enum values that are not explicitly
specified, and are handled by the default arm, this generates a
compiler warning (which breaks the build for --enable-checking):
foo_int.xdr.c:80:2 error: enumeration value 'BAR' not handled in switch [-Werror=switch]
switch (objp->op) {
To avoid this, change rxgen to always generate a 'default' arm in the
generated switch if there's one specified in the RPC-L. For a void
default, just generate an empty default arm, which avoids the compiler
warning.
Change-Id: I6ac457a4669439ef896b9cad6eb7de2f03068b69
Reviewed-on: https://gerrit.openafs.org/14798
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit "deorbit afsinstall" (ed15b0d5b3) removed afsinstall from the
source tree, but not did remove where it was referenced in
src/README.md.
Remove the reference to 'afsinstall' from the 'Not Maintained' section
in src/README.md.
Change-Id: Ie5226bf97003c21044759bd5dc2b8efe5d9138c2
Reviewed-on: https://gerrit.openafs.org/14805
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Remove trailing slashes from paths given to `fs lsmount` and `fs
flushmount`. This makes for a more pleasant user experience when shell
tab completion is used to specify the paths.
Thanks to Stephan Wiesand for pointing out this issue.
Change-Id: I756c4d7d9b3fe5cd41e02165caf1d6866a0210e6
Reviewed-on: https://gerrit.openafs.org/14779
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
With Linux 5.15-prerc1 printk is defined as a macro instead of a
function ("printk: Userspace format indexing support" 33701557)
This change is causing a build failure:
.../src/rx/rx_kernel.h:62:18: error: ‘printk’ undeclared (first use in
this function); did you mean ‘_printk’?
62 | # define osi_Msg printk)(
| ^~~~~~
The definition and use of the osi_Msg and osi_VMsg macros are
unconventional and the C preprocessor is not handling the macro
expansion when printk is itself a macro.
#define osi_Msg printk)(
...
(osi_Msg "%s", x);
Change osi_Msg to a function, and simply replace osi_VMsg with vprintf
since osi_VMsg is only used at one location within user space code.
osi_Msg is implemented in 2 locations, in rx_kcommon for kernel space
and in rx_user for userspace.
Note: The unconventional definitions of osi_Msg/osi_VMsg was historical
and due to older compilers not supporting variadic macros. All of
the currently support platforms should now support variadic functions.
Change-Id: I9f015e4929f2c5120e200d2b0378871e8d1375b3
Reviewed-on: https://gerrit.openafs.org/14791
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, when closing a vcache (via afs_close), we write any dirty
data to the net, and we do so via a background daemon if one is
available (for the rarely-used 'fs storebehind' functionality).
However, on FBSD, this breaks the locking rules, since flushing dirty
pages for a vnode requires the vnode to be locked. In the situation
where we are writing via a background daemon, the afs_close() thread
is what holds the vnode lock, but the background thread is the one
that's actually flushing the dirty pages. So the vnode is effectively
locked in this situation, but to the FreeBSD kernel it looks like we
are flushing pages without the vnode lock, and so it complains:
kernel: KDB: stack backtrace:
kernel: #0 0xffffffff80bf6557 at kdb_backtrace+0x67
kernel: #1 0xffffffff80c7a337 at assert_vop_locked+0x77
kernel: #2 0xffffffff811fc0bb at VOP_PUTPAGES_APV+0x7b
kernel: #3 0xffffffff80f016bd at vnode_pager_putpages+0x7d
kernel: #4 0xffffffff80ef424f at vm_pageout_flush+0xff
kernel: #5 0xffffffff80ee9b39 at vm_object_page_collect_flush+0x239
kernel: #6 0xffffffff80ee9819 at vm_object_page_clean+0x179
kernel: #7 0xffffffff828d7181 at osi_VM_StoreAllSegments+0x111
kernel: #8 0xffffffff82850a3d at afs_StoreAllSegments+0x9d
kernel: #9 0xffffffff8287b1ce at afs_StoreOnLastReference+0x17e
kernel: #10 0xffffffff8282dc70 at BStore+0xd0
kernel: #11 0xffffffff8282d6ec at afs_BackgroundDaemon+0x2cc
kernel: #12 0xffffffff828c2f2f at afs_syscall_call+0x35f
kernel: #13 0xffffffff82855963 at afs3_syscall+0x83
kernel: #14 0xffffffff81074581 at amd64_syscall+0x291
kernel: #15 0xffffffff8104cde0 at fast_syscall_common+0x101
kernel: vnode 0xfffff8006974fc58: tag afs, type VREG
kernel: usecount 2, writecount 0, refcount 3
kernel: flags (VI_ACTIVE)
kernel: v_object 0xfffff80023917900 ref 0 pages 171 cleanbuf 0 dirtybuf 0
kernel: lock type afs: EXCL by thread 0xfffff800852775e0 (pid 31828, ld.lld, tid 100787)
kernel: #0 0xffffffff80b81fc2 at lockmgr_lock_fast_path+0x1e2
kernel: #1 0xffffffff811fa9f6 at VOP_LOCK1_APV+0x96
kernel: #2 0xffffffff80c8c705 at _vn_lock+0x65
kernel: #3 0xffffffff80c8c8a3 at vn_close1+0x73
kernel: #4 0xffffffff80c8b76c at vn_closefile+0x4c
kernel: #5 0xffffffff80b571ba at _fdrop+0x1a
kernel: #6 0xffffffff80b5a3cc at closef+0x1ec
kernel: #7 0xffffffff80b577be at closefp+0x9e
kernel: #8 0xffffffff81074581 at amd64_syscall+0x291
kernel: #9 0xffffffff8104cde0 at fast_syscall_common+0x101
kernel: vc 0xfffffe002b8d4d30 vp 0xfffff8006974fc58 tag afs, fid: 1.536870912.13317022.33596982, opens 1, writers 1
kernel: states statd dirty
kernel: VOP_PUTPAGES: 0xfffff8006974fc58 is not locked but should be
To avoid this, force afs_close() to write dirty data in the same
thread, instead of passing it off to a background daemon.
Change-Id: Id952c7928c301fcc35d226e117dc19010b42776a
Reviewed-on: https://gerrit.openafs.org/14185
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The ptutil functions are defined to accept bounded character arrays for
user and group names. As of GCC 11, callers which provide the names as
string literals now trigger the stringop-overflow warning, since the
regions provided by the string literals are smaller than the bounded
areas.
error: ‘pr_ChangeEntry’ accessing 64 bytes in a region of size 1
[-Werror=stringop-overflow=]
note: referencing argument 4 of type ‘char *’
error: ‘pr_IsAMemberOf’ accessing 64 bytes in a region of size 22
[-Werror=stringop-overflow=]
note: referencing argument 2 of type ‘char *’
error: ‘pr_CreateUser’ accessing 64 bytes in a region of size 16
[-Werror=stringop-overflow=]
note: referencing argument 1 of type ‘char *’
error: ‘pr_Delete’ accessing 64 bytes in a region of size 16
[-Werror=stringop-overflow=]
note: referencing argument 1 of type ‘char *’
Update the callers in pts and testpt which pass literal strings. Instead
of passing char pointers to literal strings, assign the strings to
prname buffers and pass the prname buffers to the pr utility functions.
Change-Id: I7d8c67aa28d21bb6889ef92a2193a77b54c83cb1
Reviewed-on: https://gerrit.openafs.org/14769
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The CreateEntry() prototype has been fixed to match the function
definition, so callers are expected to provide bounded arrays for the
user or group name. Fix the InitialGroup() macro which is used to set
the built-in names using string literal to avoid stringop-overflow
warnings.
error: ‘CreateEntry’ accessing 64 bytes in a region of size 22 [-Werror=stringop-overflow=]
code = CreateEntry(tt, (name), &temp, /*idflag*/1, flag, SYSADMINID, SYSADMINID); \
note: in expansion of macro ‘InitialGroup’
InitialGroup(SYSADMINID, "system:administrators");
note: referencing argument 2 of type ‘char *’
note: in a call to function ‘CreateEntry’
CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], ...
(Repeated for "system:backup", "system:anyuser", "system:authuser",
"system:ptsviewers", and "anonymous".)
Change-Id: I7a37d4c8e191ffff52c2fdc1ed3783f4c3592b11
Reviewed-on: https://gerrit.openafs.org/14789
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, we free our CBR structures in shutdown_vcache, but later on
in shutdown_server, we call afs_FreeCBR on each one that's attached to
a struct server. afs_FreeCBR doesn't actually free the memory; it just
modifies some pointers to put the CBR on the free list. Since we do
this after the underlying memory has been freed, it can cause a panic
during shutdown since the structures are no longer valid.
To avoid this, make the afs_FreeCBR calls inside shutdown_vcache,
right before the memory is freed.
Change-Id: I142126d6aa811762b6c234d05abdac3764dad887
Reviewed-on: https://gerrit.openafs.org/14165
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, the FBSD afs_vop_putpages uses the credentials of the
current process for writing data back to the fileserver. Usually this
works, but sometimes we're being called from syncer(4), which runs as
root instead of as the user accessing the relevant file.
This means that instead of using the tokens from the user that wrote
to the relevant file, we use whatever tokens belong to uid 0. This
usually causes an EACCES error when trying to write the data back to
the fileserver, causing us to store EACCES (13) in avc->vc_error, and
possibly causing a message in the kernel log like so:
afs: failed to store file (0/13)
Since we set vc_error during these errors, this can also cause access
to the file to fail for the normal user process until vc_error is
cleared (such as when the file is closed).
To avoid this, store the credentials of the current user that
successfully opens the file for writing (in avc->cred), and use those
creds for writing back data to the fileserver. This is the same
approach that LINUX uses as of commit 70c8deab (Use user credentials
for Linux writepage()), and the NFS client code in FreeBSD itself (see
the usage of n_writecred in struct nfsnode as of 12.1-RELEASE).
Change-Id: I592d709b68d746bbdb326dfd7d012d6de829b905
Reviewed-on: https://gerrit.openafs.org/14164
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The CreateEntry() prototype does not match the function definition. The
aname parameter is declared as an unbounded array in the prototype but
is defined as a bounded array. As of GCC 12, a warning is issued for the
mismatch.
error: argument 2 of type ‘char[64]’ with mismatched bound
[-Werror=array-parameter=]
CreateEntry(struct ubik_trans *at, char aname[PR_MAXNAMELEN], ...
note: previously declared as ‘char[]’
extern afs_int32 CreateEntry(struct ubik_trans *at, char aname[], ...
Fix the prototype to declare the 'aname' parameter as a bounded array as
expected for this function.
Change-Id: I6d4dadcdd3f80c2b6f1b17670bbbc1e9e6076559
Reviewed-on: https://gerrit.openafs.org/14768
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The ubeacon_updateUbikNetworkAddress() prototype does not match the
function definition. The ubik_host parameter is declared as an unbounded
array in the prototype but is defined as a bounded array. As of GCC 12,
a warning is issued for the mismatch:
error: argument 1 of type ‘afs_uint32[256]’ {aka ‘unsigned int[256]’}
with mismatched bound [-Werror=array-parameter=]
ubeacon_updateUbikNetworkAddress(
afs_uint32 ubik_host[UBIK_MAX_INTERFACE_ADDR])
note: previously declared as ‘afs_uint32[]’ {aka ‘unsigned int[]’}
extern int ubeacon_updateUbikNetworkAddress(afs_uint32 ubik_host[]);
Restore the ubik_host array length in the function prototype, which was
dropped in commit 9020e6e2f0357b1082705dcaa6626573433969ec (ubik: Defer
updateUbikNetworkAddress until after RX startup).
Change-Id: I8189effc5b68ef8c1b45b4107f5e22e44ecf59fd
Reviewed-on: https://gerrit.openafs.org/14767
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The cfgutil_HostNameIsAlias() function has an output parameter called
isAlias, which is used when cfgutil_HostIsAlias() returns non-zero.
However, it possible for isAlias to not be set before returning. GCC 12
issues a warning about the possible use of the uninitialized isAlias
variable:
cfginternal.c:366:32: error: ‘isAlias’ may be used uninitialized
[-Werror=maybe-uninitialized]
Initialize the cfgutil_HostNameIsAlias() isAlias output flag to false.
Also, fix the misleading code indentation around the
cfgutil_HostNameIsAlias() call.
Change-Id: I68e66ae5f9019a613187321bb792d0505959ed30
Reviewed-on: https://gerrit.openafs.org/14772
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
All of these macros are not used anywhere in the tree; get rid of
them. Mostly this is just removing cruft, but removing
FDH_READ/FDH_WRITE/FDH_SEEK also helps make sure we don't accidentally
reintroduce code paths that use non-positional i/o.
No functional change should be incurred by this commit.
Change-Id: I9f6e885845e814cdb22d7c963e4ee6826cad8cea
Reviewed-on: https://gerrit.openafs.org/14761
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, the FBSD afs_vop_putpages() will call afs_write(), which
can then go through afs_DoPartialWrite -> afs_StoreAllSegments ->
osi_VM_StoreAllSegments -> vm_object_page_clean, which will then try
to go through afs_vop_putpages() again.
In this call stack, we're only calling afs_StoreAllSegments to reduce
our dirty cache chunks (afs_StoreAllSegments does nothing if we don't
have that many dirty cache chunks); we don't need to reduce dirty
pages, so calling osi_VM_StoreAllSegments is not needed.
To avoid calling osi_VM_StoreAllSegments in this situation, add a new
flag AFS_NOVMSYNC, which skips the osi_VM_StoreAllSegments call and
just flushes the dirty cache chunks. Use this flag in
afs_DoPartialWrite on FBSD, and slightly refactor the ifdefs to reduce
some code duplication.
Change-Id: Id6bf9a7f4ac6d5f09b0249ca6ed11674d556e909
Reviewed-on: https://gerrit.openafs.org/14163
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Clang-13 changed the default for the unused-but-set-variable resulting
in build warnings/errors with the following type of messages
vsprocs.c:3493:25: error: variable 'tentries' set but not used
[-Werror,-Wunused-but-set-variable]
afs_int32 nentries, tentries = 0;
The locations where these local variables are being flagged show that
while the variables are being updated, they are actually never used for
anything (e.g. used as part of an assignment to another variable, passed
as a parameter, used for as returned value, etc.)
Remove the variables being flagged by the clang-13 compiler.
Removal of these variables will not alter the overall functionality of
the code.
Change-Id: I521ea1323837e0d293e6f45eb8c15d5e05765f19
Reviewed-on: https://gerrit.openafs.org/14775
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>
Log a warning to the BosLog when the bosserver is not started in
restricted mode to encourage admins to enable restricted mode.
Also, log a notice when restricted mode is enabled to reassure admins
restricted mode is enabled on start up.
Change-Id: I2cbe0b95f44476d21215204f5ef186b07bba9b69
Reviewed-on: https://gerrit.openafs.org/14762
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Several static analysis tools have identified various problems:
- files left open from early exits/returns (infer)
- missing checks to ensure *alloc was successful (infer)
- possible memory leaks (scan-build, cppcheck)
To resolve the above problems:
- close files before exiting/returning
- add checks to ensure *alloc was successful before using the memory
- fix possible memory leaks by freeing memory
This commit is a reorganization of commits developed by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Change-Id: Iabebf46d2d1d8bb3bd2378cf2d1e7e4b94706bfd
Reviewed-on: https://gerrit.openafs.org/14676
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Ever since it was introduced in commit 8bc58a3d
(macos-10-6-support-20090216), rxdumptrace.c appears to have been
unused. While we do build a binary called rxdumptrace, we build it
from src/rx/rx_trace.c, not from rxdumptrace.c (which appears to be a
copy of the relevant portions of rx_trace.c).
Also remove rxdumptrace from src/rx/.gitignore, since the binary is
built in src/rxdebug/, not in src/rx/. While we're here, remove the
other entries in src/rx/.gitignore (rxperf and rxdebug), since those
are also built in different directories.
Change-Id: Ide816979168dcf70358f39db035ce16ef6c1dd17
Reviewed-on: https://gerrit.openafs.org/14748
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Since commit a62de618 (Build util tests properly with make check),
running 'make check' in tests/ also runs 'make check' in each of the
tests subdirectories, which builds the tests in that dir. (And the
same goes for 'make test' and 'make tests'.)
This does ensure that the tests are built before we run them, but it's
a bit strange to build the tests under 'make check', a target that
usually runs tests.
We do this in the top-level tests dir to make sure that the tests are
built, but this purpose is served by the existing 'make all' target.
So to reduce some duplication of logic, and reduce the number of
targets the subdirs need to implement, just have 'make check' depend
on 'make all', so we know the tests are built when we go to run them.
Change-Id: I2fcbe88daeeae94cd7ef7a4a8326c4b56fadee5a
Reviewed-on: https://gerrit.openafs.org/14636
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Both functions use the same logic to find a name in a given directory.
To avoid repetition, move this logic to a separate function called
afs_LookupName().
No functional change is incurred by this commit.
Change-Id: Ic5da09091e7050dd5f91adc7a413328edfb8292d
Reviewed-on: https://gerrit.openafs.org/14541
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, afstest_BuildTestConfig calls afstest_mkdtemp (our thin
wrapper around mkdtemp) to create its temporary config dir. We may
want to make new tests, though, that create a temp dir for other
purposes. To make that easier, move a little more code into
afstest_mkdtemp, so the caller doesn't need to construct the template.
To allow callers to clean up such temporary dirs, change
afstest_UnlinkTestConfig into a more general function,
afstest_rmdtemp. Allow this new function to remove all files in a dir,
not just files one-level-deep. To avoid needing to write our own
traversal and removal logic, just run 'rm -rf' via a new function,
afstest_systemlp().
Move these temp dir-related functions from config.c into files.c,
since they are no longer specific to config dirs.
Change-Id: I16750a2f30e98c9ca2e14dfb7d3fc9bc5d456e8d
Reviewed-on: https://gerrit.openafs.org/14632
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Add a new function (is_command), to help implementing tests that
involve running a command. This works like is_string(), except the
string value we compare against is the output of the provided command,
and we also check the exit code of the command.
Convert the existing execl() call in volser/vos-t to use this new
function.
Change-Id: I4a75b1a0333e608da6a6cd69838350116a2503a9
Reviewed-on: https://gerrit.openafs.org/14040
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, several of our tests contain logic to locate files (via
srcdir or objdir), based on the C_TAP_SOURCE/BUILD environment
variables. This logic is duplicated in several places, so consolidate
the code into a couple of new functions: afstest_src_path and
afstest_obj_path. Update all callers to use these new functions.
Change-Id: I67a5e5d7f8fd7a1edb55a45e52d877ac41f9a2ea
Reviewed-on: https://gerrit.openafs.org/14319
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Add a thin wrapper around asprintf, called afstest_asprintf (and
afstest_vasprintf), which does its own error checking. This just helps
makes tests a little less cluttered when needing to construct strings.
Adapt all asprintf callers in 'tests' to use the wrapper.
Change-Id: I6c4ae5b72af827e2c4c66ecfc57f152855b1d401
Reviewed-on: https://gerrit.openafs.org/14620
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, a few tests use the code in tests/common/ by linking
individual object files in there in addition to the test code (e.g.
linking ../common/config.o along with superuser-t.o).
This convention makes it very obnoxious to move code around in
tests/common/, since any users need to update their link lines. It
also makes it difficult for code in tests/common/ to make use of
functions in other tests/common/ files.
To fix this, just build all of the objects in tests/common/ into a
convenience library, called libafstest_common, and link the relevant
tests against that. Link a few requisite libraries (roken, rfc3961) in
libafstest_common, so each individual test doesn't need to link
against them.
Also link the TAP library itself in libafstest_common, so tests don't
have to explicitly link against it separately. To do this, convert it
into a libtool library, libafstest_tap.la.
Change-Id: I9c031c164efee20201336edcbfaff429e1d231b7
Reviewed-on: https://gerrit.openafs.org/14318
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Fix a few minor mistakes/typos that have crept up in viced over time:
- In PrintCallBackStats, struct FEs and struct CBs are 64 bytes, not
16 bytes. Show the actual size of the structs.
- A log message in cb_stateDiskEntryToFE is missing the trailing
newline.
- In cb_stateVerifyHCBList, a log message mentions the non-existent
cb_stateVerifyFCBList (instead of cb_stateVerifyHCBList)
Change-Id: I1146acae84d11980b6005cede9bb8bb53c54c70f
Reviewed-on: https://gerrit.openafs.org/14724
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
The rx_ackPacket.acks array (the SACK table) consists of up to 255
octets. Each octet stores either the value zero (RX_ACK_TYPE_NACK)
or one (RX_ACK_TYPE_ACK). Effectively only bit-zero of each octet
is used.
The rx_ackPacket.acks array cannot be enlarged but one possible
method of encoding the ACK/NACK state for packets when the
window size is greater than 255 is to use bits 1-7 of each
octet.
This change alters the test for ACK vs NACK to be a bit comparison
instead of a equality comparison. This change permits RX to be
compatible with any future use of bits 1-7.
No peer that treats the SACK table as bytes can ever send more
than 255 packets regardless of the advertised receive window.
Therefore, existing peers will never receive a SACK table with
more than 255 packets worth of bits.
Change-Id: I4dfcdc9ea18e060eeb257832297f557b7109e93a
Reviewed-on: https://gerrit.openafs.org/14465
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
afs_remunlink is called to delete a file on the fileserver after it
has been silly-renamed (due to being unlinked while the file is open).
Sometimes current->fs is NULL when this happens, typically when the
process is shutting down, after current->fs has been freed and file
handles are getting released. During afs_remunlink, we need to
interact with our cache files, and so we call dentry_open, which calls
security_file_open, which calls into the configured LSM for security
checks. Certain LSMs (e.g. Crowdstrike Falcon) will panic the kernel
in this situation if current->fs is NULL.
There's no way to skip the LSM hooks, or to flag to the LSM that we're
making an in-kernel VFS call, so the only way to avoid these panics is
to do our I/O in another thread. Fortunately, we already have a way to
defer afs_remunlink calls to a background daemon (CUnlinkedDel), since
we already do this in some cases (when someone else is holding
afs_xvcache or afs_xdcache).
So, to avoid the panic in the above scenario, defer calls to
afs_remunlink to a background daemon using CUnlinkedDel when
current->fs is NULL, and we're using a disk cache.
More details on this issue is discussed at following thread:
https://lists.openafs.org/pipermail/openafs-info/2021-March/043073.html
Change-Id: I1ee83d088a9b661d2974ce24b36bfd9f6ea4e7e9
Reviewed-on: https://gerrit.openafs.org/14691
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Tested-by: Jonathan Billings <jsbillings@jsbillings.org>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Our 'struct list' collides with a 'struct list' defined in
/usr/include/grp.h on AIX (at least on AIX 7.2). This is a very
generic structure name, so rename it to rxgen_list to avoid issues
like this.
Change-Id: Ic6d3a1fcde5e7f8a1ae57b974f711fb844f29f3f
Reviewed-on: https://gerrit.openafs.org/14702
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
A couple of places in src/volser currently have some logic to get the
size and blocksize of a file. The existing logic is nontrivial due to
platform-specific quirks, and ignores afs_fstat errors.
To fix these issue and consolidate the code into one place, introduce
a new function, FDH_BLOCKSIZE, which gets the file size and blksize.
Update the places in src/volser to use the new function.
Change-Id: I4daeec84c8fdb5756a8d6a7f477d0045a19a8fe9
Reviewed-on: https://gerrit.openafs.org/14662
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
AIX 7.2 doesn't seem to have libcsys, netinet.exp, sockets.exp, or
statcmd.exp available. To allow for our IMPORTS and LDFLAGS to change
depending on the AIX version, introduce the autoconf vars TSM_IMPORTS
and TSM_LIBS, which do not contain the missing libs on AIX 7.2, so we
can build on AIX 7.2.
Change-Id: I95feb22f8e2b35948d2024d5c29b917d064f30f3
Reviewed-on: https://gerrit.openafs.org/14703
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, tests/rx/perf-t calls functions like WIFSIGNALED and
WTERMSIG on $?. However, functions like WTERMSIG expect the native
exit status code (that is, ${^CHILD_ERROR_NATIVE}). The $? var (aka
$CHILD_ERROR), is a synthetic value calculated by perl that stores the
term sig in the lowest 7 bits, and the exit code in the second-lowest
8 bits.
For most modern platforms, these two values tend to be the same. But
on modern AIX (and some other weird platforms, like VMS and BeOS), the
exit status integer is encoded differently. On AIX specifically, the
term sig is in the third-lowest 8 bits, so a process exiting on signal
15 would result in an exit status (${^CHILD_ERROR_NATIVE}) of 0xf000f,
but $? would be just 0xf. Calling WTERMSIG on 0xf000f returns 0xf, but
calling WTERMSIG on 0xf returns 0x0.
All of this means that running rx/perf-t causes the final test to fail
with "Server died with signal 0" (even when the process was killed by
signal 15), which is rather confusing.
To fix this, call WTERMSIG et al with ${^CHILD_ERROR_NATIVE} instead
of $?. Create a local var so we don't need to spell out
${^CHILD_ERROR_NATIVE} so many times.
Change-Id: I3c27642fcaf17c320a94caf57d3665d4b6a4a76e
Reviewed-on: https://gerrit.openafs.org/14706
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
On AIX, calling sigwait() on a sigset containing SIGWAITING (or
SIGKILL or SIGSTOP), causes sigwait to return with an EINVAL error.
Currently, signalHandler() calls sigwait() with SIGWAITING, because
SIGWAITING is in the sigset returned by softsigSignalSet(). And so,
sigwait() returns EINVAL, and our opr_Verify in signalHandler() fails,
causing a crash.
To avoid this, remove SIGWAITING from the sigset in
softsigSignalSet(). This is AIX-specific, since the SIGWAITING signal
is AIX-specific.
Change-Id: I42539d859212605e6b1aaeb6a5eecb0ad034cea5
Reviewed-on: https://gerrit.openafs.org/14705
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Every va_start call is supposed to be paired with a matching va_end
call, but 'checkList' is missing any call to va_end. This doesn't
currently seem to cause any visible problems, but add the matching
va_end to avoid potential future issues.
Change-Id: I569618823f39f4da5b1787cc49c5509d8ea37528
Reviewed-on: https://gerrit.openafs.org/14704
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
2.6.18 is the minimum supported linux kernel level. There are
preprocessor checks for specific kernel versions that predate 2.6.18.
Refactor the use of the LINUX_VERSION_CODE that checks for kernel
versions older than 2.6.18.
In addition, there are a couple of locations where the kernel version
is checked with an additional test to see if module_param or
module_param_array are defined. These checks are used to determine
whether to use the macros module_param/module_param_array or to use
the macro MODULE_PARM. The macros module_param and module_param_array
were present in Linux prior to 2.6.12. The macro MODULE_PARM was marked
as deprecated prior to 2.6.12 and removed in Linux 2.6.17 (8d3b33f67fd
'[PATCH] Remove MODULE_PARM').
Remove the preprocessor checks for module_param/module_param_array and
remove the use of MODULE_PARM.
The updates should have no functional changes.
Change-Id: I1dc5dca1252abfc865917757989df235c75059a6
Reviewed-on: https://gerrit.openafs.org/14389
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 6329a523f6305541871bc3d1694065e7b17abe77 changed all
occurrences of AFS_*LINUXnn_ENV to AFS_*LINUX_ENV, but did not perform
any refactoring of the use of these variables.
This commit completes the task by refactoring the preprocessor
conditionals that involved removing dead code or collapsing statements.
The updates should have no functional changes.
Change-Id: I10fbefb7c862de2ec35ab77cafa2391e6e59f388
Reviewed-on: https://gerrit.openafs.org/14388
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
The minimum Linux kernel that is now supported is linux-2.6.18. The
Linux versioned preprocessor macros AFS_*LINUXnn_ENV are no longer
needed to distinguish the different levels of Linux and can be merged
into just a single set of macros.
Perform a global change of _LINUX\d+_ENV to _LINUX_ENV. e.g.
AFS_LINUX24_ENV -> AFS_LINUX_ENV
AFS_USR_LINUX24_ENV -> AFS_USR_LINUX_ENV
AFS_AMD64_LINUX20_ENV -> AFS_AMD64_LINUX_ENV
Replace the multiple definitions for the versioned 'AFS*_LINUXnn_ENV'
with just single non-version definitions 'AFS*_LINUX_ENV'.
Apart from replacing the now-redundant #define directives and tidying up
a few comments at the close of a preprocessor block to match their
current form, this commit was done using a mechanical change of the
variable names and did not reduce preprocessor statements that could now
be combined or eliminated. Nor does this commit remove dead code. A
follow-up commit (Cleanup AFS_*LINUX_ENV usage) will handle these
changes.
The updates should have no functional changes.
Change-Id: I71e32ca9818d28f82b4f23679868d1b9a62c44bd
Reviewed-on: https://gerrit.openafs.org/14387
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Currently, many routines in 'fs' cause afsconf_GetCellInfo to be
called for the given cell, which causes an AFSDB/SRV lookup for the
given cell if the cell has no dbservers specified in the local
CellServDB. Sites often define such CellServDB records to indicate
that the given cell exists, but the dbserver IPs should only be looked
up via DNS.
However, 'fs' is only calling afsconf_GetCellInfo in order to
canonicalize the cell name (we're allowed to give an abbreviated name
for a cell on the command line if the abbreviation is unambiguous, but
we want to give the full name to the kernel). We don't care about the
other cell info at all, so triggering a DNS lookup is completely
unnecessary.
If our DNS server is not responding, e.g. because we've lost network
access entirely, this causes the relevant 'fs' commands to fail,
possibly after a long delay. Some fs commands such as 'fs setcell' are
often run during client startup, and can still otherwise run fine
without network access, and so such failures are unnecessary.
To fix this, we introduce a new function, called afsconf_GetCellName,
which only returns the full name for a cell, and does not require a
DNS lookup for such "empty" cells in the local CellServDB. For all
code paths in 'fs' that just need the cell name (which is all of them
besides 'fs mkmount'), use this new function.
Change-Id: I45eb1bdc6ba3e3b4653be2306dcbf79c1015724c
Reviewed-on: https://gerrit.openafs.org/13540
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 0d6b9defb36cb94f3d34b058f00055e9e99d85fc added a check to avoid
overrunning a buffer when the cell name returned by RXAFSCB_GetCellServDB
is too large for the libadmin buffer, but in that case missed freeing
the server list returned in the same rpc.
Move the xdr_free() of the server list under the cleanup label so the
server list returned by the RPC will always be freed.
Thanks to Mark Vitale for finding this bug.
Change-Id: I4c7228f8c8d48e267f6fbb01cb89fdf5232de97e
Reviewed-on: https://gerrit.openafs.org/14690
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Commit 248da50aa56f19bdc8b2b322f5e17b3d2a363dce 'icl 64 bit platform
rationalization' introduced support for 64-bit ICL_TYPE_POINTER and
ICL_TYPE_LONG for fstrace running on DARWIN kernels. However, it
neglected to make the matching change in the fstrace utility itself,
which reads and reports these trace entries. The result is that all
fstrace records which contain 64-bit pointers or longs are misreported
as 32-bit values. Furthermore, any subsequent values in the same
fstrace record are also misreported because the offsets are now
incorrect.
Move the definition of ICL_LONG from afs_icl.c to icl.h so fstrace.c may
share the ICL_LONG logic and value.
Modify fstrace to use logic similar to the recording logic in afs_icl.c
so that the correct size and offsets are maintained while decoding the
contents of each fstrace record.
We can use the build-time value of ICL_LONG (rather than the runtime
value of afs_sizeofLong) because the difference only matters for SGI62
32-bit kernels. It is unknown whether the existing code works correctly
for SGI62 32-bit mode, but this commit should not affect that support
either way.
Change-Id: I240f76fed4618822b774451c5184b1160f17221d
Reviewed-on: https://gerrit.openafs.org/14558
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
SIGFPE is normally a synchronous signal, which would not be handled by
opr softsig. However, bosserver uses SIGFPE as an asynchronous signal
to modify the local restricted mode. So allow SIGFPE in softsig
handlers, so bosserver can use softsig.
Change-Id: I413edbed967babb3e60c6fa97f89b40b8d7b3ad7
Reviewed-on: https://gerrit.openafs.org/14080
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Almost all of our opr locking primitives assert that they did not
receive an error from pthreads. opr_cv_timedwait is the exception to
this, since it needs to return two possible codes that aren't
internal failures: 0 and ETIMEDOUT. As a result, most callers assert
that our returned code is one of these two values.
To make opr_cv_timedwait act more like our other locking primitives,
removing the need to add opr_Assert()s everywhere, change
opr_cv_timedwait to check that the returned code is 0 or ETIMEDOUT,
and remove the relevant asserts from its callers.
Change-Id: Ie9a62f2edb23969d66e4ed821af37077bc6400c4
Reviewed-on: https://gerrit.openafs.org/14079
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Several callers use OS_SIZE(fdP->fd_fd) instead of just calling
FDH_SIZE(fdP). Right now these calls are equivalent, but change all of
these callers to use FDH_SIZE, to make it easier to possibly change
the behavior of FDH_SIZE in the future.
Change-Id: If846c160c71f11935de8c0e4634afa2c7f7ee53f
Reviewed-on: https://gerrit.openafs.org/14657
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
