openafs.git
2 years agovol: Blank fake linkHandles 56/14656/2
Andrew Deason [Sat, 17 Feb 2018 22:57:12 +0000]
vol: Blank fake linkHandles

A couple of places in namei_ops.c use "fake" linkHandle FdHandle_t's.
Make sure these are initialized, so we don't accidentally use
uninitialized fields in the FdHandle_t.

Currently, the code never uses any fields that are not explicitly set
in this same code path, but this seems rather fragile, and future
commits may change the fields in an FdHandle_t.

Change-Id: I9372f03a4d3b68afd3ab24a27ae669cba92e0abe
Reviewed-on: https://gerrit.openafs.org/14656
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agofstrace: add dump -debug option 57/14557/3
Mark Vitale [Tue, 9 Mar 2021 22:20:22 +0000]
fstrace: add dump -debug option

As a debugging aid, add a -debug option to the dump subcommand to
display each trace record in raw hex format as well as the normal
decoded format.

Change-Id: I80dd675a07e048e25749a9afb584515effcbc08a
Reviewed-on: https://gerrit.openafs.org/14557
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoafs: Free pioctlToken in extractPioctlToken 51/14651/4
Andrew Deason [Thu, 17 Jun 2021 22:15:13 +0000]
afs: Free pioctlToken in extractPioctlToken

Ever since it was introduced in commit 5ec5ad5 (New GetToken pioctl),
extractPioctlToken has incorrectly freed pioctlToken by passing
'&pioctlToken' to xdr_free (instead of 'pioctlToken').

This causes xdr_ktc_tokenUnion to interpret &pioctlToken (which is a
struct ktc_tokenUnion **) as a struct ktc_tokenUnion *. This doesn't
cause any corruption or panics, since ktc_tokenUnion doesn't contain
any freeable fields unless its at_type is 2 (AFSTOKEN_UNION_KAD). So
as long as the bogus 'at_type' from the misinterpreted pointer is not 2,
the xdr_free call will just not free anything (and return an error,
which we ignore).

If the bogus at_type is 2, this would probably cause some memory
corruption or other nastiness. For this to happen on 32-bit systems,
the value of the 'pioctlToken' pointer itself would need to be 0x2.
On 64-bit systems, the top or bottom 32-bits of the pointer would need
to be 0x2 (depending on endianness). Those situations seem impossible
or very unlikely on most systems, and have never been seen in the
wild.

FIXES 135238

Change-Id: Id14571d090570cfacfa920048f41c3b1e434f31c
Reviewed-on: https://gerrit.openafs.org/14651
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>

2 years agocmd: version command should not call before or after procs 45/14645/3
Mark Vitale [Tue, 22 Jun 2021 03:29:09 +0000]
cmd: version command should not call before or after procs

Since the original IBM code import, the cmd_Dispatch processor has
avoided calling beforeProc and afterProc for the built-in 'help' and
'apropos' commands.  This is important, because these procs may rely on
application-defined common arguments (e.g., -cell <cell> or -noauth) for
proper operation.  However, application common arguments are not defined
for built-in commands 'help' and 'apropos'.

Unfortunately, when a new built-in 'version' command was added
(350c140d89198cb7 libcmd-support-version-switch-20060630), it was not
exempted from the before and after procs.

In order to avoid a segmentation fault in 'libadmin/test/afscp
version', modify cmd to also avoid calling beforeProc and afterProc for
the 'version' command.

Change-Id: Ic899397de808fd6c4a5f66a0f5add102340064e6
Reviewed-on: https://gerrit.openafs.org/14645
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 years agobos: Let xdr allocate rpc output strings 50/14650/5
Michael Meffie [Wed, 23 Jun 2021 00:02:18 +0000]
bos: Let xdr allocate rpc output strings

The bos client provides fixed sized buffers on the stack for RPC output
strings instead of letting xdr allocate output strings.  Unfortunately,
the fixed sized buffers do not account for the terminating nul char when
the output string is the maximum size defined for the bozo RPCs.

To avoid potential buffer overflows, and to allow for larger xdr string
sizes in the future, convert these to xdr allocated strings. Be sure to
always free the xdr allocated strings.

The following bozo RPCs have xdr output strings, and are addressed in
this commit:

BOZO_EnumerateInstance
BOZO_GetCellHost
BOZO_GetCellName
BOZO_GetInstanceInfo
BOZO_GetInstanceParm
BOZO_GetInstanceStrings
BOZO_GetStatus
BOZO_ListSUsers

Thanks to Cheyenne Wills for pointing out this issue.

Change-Id: I7fb48ff6766420a2d84723e1663ff24e86313ccf
Reviewed-on: https://gerrit.openafs.org/14650
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoihandle: Remove FDH_READV/WRITEV references 55/14655/2
Andrew Deason [Wed, 27 May 2020 23:06:24 +0000]
ihandle: Remove FDH_READV/WRITEV references

FDH_READV and FDH_WRITEV have been unused since commit
335ccb40 (ihandle positional read and write). Remove a couple of
lingering references to them.

Change-Id: I640d411782440958a823ef0e2aa810bddd8ceee2
Reviewed-on: https://gerrit.openafs.org/14655
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 years agonamei: Calculate offset before use in GetFreeTag 54/14654/2
Andrew Deason [Fri, 29 May 2020 19:22:34 +0000]
namei: Calculate offset before use in GetFreeTag

We pass 'offset' to FDH_LOCKFILE here, but 'offset' is calculated a
few lines below. The compiler doesn't catch this, since FDH_LOCKFILE
is a macro that ignores the second argument (except on WINNT).

Just move the calculation a few lines above, to ensure 'offset' is set
before it's actually used, to avoid issues if and when FDH_LOCKFILE is
changed to use the second argument.

Change-Id: If858005cb83370ca905f93f9d3d7eed2941ddf99
Reviewed-on: https://gerrit.openafs.org/14654
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 years agoviced: remove dead code from h_Alloc_r 35/14335/2
Mark Vitale [Fri, 28 Aug 2020 19:16:01 +0000]
viced: remove dead code from h_Alloc_r

These two lines have been ifdef'd out since commit 8f2df21ffe59
pull-prototypes-to-head-20020821.  Remove them.

No functional change is incurred by this commit.

Change-Id: I5d1d97d285ad50777bc5a26a53d6e0357140941d
Reviewed-on: https://gerrit.openafs.org/14335
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agolibadmin: Let xdr allocate rpc output strings 26/14626/4
Michael Meffie [Fri, 21 May 2021 16:38:01 +0000]
libadmin: Let xdr allocate rpc output strings

In most functions, the libadmin library provides fixed sized buffers for
RPC output strings instead of letting xdr allocate the output string.
Unfortunately the fixed sized buffers do not account for the terminating
nul char when the output string is the maximum length possible for the
xdr output strings.

To avoid potential buffer overflows, and to allow for larger xdr string
sizes in the future, convert these to xdr allocated strings and use safe
string functions to copy the results to the application buffers. Fail
with an error if the application buffer is too small, instead of
overflowing the buffer or truncating results.

Thanks to Cheyenne Wills for pointing out this issue.

Change-Id: I963e1b790417863c036e897811c86a634d1d4e7f
Reviewed-on: https://gerrit.openafs.org/14626
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 years agoutil: remove dead code KTimeCmp 34/14334/4
Mark Vitale [Fri, 28 Aug 2020 19:12:33 +0000]
util: remove dead code KTimeCmp

This routine has been dead code since the original IBM code import.  It
was ifdef'd out in commit 45376df63f7f util-warning-cleanup-20011005.

Remove the dead code from the tree.

No functional change is incurred by this commit.

Change-Id: I33689db8e016e1dd5abb88910af8da6208a75ce6
Reviewed-on: https://gerrit.openafs.org/14334
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 years agorxgen: remove dead code deverbatim 33/14333/2
Mark Vitale [Fri, 28 Aug 2020 19:09:17 +0000]
rxgen: remove dead code deverbatim

This routine has been dead code since the original IBM import.  It was
ifdef'd out in commit 8f2df21ffe59 pull-prototypes-to-head-20020821.

Remove it from the tree.

No functional change is incurred by this commit.

Change-Id: Ie81bd8ed1764026e872dcf07e0b12e3c8a31c64d
Reviewed-on: https://gerrit.openafs.org/14333
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agorx: remove dead code xdrrx_getpos, xdrrx_setpos 32/14332/2
Mark Vitale [Fri, 28 Aug 2020 19:01:50 +0000]
rx: remove dead code xdrrx_getpos, xdrrx_setpos

These routines have been dead code since the original IBM code import.
They were ifdef'd out in commit 1d93f2da22eb
'rx-warnings-and-prototyping-20010623'

Remove them from the tree.

No functional change is incurred by this commit.

Change-Id: I958c07487e2453690535acd01b92c59fcecd1189
Reviewed-on: https://gerrit.openafs.org/14332
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agolibafsauthent: export pr_IsAMemberOf 77/14377/2
Benjamin Kaduk [Wed, 30 Sep 2020 04:47:55 +0000]
libafsauthent: export pr_IsAMemberOf

Reported by Alexander Chernyakhovsky <achernya@mit.edu>

Change-Id: I0e0282d89a62ab577ceba3d10aa2ee2819c3f882
Reviewed-on: https://gerrit.openafs.org/14377
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoklog.krb5: remove "save and reuse password" logic 43/14643/2
Marcio Barbosa [Sun, 20 Jun 2021 03:52:36 +0000]
klog.krb5: remove "save and reuse password" logic

In order to avoid repeated requests for the user's password, klog.krb5
caches and reuses it whenever necessary. However, with the introduction
of commit 3a9a5783cd1fd73902655f0876e2069b42688c94, klog.krb5 always
requests a TGT regardless of the state of writeTicketFile, eliminating
the need for possible extra requests for the user's password.

Moreover, krb5_get_init_creds_password() does not accept a custom
prompter on macOS (returns EINVAL). Consequently, if the -password
argument is omitted, klog.krb5 fails with the following error:

klog: Invalid argument Unable to authenticate in realm <REALM>

Given that the user won't be prompted for a password multiple times,
remove the "save and reuse password" logic and use krb5_prompter_posix()
as the prompter function (instead of klog_prompter).

Relevant issue identified by gangovind@in.ibm.com.

Change-Id: Ia994a18d1d166893f12ee78d3e6f25192ff327ee
Reviewed-on: https://gerrit.openafs.org/14643
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Ganesh G. Chaudhari <gangovind@in.ibm.com>
Tested-by: Ganesh G. Chaudhari <gangovind@in.ibm.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agotests: Avoid verbose output for 'make check V=0' 19/14619/3
Andrew Deason [Mon, 28 Dec 2020 18:22:38 +0000]
tests: Avoid verbose output for 'make check V=0'

For "pretty" V=0 builds, change 'make check' to run 'runtests' in
non-verbose mode, so we just get a summary of test results, instead of
the raw test output. Without V=0, the default is unchanged, so we
still print out all test output by default.

Change-Id: I554f9d32ed5a9cd27e83fef6245af589d91e801f
Reviewed-on: https://gerrit.openafs.org/14619
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>

2 years agotests: Introduce 'make check TESTS=test/name' 17/14317/5
Andrew Deason [Wed, 1 Jul 2020 19:21:35 +0000]
tests: Introduce 'make check TESTS=test/name'

Currently 'make check' always runs all tests. We can run individual
tests manually, but doing so is a bit cumbersome to do under the same
environment as 'make check', since doing so means running something
like this:

    $ MAKECHECK=1 $(abs_top_srcdir)/tests/libwrap @TOP_OBJDIR@/lib \
        ./runtests opr/fmt util/ktime

To make it easier to run single tests introduce a way of calling 'make
check' like this:

    $ make check TESTS='opr/fmt util/ktime'

Which will run the same commands as 'make check', but will run
runtests with only the specified tests, instead of running the default
list.

Some makefiles currently use a "TESTS" or "tests" variable to list
their test binaries; rename them all to "BINS" to avoid conflicting
with this new use for "TESTS" and to make our makefiles a little more
consistent.

Change-Id: I427f83be0d4571794644a97123bcd1f32427bd05
Reviewed-on: https://gerrit.openafs.org/14317
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoRemove NoAuth procedures from Admin Guide 38/14638/2
Benjamin Kaduk [Thu, 10 Jun 2021 15:27:58 +0000]
Remove NoAuth procedures from Admin Guide

Retain the factual description of what the file/flag does, but remove
the suggestion that it is useful in favor of a disclaimer that it is
not needed, and replace the emergency-recovery procedure with a short
description using -localauth.

Change-Id: I18b0dad9740f01515717d572a0374cd2f77fc02d
Reviewed-on: https://gerrit.openafs.org/14638
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoRemove recommendation to use NoAuth from NoAuth.5 42/12142/5
Benjamin Kaduk [Fri, 25 Dec 2015 01:46:29 +0000]
Remove recommendation to use NoAuth from NoAuth.5

Do not document that there are cases when this file should exist;
there are not.

Installation no longer needs this file, and key emergencies can
be handled using asetkey or, on 1.8.x, the kerberos tooling to modify
rxkad.keytab.

Change-Id: I0c3ba15f3ffca8660be2d8b092f10053258742e6
Reviewed-on: https://gerrit.openafs.org/12142
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoRemove rpctestlib 17/14617/3
Andrew Deason [Fri, 7 May 2021 20:02:06 +0000]
Remove rpctestlib

There is some code in tests/rpctestlib that appears to be for testing
fileserver RPCs and callback processing, added in commit 262a678d (An
RPC test dispatch library for vice). However, it has never been used,
it seems unlikely that it will be used anytime soon, and it's not
clear if it even works (it contains many hard-coded references to
interfaces and IP addresses).

Just remove the unused code, and some references to rpctestlib. It can
always be added back if needed (or more likely, reimplemented to be
more in line with our current test framework in tests/ ).

Change-Id: Ied3be7474581d8ee75ae000815bb7364d143fd31
Reviewed-on: https://gerrit.openafs.org/14617
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoRemove a few unused opr/util components 47/14547/5
Andrew Deason [Wed, 3 Mar 2021 18:24:27 +0000]
Remove a few unused opr/util components

A few pieces of opr and util have no callers, have never been used
since they were added, and are not expected to be used in the near
future. Remove the unused code; if we ever want to use any of this
again, it can be re-added when we actually need it.

Specifically, this removes:

- 'stoupper' in src/opr/casestrcpy.c, added in e117599f
  (fileserver-hates-pruclient-20060626)

- 'opr_ffsll' and 'opr_flsll' in src/opr/ffs.h, added in d4369917
  (opr: implement the BSD ffs() functions)

- src/util/thread_pool.c and related headers, added in 4ca57f3f
  (Provide an abstract thread pool object)

Change-Id: Id098cb86318ac9e2412937f4b63b5d99e35be568
Reviewed-on: https://gerrit.openafs.org/14547
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agocf: Resolve implicit function definition for memset 31/14631/4
Cheyenne Wills [Fri, 4 Jun 2021 16:18:39 +0000]
cf: Resolve implicit function definition for memset

The autoconf function OPENAFS_HAVE_STRUCT_FIELD can produce a compiler
warning for an implicit function definition for memset, however with
macOS 11 (Big Sur) the default compiler flags have been changed
(-Werror=implicit-function-declaration) so that this is now flagged as an
error. As an error this can lead to an incorrect result returned by
OPENAFS_HAVE_STRUCT_FIELD.

Add an include for <string.h> to provide the necessary definition for
memset.

Note, both gcc and clang can produce the implicit function definition
warning.

Change-Id: I05ea43e1712c0450b7d1a78d4e953bfad9be28b9
Reviewed-on: https://gerrit.openafs.org/14631
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>

2 years agoafs: Avoid creating unused conns 37/14637/2
Andrew Deason [Mon, 7 Jun 2021 23:23:39 +0000]
afs: Avoid creating unused conns

In afs_LoopServers and PCallBackAddr, we loop over our known servers
to issue an RPC, but we skip over servers on various conditions. For
most of those, we skip over the server before creating the relevant
conn (via afs_ConnBySA), but not for the
SRVADDR_ISDOWN/afs_HaveCallBacksFrom check. If we skip over the server
for that reason, we'll create an afs_conn and rx_connection, and then
release our refs without using them.

This doesn't cause any big problems, but it does create extra
connections and peers that linger around for at least a couple of
hours (until they get cleaned up by afs_GCUserData).

It's very easy to avoid these extra useless conns; just perform the
SRVADDR_ISDOWN/afs_HaveCallBacksFrom check before we call
afs_ConnBYSA. This also makes the check a bit more consistent with the
other checks we do.

Change-Id: Ie49b87e5dd755f77364502528a02c14944e8c23d
Reviewed-on: https://gerrit.openafs.org/14637
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agocf: Fix typo in test for enable-shared/with-swig 28/14628/3
Cheyenne Wills [Tue, 1 Jun 2021 18:17:01 +0000]
cf: Fix typo in test for enable-shared/with-swig

The commit cf: Disable swig if shared libraries are disabled (0e84b7405)
contains a typo in a test: "x$enable_shared" != "yes".  This causes
configure to exit due to incorrectly testing --enable-shared.

Update swig.m4 to correct the typo.

Change-Id: I0d769ec41e2e7896f2232965b5eaa19734033c83
Reviewed-on: https://gerrit.openafs.org/14628
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agodeorbit afsinstall 24/14624/3
Mark Vitale [Thu, 20 May 2021 16:41:24 +0000]
deorbit afsinstall

This is a very old perl script (with accompanying documentation) to
install AFS.  It has not been maintained at all since the license was
updated in 2000.

Remove it from the tree.

Change-Id: I8fb4ce9d969b8bd246f7f640b8fd63921556a529
Reviewed-on: https://gerrit.openafs.org/14624
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agocf: Disable swig if shared libraries are disabled 06/14606/4
Cheyenne Wills [Mon, 26 Apr 2021 22:17:39 +0000]
cf: Disable swig if shared libraries are disabled

When building with the option --disable-shared and swig is also enabled
(either explicitly, or autodetected) a build failure occurs when trying
to link libuafs/PERLUAFS/ukernel.so

Update the configure test for swig to disable the swig autodetection
when --disable-shared was specified, as well as emitting a notice
message stating that the swig autodetection has been disabled.

If --with-swig=yes was specified along with --disable-shared, generate a
configure error stating --with-swig is incompatible with
--disable-shared.

Change-Id: I766cf13b41c1d160e98eb160e0f907d5de2472c9
Reviewed-on: https://gerrit.openafs.org/14606
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agorx: Remove multi_End_Ignore 25/14425/7
Andrew Deason [Tue, 3 Nov 2020 19:49:15 +0000]
rx: Remove multi_End_Ignore

Since commit "rx: Remove delays in multi_End_Ignore", multi_End_Ignore
and multi_Finalize_Ignore are unused in the tree, and should not be
used by callers. Remove the dead functions.

This commit bumps LT_current in libafsrpc to 3, because we are
removing the exported symbol multi_Finalize_Ignore.

Change-Id: I4d298f5118a69cfd04de44c3f268691a87a6d9a7
Reviewed-on: https://gerrit.openafs.org/14425
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 years agorx: Remove delays in multi_End_Ignore 95/14595/3
Andrew Deason [Fri, 16 Apr 2021 16:11:35 +0000]
rx: Remove delays in multi_End_Ignore

When using our multi_Rx mechanism, callers can use either multi_End or
multi_End_Ignore at the end of the multi_Rx block. Among other things,
these macros run 'rx_EndCall(call, code)' for each call in the
multi_Rx invocation that hasn't already ended. For multi_End, 'code'
is RX_USER_ABORT, and for multi_End_Ignore, 'code' is 0; the macros
are otherwise equivalent.

When multi_End is used, this means any un-ended calls are aborted with
RX_USER_ABORT, and the call immediately ends. But when
multi_End_Ignore is used, the call is not aborted, and so we must wait
for the peer to acknowledge that it has received our packets before
ending (done via an rxi_ReadProc call in rx_EndCall).

This means that if a caller multi_Abort's out of a multi call and uses
multi_End_Ignore, we'll wait for the peer to acknowledge our packets
for all of the calls we haven't processed. Waiting for that is a
complete waste of time, since we don't care about the results of those
calls (since we multi_Abort'd). This doesn't matter much if those
calls are responded to promptly, but if the peer is not up or is just
slow, it can cause us to wait several seconds until we timeout.

There are currently only three users of multi_End_Ignore:

- DoProbe in src/ubik/recovery.c

- MultiBreakCallBackAlternateAddress_r in src/viced/callback.c

- MultiProbeAlternateAddress_r in src/viced/callback.c

All of these use multi_Rx to try and probe multiple IPs for the same
machine in parallel, and so some of the calls may very well be trying
to contact unreachable IPs; we only need one to work for the call to
succeed.

To avoid the unnecessary delays in these codepaths, convert them to
use multi_End. Change multi_End_Ignore to be the same as multi_End,
and multi_Finalize_Ignore to the same as multi_Finalize, to make sure
the bad behavior is not used. The _Ignore macros/functions are now
unused in the tree, but keep them around for now since
multi_Finalize_Ignore is exported by libafsrpc.

Thanks to mbarbosa@sinenomine.net for discovering this weird behavior.

Change-Id: I65536a0975bd7a16bb805555943c032c5e6afdf3
Reviewed-on: https://gerrit.openafs.org/14595
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoafs: Assert avc->lock is held in afs_IAS_once 92/14592/3
Andrew Deason [Mon, 12 Apr 2021 23:21:23 +0000]
afs: Assert avc->lock is held in afs_IAS_once

Commit 3be5880d (afs: Avoid panics in afs_InvalidateAllSegments) added
an assert to check that vcache->lock is write-locked before we call
afs_InvalidateAllSegments_once from a background operation.

However, afs_InvalidateAllSegments_once should always be called with
vcache->lock write-locked; there's nothing specific about the
backgrounded call that requires this. So to make sure we catch all
cases, move this assert to afs_InvalidateAllSegments_once itself.

Also remove the conditional check for WriteLocked(&avc->lock) in here,
since clearly avc->lock must be write-locked (and actually is, since
change Ic309e4006bf47bcb38fa2b53bf103e0c645a856d "afs: write-lock
vcache->lock in afs_InactiveVCache").

Add some comments to this function while we're here, to more clearly
indicate what locks are needed.

Change-Id: I10c47021e94220879cd79c0f7390c52a13ee0eee
Reviewed-on: https://gerrit.openafs.org/14592
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoafs: write-lock vcache->lock in afs_InactiveVCache 84/14584/3
Mark Vitale [Sat, 27 Mar 2021 04:00:17 +0000]
afs: write-lock vcache->lock in afs_InactiveVCache

Since the original IBM code import, the comments for
afs_InvalidateAllSegments indicate that vcache->lock W should be held at
entry.  However, even back then, only LINUX and IRIX honored this
requirement when the 'inactive' vnode operation reached
afs_InvalidateAllSegments.

Over the years, a number of commits have changed the operation and
locking for the LINUX inactive vnode op:

5293aa35617a6ad35980ce16fdf492ea960cc18a linux-iput-and-glock-changes-20010130
e8591334602e5e8dad78dc6426d3c44d564572c1 linux-osi-clear-inode-locking-fix-20010816
652f3bd9cb7a5d7833a760ba50ef7c2c67214bba linux-dynamic-inodes-20050710
e0d9e434bb778a2507c1cd6d96c1faa2071f2b2c put-inode-speedup-20050815
b21c13dc3ab751118220dc31276995050841a1ae linux-dentry-iput-20060813

Eventually, ac52e2f3c0bec9298d020de963036409165f380e
linux-dont-lock-around-inactivevcache-20061010 removed the vcache->lock
from afs_dentry_iput (the current OpenAFS handler for inactive vcaches).
The commit message states:

    "iafs_InactiveVCache() [sic] calls afs_InvalidateAllSegments() which says
    it should be called with the vnode locked. so the lock should
    probably be moved to afs_InactiveVCache() so it can be droppped
    before calling afs_remunlink()."

Unfortunately, the vcache->lock was never moved to afs_InactiveVCache.

Finally, 3be5880d1d2a0aef6600047ed43d602949cd5f4d 'afs: Avoid panics in
afs_InvalidateAllSegments' introduced a background operation
BInvalidateSegments that contains an assert for vcache->lock.  This
assert has exposed the existing lack of proper locking for some paths to
afs_InvalidateAllSegments by causing a kernel panic:

  d_iput -> afs_dentry_iput -> afs_InactiveVCache ->
  afs_InvalidateAllSegments -> afs_BQueue(BOP_INVALIDATE_SEGMENTS..) ->
  BInvalidateSegments -> osi_Assert(WriteLocked(&vcache->lock))

Prevent the panic by modifying afs_InactiveVCache to obtain vcache->lock
W before calling afs_InvalidateAllSegments, and dropping it before
calling afs_remunlink.

Thanks to Chaskiel Grundman for reporting and diagnosing the problem.

Change-Id: Ic309e4006bf47bcb38fa2b53bf103e0c645a856d
Reviewed-on: https://gerrit.openafs.org/14584
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agodoc: Look in $srcdir for documentation sources 22/14622/2
Andrew Deason [Wed, 12 May 2021 20:35:01 +0000]
doc: Look in $srcdir for documentation sources

In several places, we look for documentation source files in e.g.
'doc/man-pages', 'doc/xml', etc. But if we are running an objdir
build, those directories won't exist relative to the current working
directory; we need to look in $srcdir to find them.

So, if we're running an objdir build, our man pages and other
documentation won't be installed. We don't report any error in this
case (the relevant steps are just skipped), since building the
documentation is optional, in case the doc sources are not present.

To fix this, look in $srcdir in the various places that reference doc
source files. Fixing the 'for' loops in the 'dest' and 'install'
targets in doc/man-pages requires some extra cd'ing around, because $M
is used as part of another path in the body of the loop.

Change-Id: Ic3c90ab5e64aeefe6235efb6f6ec26080d7b3a70
Reviewed-on: https://gerrit.openafs.org/14622
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoafs: clarify cold and warm shutdown logic 82/12182/7
Mark Vitale [Fri, 29 Jan 2016 06:00:56 +0000]
afs: clarify cold and warm shutdown logic

Currently, any code that wants to perform a cold shutdown must first set
global afs_cold_shutdown = 1, then call afs_shutdown(void).

Instead, modify afs_shutdown() to accept a single parm which specifies
AFS_WARM or AFS_COLD shutdown, and to set the value of global
afs_cold_shutdown based on this parm.  Remove all other assignments for
afs_cold_shutdown.  Modify all callers of afs_shutdown() to specify
AFS_WARM or AFS_COLD as needed to maintain equivalent function.

This should make it much easier to tell at a glance what type of
shutdown is being requested by each caller to afs_shutdown().

No functional change should be incurred by this commit.

Change-Id: I921eca5b4d2659209154fbe37d575db69bf708b8
Reviewed-on: https://gerrit.openafs.org/12182
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoafsd: remove unused variable afs_shutdown 80/14380/3
Mark Vitale [Thu, 1 Oct 2020 21:13:51 +0000]
afsd: remove unused variable afs_shutdown

Since the original IBM code import, the variable afs_shutdown has been
set but never read.

Remove it from the code base.

No functional change is incurred by this commit.

Change-Id: Iac603465ee911fa3601010b67f3f34f22d7b0e3f
Reviewed-on: https://gerrit.openafs.org/14380
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoafs: remove duplicate declaration for afs_shutdown() 81/12181/6
Mark Vitale [Fri, 29 Jan 2016 04:38:59 +0000]
afs: remove duplicate declaration for afs_shutdown()

Somehow there were two.  Now there is but one.

Change-Id: I60c793cf3b13508e89020e9f19847a41de4d0277
Reviewed-on: https://gerrit.openafs.org/12181
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

2 years agoafs: afsd -shutdown sets afs_cold_shutdown too soon 80/12180/6
Mark Vitale [Thu, 28 Jan 2016 15:01:13 +0000]
afs: afsd -shutdown sets afs_cold_shutdown too soon

'afsd -shutdown' always invokes syscall(AFSOP_SHUTDOWN)
with parm2 set to 1 to indicate a "cold" shutdown.
(There are no other callers to AFSOP_SHUTDOWN).

AFSOP_SHUTDOWN sets global variable afs_cold_shutdown
based on the value of parm2.  Then it checks to see if
AFS is still mounted; if so, we return early with EACCES.
However, global afs_cold_shutdown remains set.

Therefore, the next successful 'umount' after a "failed"
'afsd -shutdown' will always trigger a "cold" shutdown.
This is contrary to the intent of the current implementation,
which is to perform a "warm" shutdown upon 'umount' for
most platforms.  (Exceptions:  AIX, OBSD, NBSD always
specify a cold shutdown upon 'umount').

This bug would never be noticed on the "cold" exception
platforms, but on the "warm" platforms the inconsistency
of seeing an unexpected "COLD" shutdown may be confusing
and surprising.

Make shutdown operation more self-consistent by modifying
AFSOP_SHUTDOWN to defer setting of afs_cold_shutdown until
after the mount test.

Change-Id: If4ddb592d0b8fc8098f7ef96cec82f9f545b9099
Reviewed-on: https://gerrit.openafs.org/12180
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

2 years agoafs: free the Buffers array correctly during shutdown 83/12183/6
Mark Vitale [Fri, 29 Jan 2016 06:30:47 +0000]
afs: free the Buffers array correctly during shutdown

DInit() allocates 'Buffers' with afs_max_buffers = 4*nbuffers
worth of buffer structs to allow for run-time expansion.

But shutdown_bufferpackage() free 'Buffers' as if it only had
nbuffers worth of buffer structs.

Correct the size of Buffers passed to afs_osi_Free().

Discovered during Solaris shutdown testing with kmem_flags=x0f.
This bug is not Solaris-specific, but it may be symptomless on other
platforms.

Introduced by commit e7c966354c428a5a5929a3db6a829ee71c8ba2fc 'Flexible
client buffer growth'; this only affected cold shutdowns (afsd
-shutdown).

After commit 2336164d1bf63980419d3a870f908f1f384fdfc0 'afs: Actually
free resources during warm shutdown', this bug also affects warm
shutdowns (the default when /afs is unmounted).

Change-Id: I6b77f4f8f432a1c20efb1ff2349e349b46a9d58d
Reviewed-on: https://gerrit.openafs.org/12183
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>

2 years agoauth_depinstall: Add rxgk_depinstall as dependency 09/14609/4
Cheyenne Wills [Mon, 3 May 2021 16:14:37 +0000]
auth_depinstall: Add rxgk_depinstall as dependency

In a clean workspace, a 'make libuafs' fails to build due a missing
header file 'rx/rxgk_types.h' since the file rxgk_types.h was not
installed in the includes directory.  The libuafs target consumes
rxgk_types.h (installed by rxgk_depinstall) indirectly via
auth/cellconfig.p.h that includes rxgk_types.h.

 make libuafs
 ...
 In file included from .../src/afs/UKERNEL/afs_usrops.c:27:
  .../include/afs/cellconfig.h:49:10: fatal error: rx/rxgk_types.h:
  No such file or directory

Add rxgk_depinstall to the list of dependencies for the auth_depinstall
target in the top Makefile to reflect the header dependency.

Change-Id: I01239396c4a5c162b75a29a8e6aaf602b3c1ebb9
Reviewed-on: https://gerrit.openafs.org/14609
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agomacos: delegate sock_* calls to bkg daemons 31/14431/12
Marcio Barbosa [Fri, 9 Apr 2021 15:14:52 +0000]
macos: delegate sock_* calls to bkg daemons

As part of Apple's ongoing effort to modernize macOS, improve security
and reliability, the deprecation of kernel extensions was officially
announced at WWDC19. According to this announcement, Kernel programming
interfaces will be deprecated as alternatives become available, and
future OS releases will no longer load kernel extensions that use
deprecated KPIs by default.

Unfortunately, the following KPIs, extensively used by rx, are included
in the list of deprecated KPIs as of macOS 10.15:

- sock_receivembuf
- sock_close
- sock_send
- sock_socket
- sock_setsockopt
- sock_bind

To workaround this problem, delegate calls to the functions mentioned
above to bkg daemons forked by afsd. Notice that the ifadd_* and ifnet_*
functions are also deprecated. Fortunately, these calls can be avoided
enabling AFS_USERSPACE_IP_ADDR.

Thanks to Andrew Deason for his assistance (ideas, suggestions,
documentation, etc).

Change-Id: I916b66455bec73138c55e2764cc1146b998cb19f
Reviewed-on: https://gerrit.openafs.org/14431
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agomacos: packaging support for MacOS X 11.0 30/14430/4
Marcio Barbosa [Thu, 28 Jan 2021 23:49:25 +0000]
macos: packaging support for MacOS X 11.0

This commit introduces the new set of changes / files required to
successfully create the dmg installer on OS X 11.0 "Big Sur".

Change-Id: I600aba528305d8d42393e90fd56e98e4557d9233
Reviewed-on: https://gerrit.openafs.org/14430
Reviewed-by: Yadavendra Yadav <yadayada@in.ibm.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agomacos: add support for MacOS 11.0 29/14429/6
Marcio Barbosa [Thu, 28 Jan 2021 22:45:10 +0000]
macos: add support for MacOS 11.0

This commit introduces the new set of changes / files required to
successfully build the OpenAFS source code on OS X 11.0 "Big Sur".

While here, refactor the code that checks if the "-Xlinker -kext"
system-specific linker option is needed.

Change-Id: I9895ce97143aec500a5bbb4a9502eca19769c85e
Reviewed-on: https://gerrit.openafs.org/14429
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agovol: ensure ih package defaults are set for salvage 78/14378/6
Mark Vitale [Thu, 30 Jul 2020 20:42:19 +0000]
vol: ensure ih package defaults are set for salvage

Like most OpenAFS components that work with ihandles, salvager relies on
implicit invocation of ih_PkgDefaults via the one-shot in the first call
to IH_INIT.

Unfortunately, there is at least one reachable code path in salvager
that asserts (panics) because vol_io_params has not yet been
initialized.  This is when salvaging a volume group that does not have a
link table; the salvager then panics while attempting to create a new
link table:

SalvageFileSys -> SalvageFileSys1 -> DoSalvageVolumeGroup ->
CreateLinkTable -> IH_CREATE -> namei_icreate -> icreate ->
namei_SetLinkCount -> FDH_SYNC -> ih_fdsync -> osi_Assert(0)

This path was discovered while testing the non-dafs salvager, but it has
also been observed in the field with the DAFS salvageserver.  It is
possible that there are additional undiscovered paths where
vol_io_params are required but uninitialized.

Add an implicit ih_PkgDefaults call to icreate to avoid triggering the
assert via the code path above.

Change-Id: If348d7f8ab2d34d55727b5a6d78db6e1c8e14cdf
Reviewed-on: https://gerrit.openafs.org/14378
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agovol: move ih_PkgDefaultsSet check inside ih_PkgDefaults 83/14383/6
Mark Vitale [Fri, 9 Oct 2020 20:28:15 +0000]
vol: move ih_PkgDefaultsSet check inside ih_PkgDefaults

Instead of repeating the oneshot check in each caller of ih_PkgDefaults,
move the oneshot check into ih_PkgDefaults itself.

While here, ensure that ih_PkgDefaults does its work under IH_LOCK.

Finally, make ih_PkgDefaultsSet a local static variable (no longer
exported).

Change-Id: I66717de12cb5cc70de0507a768f968f6afbd38f8
Reviewed-on: https://gerrit.openafs.org/14383
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoFBSD: Add support for FreeBSD 12.2 74/14474/3
Tim Creech [Fri, 30 Oct 2020 01:29:10 +0000]
FBSD: Add support for FreeBSD 12.2

Change-Id: I28aeccbc1f4e57fc2f0aeef6e0613e43b0208705
Reviewed-on: https://gerrit.openafs.org/14474
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoafs: Remove ISAFS_GLOCK in afs_ConnBySA 81/14581/2
Andrew Deason [Sat, 12 Oct 2019 19:17:35 +0000]
afs: Remove ISAFS_GLOCK in afs_ConnBySA

We must be holding AFS_GLOCK at this point in the code, since we're
calling functions like ObtainSharedLock, which require AFS_GLOCK to be
held. Remove the call to ISAFS_GLOCK and the relevant conditionals, to
get rid of some visual noise.

The call to ISAFS_GLOCK was added in commit cb9e0292 (unix cm
rx-oblivious connection pooling), in which it was also unnecessary.

Change-Id: I9281108453359acf079828c1625556e2380c3a51
Reviewed-on: https://gerrit.openafs.org/14581
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agocf: Run AFS_LT_INIT after setting CC 85/14585/3
Cheyenne Wills [Wed, 7 Apr 2021 16:51:58 +0000]
cf: Run AFS_LT_INIT after setting CC

Since libtool support was introduced for 1.8.x in commit 69f26ece (Add
libtool support), we've run LT_INIT or AFS_LT_INIT early on in
configure.ac.

If CC isn't set, AFS_LT_INIT defaults to using gcc when it's available.
On Solaris, we set CC and CFLAGS ourselves later (in osconf.m4) to use
the Solaris Studio compiler, but this doesn't change the compiler that
AFS_LT_INIT already chose. As a result, on Solaris if no value for CC is
given during configure and gcc is available, some libtool commands will
try to use gcc with CFLAGS intended for the Solaris Studio compiler,
which will fail.

  /bin/sh ../../libtool --quiet --mode=link --tag=CC ... -mt ...
  gcc: error: unrecognized command line option '-mt'; did you mean '-t'?

To fix this, move AFS_LT_INIT into osconf.m4 after our platform-specific
macros have had a chance to set CC. Also move our checks for AR, AS,
etc. to after AFS_LT_INIT, since AFS_LT_INIT sets those.

Note.  Without GCC installed on a Solaris system, libtool will find the
Solaris Studio compiler (assuming that PATH is set up correctly) and the
build will proceed successfully. Just installing the GCC package is
sufficient to break the build.

This commit fixes a regression from 1.6.x where having the GCC package
installed on the system would not break the build.

Change-Id: I6458739fa5050eb98e6980e8d7b0ebfcc62d493f
Reviewed-on: https://gerrit.openafs.org/14585
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agovolser: Remove unused UV_* operations 80/14580/2
Andrew Deason [Thu, 31 Oct 2019 18:52:38 +0000]
volser: Remove unused UV_* operations

The UV_ functions UV_AddVLDBEntry, UV_ZapVolumeClones, and
UV_GenerateVolumeClones are not called by anyone in the tree. Remove
the dead code.

Change-Id: I8dfd0f183702d9f059cd5a71fb72272d0864ecc0
Reviewed-on: https://gerrit.openafs.org/14580
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agobozo: Fix the test for bosserver '-cores none' 59/14559/2
Cheyenne Wills [Thu, 18 Mar 2021 14:28:22 +0000]
bozo: Fix the test for bosserver '-cores none'

The check for the '-cores none' parameter is incorrect resulting in the
parameter to be taken as a directory path.

Update the string comparison.

Change-Id: I0d72c1add52d36e40f75981a86c16b5689d38fd8
Reviewed-on: https://gerrit.openafs.org/14559
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoMake OpenAFS 1.9.1 60/14560/2 openafs-devel-1_9_1
Benjamin Kaduk [Fri, 19 Mar 2021 01:50:35 +0000]
Make OpenAFS 1.9.1

Update version strings for the second 1.9.x development release.

Change-Id: I318ff00f02f618e0a25571a3c957ae6a6500e65c
Reviewed-on: https://gerrit.openafs.org/14560
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoUpdate NEWS for OpenAFS 1.9.1 39/14539/4
Michael Meffie [Thu, 18 Feb 2021 23:35:33 +0000]
Update NEWS for OpenAFS 1.9.1

Change-Id: I20c23a3d0a84491c1eb4b9c36aee62726fb0b4e9
Reviewed-on: https://gerrit.openafs.org/14539
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agofstrace: remove common dead code 56/14556/2
Mark Vitale [Thu, 11 Mar 2021 22:34:29 +0000]
fstrace: remove common dead code

Previous commits removed dead code from both fstrace.c and afs_icl.c.
Now remove anything from config/icl.h that is no longer needed.

No functional change is incurred by this commit.

Change-Id: Ibdad10ec4c91cd8c2d3fbd637354357f05ac2621
Reviewed-on: https://gerrit.openafs.org/14556
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoafs: remove dead ICL (fstrace) code 55/14555/2
Mark Vitale [Thu, 11 Mar 2021 20:36:54 +0000]
afs: remove dead ICL (fstrace) code

The ICL code (afs/afs_icl.c) which supports fstrace includes a number of
functions that have been dead code since the original IBM code import.
Some of these seem to have been intended to support fine-grained event
tracing, but the implementation was never completed.

Remove the dead code.  No functional change is incurred by this commit.

Change-Id: If4d6d993175df57d4c5d827ab178ed3ba0bc7ed8
Reviewed-on: https://gerrit.openafs.org/14555
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agofstrace: remove dead code 54/14554/2
Mark Vitale [Thu, 11 Mar 2021 20:19:58 +0000]
fstrace: remove dead code

Numerous functions in venus/fstrace.c with names icl_* have been dead
code since the original IBM code import.  Furthermore, many of them have
similar implementations in afs/afs_icl.c with names afs_icl_*.

Remove the dead code.  No functional change is incurred by this commit.

Change-Id: I3943a9cf333c4044c877b46e7b2eec4285358c18
Reviewed-on: https://gerrit.openafs.org/14554
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agobozo: Fix memory leak, check for malloc failures 51/14551/5
Cheyenne Wills [Fri, 12 Mar 2021 19:29:57 +0000]
bozo: Fix memory leak, check for malloc failures

While reading the BosConfig file, the buffer obtained to hold the notp
(notify) parameter is never freed. Reading the BosConfig is only
done once at bosserver start up, so this is a one-time memory
allocation.

There are no checks for malloc failures.

Release the notp buffer and add checks for memory allocation errors.

Change-Id: Iffcb0db12f983a6a6d6a810a98be30152fa73c89
Reviewed-on: https://gerrit.openafs.org/14551
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoLinux 5.12: Add user_namespace param to inode ops 49/14549/6
Cheyenne Wills [Fri, 5 Mar 2021 23:31:03 +0000]
Linux 5.12: Add user_namespace param to inode ops

The Linux commits:
"fs: make helpers idmap mount aware" (549c72977) and
"attr: handle idmapped mounts" (2f221d6f7) that were merged into
Linux-5.12-rc1 cause a build failure when creating the kernel module.

Several functions within the inode_operations structure had their
signature updated to include a user_namespace parameter.  This allows
a filesystem to support idmapped mounts.

OpenAFS only implements some of the changed functions.

   LINUX/vnodeops function inode_operation
   =====================   ===============
   afs_notify_change       setattr
   afs_linux_getattr       getattr
   afs_linux_create        create
   afs_linux_symlink       symlink
   afs_linux_mkdir         mkdir
   afs_linux_rename        rename
   afs_linux_permission    permission

Update the autoconf tests to determine if the Linux kernel requires
the user_namespace structure for inode_operations functions. If so,
define a generic "IOP_TAKES_USER_NAMESPACE" macro.

Update the above vnodeops functions to accept a 'struct user_namespace'
parameter.

When using the 'setattr_prepare' function a user namespace must be
now provided. In order to provide compatibility as a non-idmapped mount
filesystem the initial user namespace can be used. With OpenAFS, the
initial user namespace obtained at kernel module load time is stored in
a global variable 'afs_ns'.

Update the call to setattr_prepare to pass the user namespace pointed
to by the 'afs_ns' global variable.

Update calls to setattr to pass the user namespace pointed to by
the 'afs_ns' global variable.

Notes:

The changes introduced with Linux 5.12 allow a filesystem to support
idmapped mounts if desired. This commit does not implement support for
idmapped mounts, but will continue to use the same initial user
namespace as prior to Linux 5.12.

With Linux 5.12 the following autoconf checks fail:

 HAVE_LINUX_INODE_OPERATIONS_RENAME_TAKES_FLAGS
 HAVE_LINUX_SETATTR_PREPARE
 IOP_CREATE_TAKES_BOOL
 IOP_GETATTR_TAKES_PATH_STRUCT
 IOP_MKDIR_TAKES_UMODE_T

The new macro 'IOP_TAKES_USER_NAMESPACE' covers the cases where these
macros where used.

Change-Id: Id450d5c716137340ed20af5531c0cd756e4435cd
Reviewed-on: https://gerrit.openafs.org/14549
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoLinux: Create wrapper for setattr_prepare 48/14548/5
Cheyenne Wills [Mon, 8 Mar 2021 16:22:04 +0000]
Linux: Create wrapper for setattr_prepare

Move call to setattr_prepare/inode_change_ok into an osi_compat.h
wrapper called 'afs_setattr_prepare'.  This moves some of the #if logic
out of the mainline code.

Change-Id: Ie17cf4c645d754c9e9efd8a603f1bc752d07cf36
Reviewed-on: https://gerrit.openafs.org/14548
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoFBSD: Avoid recursive osi_VM_StoreAllSegments lock 00/14000/6
Andrew Deason [Wed, 1 Jan 2020 23:09:24 +0000]
FBSD: Avoid recursive osi_VM_StoreAllSegments lock

Currently, osi_VM_StoreAllSegments calls vget() for the given vnode,
which requires locking the vnode. However, the vnode should already be
locked. For example, when called from the close syscall, we reach this
function via: vn_close1 -> afs_vop_close -> afs_close ->
afs_StoreOnLastReference -> afs_StoreAllSegments ->
osi_VM_StoreAllSegments. This causes a panic like so:

    kernel: panic: lockmgr_xlock_hard: recursing on non recursive lockmgr 0x[...] @ /usr/src/sys/kern/vfs_subr.c:2730

We can also reach this code path from the BOP_STORE background
operation (BStore -> afs_StoreOnLastReference -> afs_StoreAllSegments
-> osi_VM_StoreAllSegments), initiated from afs_close(), which has the
vnode locked. In this case, we won't be recursively locking the vnode,
since the process calling afs_close() is the one that holds the lock,
and the background thread is the process trying to lock the vnode
again. So we'll just deadlock.

From the comments in this function, it seems like locking the vnode at
all in here is unnecessary, since the vnode should be locked from the
higher-level functions anyway. So just skip the vget and all of the
related looping retry logic. As a result, this function can now become
somewhat simplified.

Change-Id: Ic5a18de46e51dc86190207163ad0fe73bc03cbd7
Reviewed-on: https://gerrit.openafs.org/14000
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoFBSD: Accommodate 12.0's 64-bit inodes 54/13854/9
Tim Creech [Fri, 30 Aug 2019 01:35:36 +0000]
FBSD: Accommodate 12.0's 64-bit inodes

In FreeBSD 12 (see: https://reviews.freebsd.org/rS318736), the layout
of struct dirent changed to allow for 64-bit inodes and a few other
changes. Update our struct min_direct to accommodate, to allow our
readdir() results to be accurate. Without this, readdir() can yield
garbage entries, due to the mismatch in the structure definitions.

Change-Id: I36c2bf1f35b4d1ab61a2b4d51da7514827b3551b
Reviewed-on: https://gerrit.openafs.org/13854
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agodir: Explicitly 'make all' in src/dir/test 50/14550/2
Andrew Deason [Sat, 6 Mar 2021 04:20:35 +0000]
dir: Explicitly 'make all' in src/dir/test

Currently, we 'cd test' and then just run 'make', which makes the
first target specified in the Makefile. On some platforms (FreeBSD),
this results in 'make' trying to build '%.c', which of course we
cannot do, since that's a pattern rule, and so 'make' fails.

To fix this, just 'make all' explicitly, to make the intended targets
in src/dir/test.

Change-Id: Icbbf60c9c163c24fbbed01c754c4f1eefeae6b78
Reviewed-on: https://gerrit.openafs.org/14550
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoFBSD: Lock vm object before vm_page_undirty 62/14162/4
Andrew Deason [Sun, 26 Apr 2020 01:56:01 +0000]
FBSD: Lock vm object before vm_page_undirty

We must write-lock the underlying vm object before calling
vm_page_undirty; otherwise vm_page_undirty asserts if INVARIANTS is
defined. For example:

    kernel: panic: Lock vm object not exclusively locked @ /usr/src/sys/vm/vm_page.c:4487
    kernel:
    kernel: cpuid = 0
    kernel: time = 1587858280
    kernel: KDB: stack backtrace:
    kernel: #0 0xffffffff80bf0c07 at kdb_backtrace+0x67
    kernel: #1 0xffffffff80ba7f8d at vpanic+0x19d
    kernel: #2 0xffffffff80ba7d73 at panic+0x43
    kernel: #3 0xffffffff80ba3a7e at __rw_assert+0x17e
    kernel: #4 0xffffffff828da525 at vm_page_undirty+0x15
    kernel: #5 0xffffffff828da33e at afs_vop_putpages+0x36e
    kernel: #6 0xffffffff811ef0ae at VOP_PUTPAGES_APV+0x8e
    kernel: #7 0xffffffff80ef4c2d at vnode_pager_putpages+0x7d
    kernel: #8 0xffffffff80ee77cf at vm_pageout_flush+0xff
    kernel: #9 0xffffffff80edd1b9 at vm_object_page_collect_flush+0x239
    kernel: #10 0xffffffff80edce99 at vm_object_page_clean+0x179
    kernel: #11 0xffffffff828d681c at osi_VM_StoreAllSegments+0x18c
    kernel: #12 0xffffffff828508cd at afs_StoreAllSegments+0x9d
    kernel: #13 0xffffffff8287ae0e at afs_StoreOnLastReference+0x17e
    kernel: #14 0xffffffff8287c3a5 at afs_close+0x245
    kernel: #15 0xffffffff828d7766 at afs_vop_close+0x166
    kernel: #16 0xffffffff811eb7a8 at VOP_CLOSE_APV+0x88
    kernel: #17 0xffffffff80c80ba3 at vn_close1+0xe3

So, lock the vm object before undirtying our pages in
afs_vop_putpages.

Change-Id: Ifd047e3caf8c2b3e624aaf2bbdb1235a8c38a414
Reviewed-on: https://gerrit.openafs.org/14162
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoFBSD: Use VM_CNT_INC/VM_CNT_ADD on FreeBSD 12 59/13859/7
Tim Creech [Fri, 30 Aug 2019 02:12:41 +0000]
FBSD: Use VM_CNT_INC/VM_CNT_ADD on FreeBSD 12

r317061 changed where v_vnodein &c are stored. Use the new
VM_CNT_INC/VM_CNT_ADD macros when available to accommodate.

Change-Id: I576e333ebdf9e1c6ebb14ff1a1af4c3ad89faa47
Reviewed-on: https://gerrit.openafs.org/13859
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoFBSD: avoid vrefl() 73/14373/3
Benjamin Kaduk [Fri, 25 Sep 2020 16:22:16 +0000]
FBSD: avoid vrefl()

Commit 20dc2832268eb (correctly) introduced changes so that we
avoid interacting with vnodes marked as VI_DOOMED to the extent
possible, but in doing so inadvertendly used the vrefl() KPI that
was only introduced in FreeBSD 11.0.

Rewrite the relevant logic to use the older vref() KPI, at the cost
of a few more unlock/locks, in order to have a single codepath that
works on all supported FreeBSD versions.

Change-Id: Ib315d59ea6c6208bbd0c908d8eaf502a4de51869
Reviewed-on: https://gerrit.openafs.org/14373
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoFBSD: Handle missing LINK_MAX 60/13860/7
Tim Creech [Fri, 30 Aug 2019 02:13:20 +0000]
FBSD: Handle missing LINK_MAX

LINK_MAX was removed in r327598. When we don't have a LINK_MAX, just
use its value from before it was removed (32767).

Change-Id: Id66a2ba8b7085b392def1d17eace22c7f742e1a4
Reviewed-on: https://gerrit.openafs.org/13860
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoFBSD: Use syscall "helper" functions 58/13858/8
Tim Creech [Fri, 30 Aug 2019 01:55:05 +0000]
FBSD: Use syscall "helper" functions

syscall_register/syscall_deregister were effectively removed in
r329647. Use syscall_helper_register/syscall_helper_unregister
instead, which have existed since r205321 in FreeBSD 9.

Change-Id: I2d5e3101024a44c18395d7eb95c644df6005e0aa
Reviewed-on: https://gerrit.openafs.org/13858
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoFBSD: Handle malloc/free changes in FBSD 12 56/13856/8
Tim Creech [Fri, 30 Aug 2019 01:40:26 +0000]
FBSD: Handle malloc/free changes in FBSD 12

FreeBSD 12 (r328417) removed the deprecated compatibility macros
MALLOC and FREE. Convert our users to just use the normal malloc and
free, so we can build.

FreeBSD 12 (r334545) also changed malloc() into a macro, which breaks
our own malloc macro in our hcrypto config.h. To fix this, just undef
malloc, if it's already a macro.

Change-Id: I5c683e3834710a60cc78476cbaa7203218b11fe0
Reviewed-on: https://gerrit.openafs.org/13856
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoFBSD: Use CK_STAILQ_FOREACH for ifaces on FBSD 12 99/13999/7
Andrew Deason [Sun, 22 Dec 2019 00:34:20 +0000]
FBSD: Use CK_STAILQ_FOREACH for ifaces on FBSD 12

FreeBSD 12 changed how network interfaces and network addresses are
linked together; we're supposed to use CK_STAILQ_FOREACH to traverse
them now, instead of TAILQ_FOREACH. To try to keep this change
simpler, introduce a new macro, AFS_FBSD_NET_FOREACH, which picks the
right macro to use.

Based on a commit by tcreech@tcreech.com.

Change-Id: Iab0f93701dd60dcf4237a7fbbf461019bceaeb38
Reviewed-on: https://gerrit.openafs.org/13999
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoFBSD: Add proper locks when traversing net ifaces 98/13998/7
Tim Creech [Sun, 22 Dec 2019 00:22:40 +0000]
FBSD: Add proper locks when traversing net ifaces

When traversing the list of network interfaces, or the list of
addresses for a network interface, we're supposed to lock the relevant
resource with IFNET_RLOCK, if_addr_rlock, or IN_IFADDR_RLOCK. Add
these locks around our code that examines network interfaces, to
avoid issues if the interface or address list changes while we're
traversing them.

While we're doing this, move around some "AFS_DARWIN_ENV ||
AFS_FBSD_ENV" ifdefs, since these were getting a bit hard to read.
This commit adds some duplicated code, but the result should be easier
to follow.

Also for FreeBSD 12, we must be in NET_EPOCH_ENTER when calling
ifa_ifwithnet/rx_ifaddr_withnet (it panics if we don't, with
INVARIANTS). Add the needed NET_EPOCH_ENTER/EXIT calls, but do so a
bit higher up the call stack, since the returned structures are
potentially no longer valid after we NET_EPOCH_EXIT. Since this means
we're calling these in a few places in libafs, create a couple of rx
abstractions (RX_NET_EPOCH_ENTER) to handle the relevant ifdefs.

[adeason@dson.org: Various adjustments to locking calls; splitting up
DARWIN/FBSD ifdefs.]

Change-Id: I65d63b99b6f6ef3254325cce9338be27ef78478c
Reviewed-on: https://gerrit.openafs.org/13998
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agorx: Indent ifdef maze in rx_kernel.h 61/14161/4
Andrew Deason [Sat, 25 Apr 2020 22:20:54 +0000]
rx: Indent ifdef maze in rx_kernel.h

Change-Id: I3a10206234496b9de6f7ddeafebdee8ab10e5546
Reviewed-on: https://gerrit.openafs.org/14161
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: Indent ifdef maze in rx_kcommon.c 97/13997/7
Andrew Deason [Sat, 21 Dec 2019 04:09:35 +0000]
rx: Indent ifdef maze in rx_kcommon.c

Change-Id: I8b898fb5f7bcc142de3a111baaa6dfb9606fa199
Reviewed-on: https://gerrit.openafs.org/13997
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoafs: Indent ifdef maze in afs_server.c 96/13996/6
Andrew Deason [Sat, 21 Dec 2019 03:51:18 +0000]
afs: Indent ifdef maze in afs_server.c

Change-Id: I223b932490ca1e89711844e41cbff2cd9b50a0f4
Reviewed-on: https://gerrit.openafs.org/13996
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: correctly count RX_PACKET_TYPE_VERSION packets 19/14519/2
Mark Vitale [Wed, 17 Feb 2021 18:53:55 +0000]
rx: correctly count RX_PACKET_TYPE_VERSION packets

Since the original IBM code import, rx statistics for counting incoming
packet types have inadvertently omitted RX_PACKET_TYPE_VERSION packets.
This results in rxdebug -rxstats always reporting 0 for the number of
version packets read.

A similar bug causes a debugging facility in rxi_ReceivePacket to emit
"*UNKNOWN*" instead of "version" for version packets.

Correct all versions of the offending logic.

Change-Id: I9e713eb595b75ef06a347a1c05edb9efffd0b366
Reviewed-on: https://gerrit.openafs.org/14519
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoResolve missing printf args 55/13155/5
Pat Riehecky [Fri, 1 Jun 2018 20:32:57 +0000]
Resolve missing printf args

A handful of printf's requested more args than they were given.  The
missing args are now provided. (via cppcheck)

Change-Id: I3d2bfd1b68a3518ee4c8a65f02446a2bae85d926
Reviewed-on: https://gerrit.openafs.org/13155
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoautoconf: use AC_CHECK_TOOL for as and ld 44/14544/5
Cheyenne Wills [Mon, 22 Feb 2021 18:08:39 +0000]
autoconf: use AC_CHECK_TOOL for as and ld

Some platforms use the GNU target triplet as a prefix to the toolchain
utilities (e.g. x86_64-pc-linux-gnu-as) to allow the use of alternative
toolchains, cross-compiling, etc.

The Gentoo Linux distribution has a mode of building packages
(-native-symlinks) where the toolchain utilities only exist as their
prefixed names (e.g. 'as' does not exist, but 'x86_64_pc-linux-gnu-as'
does). This results in configure failing to locate the tools when using
AC_CHECK_PROGS.  (Gentoo uses the --host and --build configure
parameters to specify the prefix names for the tools).

Replace AC_CHECK_PROGS with AC_CHECK_TOOL for the toolchain related
commands 'as' and 'ld'.

AC_CHECK_TOOL works like AC_CHECK_PROGS but it will also look for
the program with a prefix (specified by using configure's --host
parameter).

Note: libtool.m4 runs AC_CHECK_TOOL for ar.

Change-Id: I8005c765d213b7d1d6292a7dd80f10a3d0e2ec68
Reviewed-on: https://gerrit.openafs.org/14544
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agostrlcpy restricted to array length. 63/13163/14
Pat Riehecky [Wed, 6 Jun 2018 15:55:18 +0000]
strlcpy restricted to array length.

For kaprocs, size of 'caller.userID.name' is defined by a different macro
than size of 'name'.  They can become out of sync, so restricting to
size of dest.
For scout and afsmon-win, the if statement determined that the string
was longer than the dest buffer.  So we are using the size of the buffer
as the max length to setup for truncation.
(via cppcheck)

Change-Id: I38a2bff1d59d17ea02e136c35cd5b132a75a8ed8
Reviewed-on: https://gerrit.openafs.org/13163
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agolocaltime can return NULL if unable to read system clock 06/13206/6
Pat Riehecky [Tue, 12 Jun 2018 18:33:31 +0000]
localtime can return NULL if unable to read system clock

This adds checks for some invocations of localtime() to avoid possible
NULL dereference. (via facebook-infer)

Change-Id: I2b779d8f60c032563eb4ee3cebe20b14afbb0fa3
Reviewed-on: https://gerrit.openafs.org/13206
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>

3 years agoconfigure.ac: Add missing double include guard 33/13333/4
Pat Riehecky [Wed, 19 Sep 2018 20:51:00 +0000]
configure.ac: Add missing double include guard

This is primarily a sanity check (identified by clang-tidy).

Change-Id: I92d05fdfed0e32c0e39cc2f8ce412b613c0a38fc
Reviewed-on: https://gerrit.openafs.org/13333
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoIf realloc() == NULL we lost the pointer to old memory 56/13156/5
Pat Riehecky [Fri, 1 Jun 2018 20:59:37 +0000]
If realloc() == NULL we lost the pointer to old memory

Systems under memory pressure may fail to realloc().  If so, the pointer
to the old memory is lost, but not released.  This code catches the
pointer before hand to ensure the memory isn't leaked. (via cppcheck)

Change-Id: I4c5a11c1daf4e78f7ffde71af0175d9106f6c3cd
Reviewed-on: https://gerrit.openafs.org/13156
Reviewed-by: Joe Gorse <jhgorse@gmail.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoSOLARIS: provide cache manager stats via kstat 70/13170/14
Michael Meffie [Tue, 31 May 2016 20:23:41 +0000]
SOLARIS: provide cache manager stats via kstat

Provide statistical information via the solaris kstat framework.  Data
can be examined with the kstat tool or the kstat userspace api.

The kstat module is called openafs. Three kstat names are provided.  The
"param" name provides cache manager parameters as given by the cmdebug
-cache program.

    # kstat -m openafs -n param

The "cache" name provides cache manager statistics as given by the
xstats plus some additional cache related stats. The "cache" name also
provides the libafs kernel module version string and the current local
cellname.

    # kstat -m openafs -n cache

The "rx" name provides general rx statistics as given by rxdebug -rxstat.

    # kstat -m openafs -n rx

Change-Id: Ic07e3b58fa5c79145f12f8519a6f7fce0d91138b
Reviewed-on: https://gerrit.openafs.org/13170
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoRetire AFS_MOUNT_AFS 23/14323/4
Andrew Deason [Wed, 26 Aug 2020 18:54:00 +0000]
Retire AFS_MOUNT_AFS

Currently, the AFS_MOUNT_AFS #define is used to mean two completely
different things:

- The string "afs", corresponding to the first argument to mount(2) on
  many platforms and some related calls inside libafs (e.g.
  getnewvnode() on FBSD).

- An integer identifying the AFS filesystem (e.g. gfsadd() on AIX).

Depending on the platform and the build context (UKERNEL vs KERNEL),
AFS_MOUNT_AFS gets defined to one of those two things. This is very
confusing, and has led to mistakes in the past, such as those fixed in
commit 446457a1 (afs: Set AFS_VFSFSID to a numerical value).

To avoid such confusion, get rid of AFS_MOUNT_AFS completely, and
replace it with two new symbols:

- AFS_MOUNT_STR, the string "afs".

- AFS_FSNO, the integer given to gfsadd() et al.

When AFS_MOUNT_AFS is split this way, AFS_MOUNT_STR then is always
defined to the same value, so remove it from the param.h files for our
platforms. Instead, define it in afs.h for libafs use, and in
afsd_kernel.c (the only place outside of src/afs that uses it).

Also remove the logic for conditionally defining MOUNT_AFS from the
param.h files, moving the logic to the same locations as
AFS_MOUNT_STR.

Note that this commit removes the numeric definition for AFS_MOUNT_AFS
in param.sgi_65.h (aka AFS_FSNO). We never actually used this value,
since AFS_FSNO is not used on IRIX; instead, we tend to use the
'afs_fstype' global instead of a constant number.

Change-Id: I6cbf051dc938cd1c456cbe236c0afe99a3c3dd87
Reviewed-on: https://gerrit.openafs.org/14323
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoRemove AFS_PARISC_LINUX24_ENV references 72/14472/5
Andrew Deason [Tue, 22 Dec 2020 02:43:56 +0000]
Remove AFS_PARISC_LINUX24_ENV references

Since commit 91713206 (Remove LINUX24 from src/afs),
AFS_PARISC_LINUX24_ENV is never defined. Remove references to it.

Change-Id: I854701f26ec86b9b9fb99dc57c36f04f78a09517
Reviewed-on: https://gerrit.openafs.org/14472
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoLinux 5.11: Test 32bit compat with in_compat_syscall 99/14499/7
Cheyenne Wills [Fri, 22 Jan 2021 14:57:55 +0000]
Linux 5.11: Test 32bit compat with in_compat_syscall

Linux 5.11 removed the TIF_IA32 thread flag with commit:
  x86: Reclaim TIF_IA32 and TIF_X32 (8d71d2bf6efec)

The flag TIF_IA32 was being used by openafs to determine if the task was
handling a syscall request from a 32 bit process.  Building against a
Linux 5.11 kernel results in a build failure as TIF_IA32 is undefined.

The function 'in_compat_syscall' was introduced in Linux 4.6 as
the preferred method to determine if a syscall needed to handle a
compatible call (e.g. 32bit application).

To resolve the build problem, use 'in_compat_syscall' if present (Linux
4.6 and later) to determine if the syscall needs to handle a
compatibility mode call.

Add autoconf check for in_compat_syscall.

Notes about in_compat_syscall:

In Linux 4.6 'in_compat_syscall' was defined for all architectures with
a generic return of 'is_compat_task', but allows architecture specific
overriding implementations (x86 and sparc).

At 4.6 (and later), the function 'is_compat_task' is defined only for
the following architectures to return:

Arch              Returns
=======           ==============================
arm64             test_thread_flag(TIF_32BIT);
mips              test_thread_flag(TIF_32BIT_ADDR)
parisc            test_ti_thread_flag(task_thread_info(t), TIF_32BIT)
powerpc           is_32bit_task()
s390              test_thread_flag(TIF_31BIT)
sparc             test_thread_flag(TIF_32BIT)

If the Linux kernel is not built with compat mode, is_compat_task and
in_compat_syscall is set to always return 0

Linux commit that introduced in_compat_syscall:
  compat: add in_compat_syscall to ask whether we're in a compat syscall
  (5180e3e24fd3e8e7)

Change-Id: I59deebfe5d8cddaf845b15ef69e65a684a961280
Reviewed-on: https://gerrit.openafs.org/14499
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoLinux: Refactor test for 32bit compat 00/14500/5
Cheyenne Wills [Fri, 29 Jan 2021 18:32:36 +0000]
Linux: Refactor test for 32bit compat

Refactor the preprocessor checks for determining the method to test for
32bit compatibility (64bit kernel performing work for a 32bit task) into
a common inline function, 'afs_in_compat_syscall' that is defined in
LINUX/osi_machdep.h.  Update osi_ioctl.c and afs_syscall.c to use
afs_in_compat_syscall.

Add include afs/sysincludes into osi_machdep.h to ensure linux/compat.h
is pulled for the functions called in afs_in_compat_syscall.

Change-Id: I6610cc19fedd909de8e8941ded05ed1608e52403
Reviewed-on: https://gerrit.openafs.org/14500
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoLINUX: Fix includes for fatal_signal_pending test 08/14508/2
Andrew Deason [Thu, 28 Jan 2021 22:59:47 +0000]
LINUX: Fix includes for fatal_signal_pending test

Commit 8b6ae289 (LINUX: Avoid lookup ENOENT on fatal signals) added a
configure test for fatal_signal_pending(). However, this check fails
incorrectly ever since Linux 4.11, because fatal_signal_pending() was moved
from linux/sched.h to linux/sched/signal.h in Linux commit 2a1f062a
(sched/headers: Move signal wakeup [...]). Fix this by including
linux/sched/signal.h if we have it during the configure test.

A false negative on this configure test doesn't break the build, but
it disables one of our safeguards preventing incorrect negative
dentries at runtime. The function fatal_signal_pending() hasn't
changed in quite some time (except for what header it lives in); it
was introduced in Linux 2.6.25 via Linux commit f776d12d (Add
fatal_signal_pending). So to try to avoid this mistake again in the
future, make it so a missing fatal_signal_pending() breaks the build
if we're on Linux 2.6.25+.

Change-Id: Id0b91b2f24e2ea87c9c900076ab7ab1fcab3d304
Reviewed-on: https://gerrit.openafs.org/14508
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoafs: Cleanup afsincludes.h indentation 71/14471/5
Cheyenne Wills [Tue, 22 Dec 2020 18:06:42 +0000]
afs: Cleanup afsincludes.h indentation

Clean up the indentation of preprocessor statements

Remove commented out code.

Change-Id: I37fec6f15a8972651ef05aa00580a2628e0a1a46
Reviewed-on: https://gerrit.openafs.org/14471
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoafs: Clean up VNOPS/afs_vnops_attrs.c indentation 70/14470/8
Cheyenne Wills [Tue, 22 Dec 2020 18:03:33 +0000]
afs: Clean up VNOPS/afs_vnops_attrs.c indentation

Clean up the indentation of preprocessor statements, add #endif comments
where helpful.

Clean up whitespace in code indentation.

Change-Id: I5e6eb3d8ad2688f2b5a56b760d1c1f031f6ca9ec
Reviewed-on: https://gerrit.openafs.org/14470
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: Remove dead reference to rxk_ListenerProc 38/14038/3
Cheyenne Wills [Thu, 21 Nov 2019 18:58:04 +0000]
rx: Remove dead reference to rxk_ListenerProc

rx_prototypes.h has an extern definition for rxk_ListenerProc.  That
function was removed in commit e261238470ed28ee7c1068d914de171b34033e09
'SOLARIS: Perform daemon syscalls as kernel threads'

Remove the extern definition/reference in rx_prototypes.h

Change-Id: I9f845f24f993f5a5cfb353e594ecdf3ec6de73ab
Reviewed-on: https://gerrit.openafs.org/14038
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: BuildBot <buildbot@rampaginggeek.com>

3 years agoafs: Clean up afs_init.c indentation 69/14469/8
Cheyenne Wills [Wed, 23 Dec 2020 20:25:31 +0000]
afs: Clean up afs_init.c indentation

Clean up the indentation of preprocessor statements, add #endif comments
where helpful.

Clean up whitespace in code indentation.

Change-Id: Id7eeeabfea52c99f783e23468cfc89ce9ed8eae5
Reviewed-on: https://gerrit.openafs.org/14469
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agotests: fix potential divide by zero condition 01/14501/3
Cheyenne Wills [Fri, 22 Jan 2021 19:48:21 +0000]
tests: fix potential divide by zero condition

Running clang's static analysis revealed a possible divide by zero
condition.

There is a random chance of the divide by zero.

- it has to be in the first pass of the main loop testing events
  (counter = 0)
- 90% chance path :   if (counter < (NUMEVENTS -1) &&
                          random() % 10 == 0)      -- needs to be false
- 25% chance path:    if (random() % 4 == 0)       -- needs to be true

if the above conditions are met, the statement
    int victim = random() % counter
is a divide by zero.

Add a check to ensure the counter is greater than zero.  Add a comment
to document that only events prior to the current event are randomly
selected.

Change-Id: I4b4e73fa324842bb504bcc952079af15aea8a6a3
Reviewed-on: https://gerrit.openafs.org/14501
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>

3 years agoRemove overflow check from update_nextCid 96/14496/3
Benjamin Kaduk [Thu, 14 Jan 2021 18:20:59 +0000]
Remove overflow check from update_nextCid

The rx_nextCid global has been an unsigned type since
http://gerrit.openafs.org/11106 (which was actually merged before
the refactoring of overflow check to avoid signed integer overflow)
and thus there is no need to avoid signed overflow.  The per-connection
cid has been unsigned since the IBM import.

The natural unsigned behavior on overflow of wrapping is the desired
behvaior here, so just remove the extra logic and always increment.

Change-Id: I2d9fd24082b762eb871199da3ac1cc0983764585
Reviewed-on: https://gerrit.openafs.org/14496
Reviewed-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: update_nextCid overflow handling is broken 92/14492/3
Jeffrey Altman [Thu, 14 Jan 2021 14:57:13 +0000]
rx: update_nextCid overflow handling is broken

The overflow handling in update_nextCid() produces a rx_nextCid
value of 0x80000001 which itself is out of the valid range.   When
used to construct the first call of a new connection the connection
id for the call becomes 0x80000002, and all subsequent connections
also trigger the overflow handling and thus also receive connection
id 0x80000002.

If the same connection id is used for multiple connections from
the same endpoint the accepting rx peer will be very confused.

When authenticated connections are used, the CHALLENGE/RESPONSE
will fail because of a mismatch in the connection's callNumber
array.

If an initiator makes only a single connection to a given rx peer,
that connection would succeed, but once multiple connections are
initiated all communication from a broken initiator to any rx peer
will fail.

The incorrect overflow calculation was introduced by
39b165cdda941181845022c183fea1c7af7e4356 ("Move epoch and cid
generation into the rx core").

This change corrects the overflow value to become

  1 << RX_CIDSHIFT

Change-Id: If36e3aa581d557cc0f4d2d478f84a6593224c3cc
Reviewed-on: https://gerrit.openafs.org/14492
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: rx_InitHost do not overwrite RAND_bytes rx_nextCid 91/14491/2
Jeffrey Altman [Thu, 14 Jan 2021 14:41:39 +0000]
rx: rx_InitHost do not overwrite RAND_bytes rx_nextCid

39b165cdda941181845022c183fea1c7af7e4356 ("Move epoch and cid
generation into the rx core") introduced the use of RAND_bytes()
to generate the initial 'rx_nextCid' but failed to remove the

  rx_nextCid = ((tv.tv_sec ^ tv.tv_usec) << RX_CIDSHIFT;

assignment inherited from IBM/Transarc.

At Thu, 14 Jan 2021 08:25:36 GMT the IBM inherited calculation
overflows the value CID range.   This triggers broken overflow
logic in update_nextCid().

Change-Id: Ib7283def1ded9792d394133a3969a6d86f3a6123
Reviewed-on: https://gerrit.openafs.org/14491
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Jeffrey Hutzelman <jhutz@cmu.edu>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agovolser: correctly attribute 'vos partinfo' errors 89/14489/2
Mark Vitale [Tue, 12 Jan 2021 20:50:07 +0000]
volser: correctly attribute 'vos partinfo' errors

Since the original IBM code import, the 'vos partinfo' error message
blames the wrong command, 'vos listpart':

    $ vos partinfo afs01.sinenomine.net
    <unknown>: Could not get afs tokens, running unauthenticated.
    Could not fetch the list of partitions from the server
    Possible communication failure
    Error in vos listpart command.
    Possible communication failure

Correct the error message to specify 'vos partinfo' instead.

Change-Id: I966dee8c679db89c7ed5ce21d31ebc7424803cf2
Reviewed-on: https://gerrit.openafs.org/14489
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agovol: prevent salvage segfault for orphaned vnode with out-of-range parent 85/14385/3
Mark Vitale [Thu, 20 Aug 2020 20:09:02 +0000]
vol: prevent salvage segfault for orphaned vnode with out-of-range parent

While salvaging a RW volume, salvager may segfault if it encounters an
orphaned directory with a parent vnode that does not exist.  For
example, if the large vnode index contains a maximum vnode of 2901, any
parent vnode encountered that is larger than 2901 will result in an
out-of-bounds reference to our vnode essence array, leading to a
segfault or undefined behavior.

Modify the logic to check for out-of-bounds parent vnodes, and log them
rather than segfaulting.

Change-Id: I49f53935830fbb428fe0bff04c33248d3806a4b2
Reviewed-on: https://gerrit.openafs.org/14385
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoafs: Remove SRXAFSCB_GetDE 88/14488/2
Andrew Deason [Sat, 9 Jan 2021 18:50:03 +0000]
afs: Remove SRXAFSCB_GetDE

The GetDE RPC has been commented out from afscbint.xg effectively
since it was introduced, but we still define the SRXAFSCB_GetDE server
stub for it.

This is useless, but also potentially dangerous, since the stub
routine just returns success, without populating the output arguments.
One of the output arguments is a string, and so if this RPC is
actually run, the rxgen-generated server code will try to xdr_string()
that string. Since we never set it to anything, this will result in
xdr_string trying to dereference a NULL pointer.

None of this actually happens currently, since the GetDE RPC is
commented out. But to avoid the above situation if it's ever
uncommented, remove the useless SRXAFSCB_GetDE function.

Change-Id: I6ef478ee69a8de1ac14baa86aa82489181d67452
Reviewed-on: https://gerrit.openafs.org/14488
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoWINNT: Restore missing '#ifdef PC' 87/14487/2
Andrew Deason [Sat, 9 Jan 2021 18:47:09 +0000]
WINNT: Restore missing '#ifdef PC'

Commit 339167ef (Remove dead code) meant to remove the '#ifdef notdef'
block in here, but we accidentally also removed the subsequent '#ifdef
PC'.

This file may not be very important, since WINNT still builds with
this mistake, but an unbalanced #ifdef is potentially super confusing,
so fix it.

Change-Id: I100792830e1bed0af08bcb81a34bb185b2c6358a
Reviewed-on: https://gerrit.openafs.org/14487
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoMove key-related warnings to common server code 31/10831/6
Andrew Deason [Mon, 10 Feb 2014 21:57:43 +0000]
Move key-related warnings to common server code

Each server process can log a couple of different warnings about the
server keys found on disk:

- If afsconf_GetLatestKey() returns success (indicating a single-DES
  key is present), we call LogDesWarning().

- If afsconf_CountKeys() returns 0 (indicating there are no keys at
  all on disk), we log a warning that all authenticated access will
  fail.

Currently, the code to do these checks and log the relevant warning is
duplicated across the startup code for nearly every server process. To
avoid this duplication, and to make sure the checks aren't
accidentally skipped for anyone, move these checks to
afsconf_BuildServerSecurityObjects, which every server process calls.

We must add an additional parameter to
afsconf_BuildServerSecurityObjects to handle the different logging
mechanism these servers use, but afsconf_BuildServerSecurityObjects is
declared in a public header (cellconfig.h), and is exported in a
public library (libafsauthent). So to avoid changing a public symbol,
introduce a new variant of the function, called
afsconf_BuildServerSecurityObjects_int. Declare this in a new internal
header, authcon.h.

We don't have easily-usable logging functions for upserver and butc,
so just don't log the warnings for those. For ubik servers, don't
update ubik_SetServerSecurityProcs to use the new function; the
initial call to afsconf_BuildServerSecurityObjects_int in the server's
startup code will cover logging the warning on startup.

Change-Id: I5d5fceefdaf907f96db9f1c0d21ceb6957299a59
Reviewed-on: https://gerrit.openafs.org/10831
Tested-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: Split out rxi_ConnectionMatch 03/13603/9
Andrew Deason [Sun, 9 Dec 2018 00:05:36 +0000]
rx: Split out rxi_ConnectionMatch

Split out the connection-matching logic in rxi_FindConnection into a
new function called rxi_ConnectionMatch, so we can use it in other
functions in future commits.

This commit should have no visible impact; it is just code
reorganization.

Change-Id: Ibacec68d268977a8a2a3aca172653fc088334da6
Reviewed-on: https://gerrit.openafs.org/13603
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: Remove unneeded rxi_ReceiveDataPacket params 02/13602/9
Andrew Deason [Sat, 8 Dec 2018 23:16:43 +0000]
rx: Remove unneeded rxi_ReceiveDataPacket params

rxi_SplitJumboPacket doesn't use its 'host', 'port', or 'first'
arguments, and rxi_ReceiveDataPacket only uses its 'host' and 'port'
arguments to pass to rxi_SplitJumboPacket.

Remove these unused parameters from both functions. While we're
changing rxi_SplitJumboPacket anyway, move the declaration for
rxi_SplitJumboPacket to rx_internal.h, so it's no longer in a public
header.

This commit should have no visible impact; it is just code
reorganization.

Change-Id: I16a7f613957d8cd2d415f65fa083e11d8a13edc8
Reviewed-on: https://gerrit.openafs.org/13602
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: Avoid new server calls for big-seq DATA pkts 76/13876/7
Andrew Deason [Thu, 19 Sep 2019 17:18:08 +0000]
rx: Avoid new server calls for big-seq DATA pkts

We currently never open our receive window to more than 32 packets. If
we received a DATA packet for an unrecognized call with a seq of 33 or
more, the packet is almost certainly from a previously-running call
that we were restarted during.

As described in commit 7b204946 (rx: Avoid lastReceiveTime update for
invalid ACKs) and commit "rx: Avoid new server calls for non-DATA
packets", clients can get confused when we respond to calls in these
situations, so drop the packets instead.

Change-Id: I5b3a699bf245375e92ac97a24ad3638cbb3b8f3c
Reviewed-on: https://gerrit.openafs.org/13876
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agoafs: Fix XBSD check for VNOVAL va_uid 73/14473/2
Andrew Deason [Wed, 23 Dec 2020 18:44:35 +0000]
afs: Fix XBSD check for VNOVAL va_uid

Commit e86eb73e (obsd-vattrs-20040125) introduced an XBSD-specific
check to detect some unchanged attributes. But the #ifdef for XBSD for
the va_uid section was added in the middle of an HPUX-specific block
by mistake.

Move this #ifdef one level higher, so it's actually used on BSD
platforms.

Change-Id: I606f87f21d6c4830ed8bcf50abd6fb5807868ff5
Reviewed-on: https://gerrit.openafs.org/14473
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Tim Creech <tcreech@tcreech.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: Avoid new server calls for non-DATA packets 58/13758/10
Mark Vitale [Thu, 8 Aug 2019 22:18:22 +0000]
rx: Avoid new server calls for non-DATA packets

Normally, a client starts a new Rx call by sending DATA packets for
that call to a server, and rxi_ReceiveServerCall on the server creates
a new call struct for that call (since we don't recognize it as an
existing call).

Under certain circumstances, it's possible for a server to see a
non-DATA packet as the first packet for a call, and currently
rxi_ReceiveServerCall will create a new server call for any packet
type. The call cannot actually proceed until the server receives data
from the client (and goes through the challenge/response auth
handshake, if needed), but usually this is harmless, since the
existence of any packets for a particular call channel indicate that
the client is trying to run such a call. The server will respond to
the client with ACKs to indicate that it is missing the needed DATA
packet(s), and the client will send them and the call can proceed.

However, if a call is in the middle of running when the server is
restarted, the client may be sending ACKs for a pre-existing call that
the server doesn't know about. In this case, the server generates ACKs
that indicate the server has not received any DATA packets, which may
appear to violate the protocol, depending on the prior state of the
call (e.g. the server appears to try to move the window backwards).

Clients should be able to detect this and kill the call, but many do
not. For many OpenAFS releases before commit 7b204946 (rx: Avoid
lastReceiveTime update for invalid ACKs), the client will get confused
in this situation and will keep the call open forever, never making
progress.

There isn't any benefit to creating a new server call in these
situations, so just ignore non-DATA packets for unrecognized calls, to
avoid stalled calls from such clients. Those clients will not get a
response from the server, and so the call will eventually die from the
normal Rx call timeout.

Change-Id: I565625ba8b6901f9b745124a8816a9ba816c0264
Reviewed-on: https://gerrit.openafs.org/13758
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

3 years agorx: Avoid lastReceiveTime update for invalid ACKs 75/13875/6
Andrew Deason [Wed, 28 Aug 2019 03:58:23 +0000]
rx: Avoid lastReceiveTime update for invalid ACKs

Currently, we ignore ACK packets in a few cases:

- If the ACK appears to move the window backwards (if firstPacket is
  smaller than call->tfirst).

- If the ACK appears to have been received out of order (if
  previousPacket is smaller than call->tprev).

- If the ACK packet appears truncated.

In all of these cases, we ignore the ACK packet completely in our ACK
processing code (rxi_ReceiveAckPacket), but we still process the
packet at higher levels (rxi_ReceivePacket). Notably, this means we
update call->lastReceiveTime after rxi_ReceiveAckPacket returns, even
for ACK packets we haven't really looked at.

Normally this does not cause any noticeable problems, because such
packets should either never be encountered, or only consist of a small
number of packets that are mixed in with valid packets.

However, if our peer is a server, and it is restarted in the middle of
a call, our peer may exclusively send us packets that fall into the
above categories. (This does not happen if our peer is a client,
because clients just ignore packets for calls they do not recognize.)
For example:

Consider a call where a client is sending data to a server, and the
server restarts after the client has sent a DATA packet with sequence
number 1000. The server may then start responding to the client with ACKs
with firstPacket set to 1, since the restarted server has no knowledge
of the call's state.

In this case, a firstPacket of 1 is well below where our window was,
so all of the ACKs from the server are ignored. But we keep updating
call->lastReceiveTime for all of these packets, and so the call stays
alive forever until an idle-dead or hard-dead timeout activates (if
any are set).

As another example, consider the case where a client is sending data
to a server, and the server receives a full window of packets (say, 16
packets), has not yet passed any data to the application yet, and the
server restarts. The restarted server then starts responding to the
client with ACKs with firstPacket set to 1, and previousPacket set to
0. We also ignore all of the ACKs from the server in this case,
because even though firstPacket looks sane, it looks like
previousPacket has gone backwards. We still update
call->lastReceiveTime for each ignored ACK we get, keeping the call
alive.

Before commit 4e71409f (Rx: Reject out of order ACK packets) was
introduced in 1.6.0, neither of these issues could occur. That commit
introduced the issue specifically if previousPacket goes backwards;
that is, if the server restarts before firstPacket moves forwards.

Commit 8d359e6d (rx: Remove duplicate out of order ACK check) in 1.8.0
introduced the issue when 'firstPacket' goes backwards, since
previously the FIRSTACKOFFSET-based check caused us to ignore those
packets without updating call->lastReceiveTime. That is, if the server
restarts after firstPacket moves forwards.

In this commit, we still ignore packets in the above cases, but we
also avoid updating lastReceiveTime when we update such packets, to
make sure that we do not keep a call alive solely from receiving these
invalid packets.

Alternatively, we could change our logic to immediately abort calls
where firstPacket moves backwards (since this violates the Rx
protocol), or to not ignore some packets where previousPacket goes
backwards (since these calls may be recoverable). And we could also
skip updating lastReceiveTime for invalid packets of other types. But
for now, this commit just avoids updating lastReceiveTime for invalid
ACK packets, in order to just try to restore our behavior before
1.6.0, while still retaining the benefits of ignoring out-of-order
ACKs. Further changes in this area can potentially be handled
separately by future commits.

Also increment the spuriousPacketsRead counter for packets that we
ignore in this way (which we used to do for some packets before commit
8d359e6d), so we are not entirely silent about ignoring them.

Written in collaboration with mvitale@sinenomine.net.

Change-Id: Ibf11bcb2417d481ab80cf4104f2862d1d6502bf4
Reviewed-on: https://gerrit.openafs.org/13875
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>