openafs.git
8 months 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>

8 months 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>

8 months 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>

8 months 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>

8 months 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>

8 months 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>

8 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

9 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

10 months 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>

11 months 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>

11 months 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>

11 months 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>

11 months 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>

11 months 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>

11 months 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>

11 months agorx: Introduce ack_is_valid 74/13874/6
Andrew Deason [Wed, 28 Aug 2019 22:12:53 +0000]
rx: Introduce ack_is_valid

Take some of our existing logic for ignoring invalid ACK packets and
split it out into a separate function, ack_is_valid. This just makes
it easier to add more complex logic in here and write longer comments
explaining the decisions.

Note that the bug mentioned regarding the previousPacket field was
introduced in IBM AFS 3.5, and was fixed in OpenAFS in commit bbf92017
(rx: rxi_ReceiveDataPacket do not set rprev on drop), included in
OpenAFS 1.6.23.

This commit incurs no functional change; it is just code
reorganization.

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

11 months agorx: For AFS_RXERRQ_ENV, retry sendmsg on error 24/14424/4
Andrew Deason [Mon, 2 Nov 2020 19:52:25 +0000]
rx: For AFS_RXERRQ_ENV, retry sendmsg on error

When AFS_RXERRQ_ENV is defined, we currently end up doing something
like this for our sendmsg abstractions:

    if (sendmsg(...) < 0) {
        while (rxi_HandleSocketError(sock))
            ;
        return error;
    }
    return success;

This means that when sendmsg() returns an error, we process the socket
error queue before returning an error.

The problem with this is that when we receive an ICMP error on our
socket, it creates a pending socket error that is returned for any
operation on the socket. So, if we receive an ICMP error after trying
to contact any peer, sendmsg() could return an error when trying to
send for any other peer. Even though there is no issue preventing us
from sending the packet, we'll fail to actually send the packet
because sendmsg() returned an error. This effectively causes an extra
outgoing packet drop, possibly delaying the related RPC.

To avoid this, change Rx to retry the sendmsg call when it returns an
error, since the error may be due to an unrelated ICMP error.

To avoid needing to implement this retry loop in multiple places, move
around our sendmsg code for AFS_RXERRQ_ENV, so that the higher-level
function rxi_NetSend performs the retry and checks for socket errors
(instead of the lower-level rxi_Sendmsg or osi_NetSend). Also change
our functions to process socket errors to be more consistent between
kernel and userspace: now we always have rxi_HandleSocketErrors, which
runs a loop around the platform-specific osi_HandleSocketError.

With this commit, osi_HandleSocketError is now required to be
implemented when AFS_RXERRQ_ENV is defined. We hadn't been
implementing this for UKERNEL, so just turn off AFS_RXERRQ_ENV for
UKERNEL.

Thanks to mbarbosa@sinenomine.net for discovering and providing
information about the relevant issue.

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

11 months agorx: Save errno in pthread rxi_Sendmsg 23/14423/3
Andrew Deason [Mon, 2 Nov 2020 19:16:41 +0000]
rx: Save errno in pthread rxi_Sendmsg

Currently, our pthread version of rxi_Sendmsg uses 'errno' in some
logic if sendmsg fails, but we do so after calling functions that
might alter errno (e.g. fflush).

To make sure we get the correct errno value, save the value of errno
right after sendmsg returns an error. Reorganize this function a bit
to help make the logic easier to follow.

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

11 months agoafsio: readdir/fidreaddir commands 81/12381/3
Michael Meffie [Mon, 23 May 2016 00:35:26 +0000]
afsio: readdir/fidreaddir commands

Add the readdir/fidreaddir sub-commands to afsio dump AFS3 directory
objects.  This command dumps the raw directory object to stdout.  Pipe
the output to a program, such as the afsdump_dirlist program (from the
CMU dumpscan tool kit), to parse the directory object.

Example usage:

   afsio readdir -dir /afs/mycell/mypath/somedir | afsdump_dirlist

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

11 months agovol: always build vol-bless utility 64/14464/2
Mark Vitale [Mon, 7 Dec 2020 19:42:54 +0000]
vol: always build vol-bless utility

In order to avoid future bit-rot, always build vol-bless.  Also add it
to the clean rule.  However, continue to leave it undistributed and
uninstalled by default.

Change-Id: I3d2dc94c28a7feeb20167223655e97538e807ce6
Reviewed-on: https://gerrit.openafs.org/14464
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>

11 months agoFix spelling of struct rx_ackPacket in comment 68/14468/2
Benjamin Kaduk [Fri, 11 Dec 2020 19:09:20 +0000]
Fix spelling of struct rx_ackPacket in comment

A comment in rx_packet.h referred to the size of struct rx_ackpacket,
but the actual structure is spelled with a majuscule 'P'.

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

11 months agorx: Reorganize LWP rxi_Sendmsg to use 'goto error' 22/14422/2
Andrew Deason [Mon, 2 Nov 2020 19:11:49 +0000]
rx: Reorganize LWP rxi_Sendmsg to use 'goto error'

Our LWP version of rxi_Sendmsg can allocate an fd_set, but we don't
free the fd_set if sendmsg() returns certain errors afterwards.

To make sure we go through the same cleanup code for the different
possible error code paths, reorganize the function to go through a
'goto error'-style destructor. This also makes our return codes a bit
more consistent; we should always return -errno now for errors.

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

11 months agoaudit: Add missing AUD_TSTT case 66/14466/2
Andrew Deason [Thu, 10 Dec 2020 20:17:56 +0000]
audit: Add missing AUD_TSTT case

In commit 9ebff4c6 (OPENAFS-SA-2018-001 audit: support butc types),
several new butc-related audit data types were added. In the
AIX-specific audmakebuf() function, the case for the AUD_TSTT type is
missing the actual "case" clause in the code, causing AUD_TSTT types
to be treated as invalid (and so falling through to the
"AFS_Aud_EINVAL" case).

Add the "case" for AUD_TSTT, so it's treated properly on AIX. Note
that the non-AIX printbuf() already handled this properly, so no
changes are needed there.

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

11 months agovol: add vol-bless to .gitignore 63/14463/2
Mark Vitale [Mon, 7 Dec 2020 19:40:33 +0000]
vol: add vol-bless to .gitignore

No functional change is incurred by this commit.

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

11 months agovol: make vol-bless buildable again 62/14462/2
Mark Vitale [Mon, 7 Dec 2020 18:13:28 +0000]
vol: make vol-bless buildable again

The vol-bless utility is not built by default and so is subject to
bit-rot.  Thus commit 170dbb3ce301329ff127bb23fb588db31439ae8d 'rx: Use
opr queues' overlooked vol-bless.c when adding includes for users of
struct rx_queue.

Add the required #include <rx/rx_queue.h> so vol-bless builds again.

Note to maintainers: this change is only required for 1.8.x and later;
vol-bless builds fine in 1.6.x and earlier releases.

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

11 months agoafs: consolidate duplicated wait-for-cache-drain code 78/13278/8
Mark Vitale [Thu, 9 Aug 2018 21:40:09 +0000]
afs: consolidate duplicated wait-for-cache-drain code

Consolidate duplicated logic into a new routine
afs_MaybeWaitForCacheDrain().

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

11 months agoafs: more cache truncation stats 68/13168/10
Michael Meffie [Mon, 20 Jun 2016 19:29:45 +0000]
afs: more cache truncation stats

Add counters for cache too full and waiting to drain occurrences. These
will be used in later commits to indicate how often the cache truncation
is required and how often the cache manager is waiting for cache
truncation to complete.

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

11 months agokauth: Add support for updated audit facility 82/13782/28
Cheyenne Wills [Wed, 25 Sep 2019 19:39:40 +0000]
kauth: Add support for updated audit facility

New functionality was added to the audit facility that allows multiple
audit logs. The updated audit interfaces require a specific calling
sequence even if multiple audit logs are not used.

Support for multiple auditlogs is not supported for kauth. Since kauth
does not use libcmd for processing the command line, and adding support
for multiple audit log instances requires additional effort, that is not
warranted.

Update kauth to follow the proper calling sequences for the audit
facility.

Update help message and manpage entries for -auditlog and
-audit-interface.  Make note that multiple -auditlogs are not supported.

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

12 months agoAdd command line support for multiple audit logs 76/13776/28
Cheyenne Wills [Fri, 4 Dec 2020 17:16:57 +0000]
Add command line support for multiple audit logs

Gerrits #13774 (audit: Support multiple audit interfaces and interface
options) and #13775 (audit: Add cmd helper for processing audit options)
added support in the audit facility for multiple audit logs.

Add command line support to use multiple audit logs for daemons that use
libcmd for command line processing: bosserver, buserver, butc,
fileserver, volserver, ptserver, and vlserver.

Update the daemons to add a call to audit_open, and where possible add a
call to audit_close when shutting down the daemon.

Update help message and manpage entries for -auditlog and
-audit-interface

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

12 months agoaudit: Add cmd helper for processing audit options 75/13775/24
Cheyenne Wills [Mon, 9 Nov 2020 19:27:36 +0000]
audit: Add cmd helper for processing audit options

osi_audit_cmd_Options will handle the processing for the
-audit-interface and -audit-log command line options.

The auditlog / audit-interface options are used by several services;
this new helper routine provides a simple method to process the audit
related command line options in a consistent fashion.

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

12 months agoaudit: Support multiple audit interfaces and interface options 74/13774/23
Cheyenne Wills [Fri, 13 Nov 2020 18:20:15 +0000]
audit: Support multiple audit interfaces and interface options

Currently, the audit subsystem only allows for one audit log to exist
for the entire process.  This can make it cumbersome to use for sites
that have multiple tools or destinations that want to read the audit
data. For example, to feed the audit data to two separate scripts, one
script needs to read the data, and retransmit the data to the second
script.

To make such a setup easier, change the audit system to allow for
multiple audit logs to exist at once.  To allow callers to associate
each audit log with an interface, we change the syntax for the value to
the -auditlog parameter to the following:

  [interface:]filespec[:options]

For example:

  -auditlog sysvmq:/tmp/msgqueue

To accommodate the existing -audit-interface parameter, change the
behavior of -audit-interface so that it sets the default audit interface
if none is specified for -auditlog.  This allows existing users of
-audit-interface to experience the same behavior as before.

In order to implement this, change the audit API and all existing audit
interfaces to avoid using per-interface globals, and instead allocate
per-instance contexts during startup.  Also change the code so the audit
message is constructed inside audit.c, instead of via a per-interface
callback, which eliminates the duplicated logic in each interface's
append_msg(), and lets us avoid holding 'audit_lock' during message
construction.

While we're changing the audit API, also introduce a few new operations:
open_interface, close_interface and set_options.  This commit and the
existing interfaces do not make use of these new functions, but future
commits will do so.

This commit also only changes the audit subsystem itself to be able to
handle multiple audit logs, and doesn't change any command-line parsing
logic.  Future commits will add the command-line parsing logic changes
required so daemons can actually configure multiple interfaces.

Thanks to Andrew Deason (adeason@sinenomine.net) for providing the
changes needed to reduce holding the 'audit_lock' and improve
performance as well as providing input during the development of this
change.

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

12 months agoLINUX: Return errors in our d_revalidate 17/14417/2
Andrew Deason [Mon, 26 Oct 2020 17:35:32 +0000]
LINUX: Return errors in our d_revalidate

In our d_revalidate callback (afs_linux_dentry_revalidate), we
currently 'goto bad_dentry' when we encounter any error. This can
happen if we can't allocate memory or some other internal errors, or
if the relevant afs_lookup call fails just due to plain network
errors.

For any of these cases, we'll treat the dentry as if it's no longer
valid, so we'll return '0' and call d_invalidate() on the dentry.
However, the behavior of d_invalidate changed, as mentioned in commit
afbc199f1 (LINUX: Avoid d_invalidate() during
afs_ShakeLooseVCaches()). After a certain point in the Linux kernel,
d_invalidate() will also effectively d_drop() the given dentry,
unhashing it. This can cause getcwd() calls to fail with ENOENT for
those directories (as mentioned in afbc199f1), and can cause
bind-mount calls to fail similarly during a small window.

To avoid all of this, when we encounter an error that prevents us from
checking if the dentry is valid or not, we need to return an error,
instead of saying 'yes' or 'no'. So, change
afs_linux_dentry_revalidate to jump to the 'done' label when we
encounter such errors, and avoid calling d_drop/d_invalidate in such
cases. This also lets us remove the 'lookup_good' variable and
consolidate some of the related logic.

Important note: in older Linux kernels, d_revalidate cannot return
errors; callers just interpreted its return value as either 'valid'
(non-zero) or 'not valid' (zero). The treatment of negative values as
errors was introduced in Linux commit
bcdc5e019d9f525a9f181a7de642d3a9c27c7610, which was included in
2.6.19. This is very old, but technically still above our stated
requirements for the Linux kernel, so try to handle this case, by
jumping to 'bad_dentry' still for those old kernels. Just do this with
a version check, since no configure check can detect this (no function
signatures changed), and the only Linux versions that are a concern
are quite old.

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

12 months agovldb_check: Check for volume lock inconsistencies 07/14307/8
Michael Meffie [Mon, 17 Aug 2020 19:44:55 +0000]
vldb_check: Check for volume lock inconsistencies

Verify the a lock timestamp is set if, and only if, a lock volume
operation flag is also set.

When running vldb_check with the -fix option, fix the inconsistent
entries by setting the lock timestamp to the current time if a lock flag
is set, or by setting the VLOP_DELETE flag if the lock timestamp is set
but no lock flags are set. (The VLOP_DELETE flag is the flag set by the
'vos lock command, and is shown in vos output as "delete/misc".)

Volume lock fields can be put into an inconsistent state, at least, by
interupted vos rename operations, due to bugs in vos rename. When the
volume lock timestamp and lock flags are in this inconsistent state, the
volume is locked, but that is not indicated by 'vos listvldb'. The
volume can be unlocked by issuing 'vos unlock'.

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

12 months agovsprocs: Remove dead code 04/14004/5
Michael Meffie [Fri, 27 Dec 2019 16:53:05 +0000]
vsprocs: Remove dead code

Remove the dead code in UV_VolumeMove() commented out with the macro
ENABLE_BUGFIX_1165.

Remove two commented out lines of code in UV_ConvertRO().

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

12 months agovos: Cleanup function definitions 09/14009/5
Cheyenne Wills [Thu, 5 Nov 2020 20:50:59 +0000]
vos: Cleanup function definitions

The functions defined within vos.c are not referenced outside of vos.c
but are not declared as static.

Convert the functions within vos.c to static declarations.

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

12 months agovos: Remove dead code 08/14008/5
Cheyenne Wills [Thu, 5 Nov 2020 20:49:54 +0000]
vos: Remove dead code

Clean out dead code from vos.c

GetVolumeType - not referenced anywhere
CompareVLDBEntry - commented out since 1st git commit
osi_audit - Comment indicates this might have been needed at one point.
            Builds without it.  Does not look like the vos executable
            is pulling in any of the audit code.
RestoreVolume - remove stale comment about typo previous to openafs 1.0
RemoveSite - remove commented out partition check

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

12 months agovolser: take RO volume offline during convertROtoRW 40/14340/6
Marcio Barbosa [Thu, 3 Sep 2020 23:57:34 +0000]
volser: take RO volume offline during convertROtoRW

The vos convertROtoRW command converts a RO volume into a RW volume.
Unfortunately, the RO volume is not checked out from the fileserver
during this process. As a result, accesses to the volume being converted
can leave volume objects in an inconsistent state.

Moreover, consider the following scenario:

1. Create a volume on host_b and add replicas on host_a and host_b.

$ vos create host_b a vol_1
$ vos addsite host_b a vol_1
$ vos addiste host_a a vol_1

2. Mount the volume:

$ fs mkmount /afs/.mycell/vol_1 vol_1
$ vos release vol_1
$ vos release root.cell

3. Shutdown dafs on host_b:

$ bos shutdown host_b dafs

4. Remove RO reference to host_b from the vldb:

$ vos remsite host_b a vol_1

5. Attach the RO copy by touching it:

$ fs flushall
$ ls /afs/mycell/vol_1

6. Convert RO copy to RW:

$ vos convertROtoRW host_a a vol_1

Notice that FSYNC_com_VolDone fails silently (FSYNC_BAD_STATE), leaving
the volume object for the RO copy set as VOL_STATE_ATTACHED (on success,
this volume should be set as VOL_STATE_DELETED).

7. Add replica on host_a:

$ vos addsite host_a a vol_1

8. Wait until the "inUse" flag of the RO entry is cleared (or force this
to happen by attaching multiple volumes).

9. Release the volume:

$ vos release vol_1

Failed to start transaction on volume 536870922
Volume not attached, does not exist, or not on line
Error in vos release command.
Volume not attached, does not exist, or not on line

Notice that this happens because we cannot mark an attached volume as
destroyed (FSYNC_com_VolDone).

To avoid the problem mentioned above and to prevent accesses to the
volume being converted, take the RO volume offline before converting it
to RW.

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

12 months agovos: Cleanup indentation whitespace 07/14007/5
Cheyenne Wills [Tue, 10 Nov 2020 16:17:16 +0000]
vos: Cleanup indentation whitespace

Fix the indentation whitespace in vos.c, and remove double blank
lines.  No functional change.

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

12 months agovos: Return true when GetServerAndPart finds a site 06/14006/4
Michael Meffie [Thu, 19 Dec 2019 01:30:17 +0000]
vos: Return true when GetServerAndPart finds a site

Change the GetServerAndPart() function to return true when a volume site
in the vldb entry is found. Do not change the output arguments unless
the site is found.  Also, add a function comment header and fix some
comment typos in this function.

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

12 months agovos: Add missing -partition requires -server checks 05/14005/7
Michael Meffie [Mon, 23 Dec 2019 23:37:21 +0000]
vos: Add missing -partition requires -server checks

The `vos remove` command was missing a check for the -server option when
the -partition option is given. This command requires the -server option
when the -partition is given, as documented in the man page.

The `vos syncvldb` command performed the check for the -server option
when the -partition option is given, but in the wrong location.

As documented, the `vos unlockvldb` command permits the -partition
option without a -server option, in which case all of the volumes listed
in the VLDB with sites on the specified partition are unlocked.
However, this command incorrectly issued an RPC to a volume server at
address 0.0.0.0 when only the partition is given.

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

12 months agovos: avoid double release of a volume lock 26/14426/2
Mark Vitale [Thu, 5 Nov 2020 23:16:51 +0000]
vos: avoid double release of a volume lock

To update a volume entry in the VLDB, vos commands typically lock the
volume entry via VL_SetLock, then call VL_UpdateEntryN, then release the
lock via VL_ReleaseLock.  However, some vos commands exploit the
optional lock release flags of VL_UpdateEntryN to combine the update and
unlock operations into a single RPC.  This approach requires extra care
to ensure that VL_ReleaseLock is issued for a failed VL_UpdateEntryN,
but NOT for a successful VL_UpdateEntryN.

Unfortunately, the following commands have success paths that fall
through to the error path, resulting in a double release of the volume
lock:
 - vos convertROtoRW
 - vos release

A second VL_ReleaseLock of a volume entry that has already been unlocked
via VL_UpdateEntryN is essentially a harmless no-op (other than negating
any benefit of exploiting the VL_UpdateEntryN lock flags).  However, if
there is a race with another volume operation on the same volume, it is
possible for this bug to release the volume lock of a different volume
operation.

This problem has been present in 'vos release' since OpenAFS 1.0.  This
problem has been present in 'vos convertROtoRW' since the command's
introduction in commit 8af8241e94284522feb77d75aee8ea3deb73f3cc
vol-ro-to-rw-tool-20030314.

Properly maintain state to avoid unlocking a volume (with
VL_ReleaseLock) that has already been unlocked (via VL_UpdateEntryN).

Thanks to Andrew Deason for discovering the issue and suggesting the
fix.

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

12 months agovolser: document 'vos restore -readonly' restriction 48/14348/4
Mark Vitale [Fri, 28 Aug 2020 20:19:29 +0000]
volser: document 'vos restore -readonly' restriction

Commit 0c03f8607e15 vos-command-enhancements-20011008 introduced the
'vos restore' -readonly option, which allows the restored volume to be
RO instead of the default RW.  The commit message documents the
following restriction:

- ... This option causes the restored volume to be an RO volume.  It is
  not permitted to restore an RO volume when the associated RW volume
  already exists.  While it is possible to restore an RW volume where an
  RO volume exists, caution should be used to avoid doing this with VLDB
  entries created by 'vos restore -readonly', since such entries have
  their ROVOL and RWVOL ID's set to the same thing.

Document this restriction in the 'vos restore' man page, and in a code
comment.

No functional change is incurred by this commit.

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

12 months agovolser: improve error checking for 'vos restore' 47/14347/4
Mark Vitale [Fri, 28 Aug 2020 19:42:06 +0000]
volser: improve error checking for 'vos restore'

UV_RestoreVolume2 calls VLDB_GetEntryByName to obtain information for
sanity checking, but only checks for a VL_NOENT error code; other codes
are thus ignored, which may lead to confusing results.

Add an additional error check for 'vos restore' (and other callers of
UV_RestoreVolume2) to stop and issue an error message if a non-VL_NOENT
error code is received from VLDB_GetEntryByName.

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

12 months agovolser: fix 'cant' typos 46/14346/4
Mark Vitale [Mon, 31 Aug 2020 18:30:26 +0000]
volser: fix 'cant' typos

Correctly spell "can't" in a log message and a comment.

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

12 months agoafs: avoid panic in DNew when afs_WriteDCache fails 04/13804/13
Mark Vitale [Fri, 19 Jul 2019 18:41:55 +0000]
afs: avoid panic in DNew when afs_WriteDCache fails

afs_WriteDCache may fail for an IO error, or if interrupted (EINTR).
Unfortunately, DNew will panic in this case, crashing the entire
machine.

In order to avoid an outage in this case, don't panic.  Instead, reflect
the error back to the caller of DNew.

While here, add Doxygen comments to DNew.

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

12 months agoafs: remove redundant assignment 02/13802/13
Mark Vitale [Mon, 19 Aug 2019 19:43:09 +0000]
afs: remove redundant assignment

DRelease has two assignments for tp = entry->buffer; remove the second
(redundant) one.

Introduced with 0284e65f97861e888d95576f22a93cd681813c39 'dir: Explicitly
state buffer locations for data'.

No functional change should be incurred by this commit.

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

12 months agodir: check DNew return code 01/13801/14
Mark Vitale [Wed, 5 Feb 2020 22:49:03 +0000]
dir: check DNew return code

Commit 0284e65f97861e888d95576f22a93cd681813c39 'dir: Explicitly state
buffer locations for data' changed DNew and DRead to return a return
code.  However, the callers of DNew were not modified to check the new
return code.  (This commit applied only to the implementations dealing
with AFS directories, in afs/afs_buffer.c and dir/dir.c.  The ubik
implmentations of DNew and DRead, dealing with ubik databases, were not
modified.)

Modify all (non-ubik) callers of DNew to check the return code.  In
addition, modify code as needed so return codes are properly propagated
to the callers.

While here, add Doxygen comments for AddPage and FindBlobs.

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

12 months agoRemove unused xdr types 04/14404/4
Andrew Deason [Mon, 19 Oct 2020 23:30:27 +0000]
Remove unused xdr types

Numerous types and constants are defined in our various RPC-L files
that are never used or referenced by anything. Remove them.

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

12 months agovolser: apply static keyword to VolPartitionInfo definition 21/13321/3
Benjamin Kaduk [Sun, 2 Sep 2018 22:10:56 +0000]
volser: apply static keyword to VolPartitionInfo definition

The function declaration was already marked as static; mark the
definition as well for consistency (and consistency with the other
helpers in this file).

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

12 months agodir: check afs_dir_Create return code in afs_dir_MakeDir 00/13800/13
Mark Vitale [Mon, 4 Mar 2019 01:20:58 +0000]
dir: check afs_dir_Create return code in afs_dir_MakeDir

afs_dir_MakeDir() ignores the return code from afs_dir_Create() for the
'.' and '..' ("dot" and "dotdot") directories.  This has been the case
from the earliest implementation (MakeDir() calling Create()) in the
original IBM import.

Instead, check the return codes to prevent the possibility of creating
malformed directories.

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

12 months agoptserver: rename NameToID and IDToName helpers 20/13320/3
Benjamin Kaduk [Sun, 2 Sep 2018 22:06:38 +0000]
ptserver: rename NameToID and IDToName helpers

These helper function names alias the names of public RPCs and can
cause confusion when grepping the code.  Rename them in a different
style to provide greater hamming distance between the various
functions involved in handling these RPCs.

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

12 months agodir: check afs_dir_MakeDir return code in DirSalvage 99/13799/10
Mark Vitale [Mon, 4 Mar 2019 01:51:45 +0000]
dir: check afs_dir_MakeDir return code in DirSalvage

Since the original IBM import, DirSalvage() has ignored the return code
from afs_dir_MakeDir() (f.k.a. MakeDir).  This has been safe because, as
the comment states, afs_dir_MakeDir returns no (non-zero) error code.

In preparation for a future commit, add a check for the return from
afs_dir_MakeDir and remove the comment.

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

12 months agodir: distinguish logical and physical errors on reads 98/13798/10
Mark Vitale [Thu, 30 Jan 2020 19:04:05 +0000]
dir: distinguish logical and physical errors on reads

The directory package (src/dir) salvage routines DirOK and DirSalvage
check a global variable 'DErrno' to distinguish logical errors (e.g.
short read) from physical errors (e.g. EIO).  However, since the
original IBM import, this logic has not worked correctly because there
is no longer any code that sets the value of DErrno - its value is
always zero.

Instead, modify all implementations of ReallyRead to optionally return
the errno for low-level IO errors.

Also, create a new userspace-only variant - DReadWithErrno() - of the
src/dir/buffer.c version of DRead (the version called by DirOK and
DirSalvage, and the only caller of ReallyRead) to return the ReallyRead
errno upon request.

Also create an analogous variant of afs_dir_GetBlobs,
afs_dir_GetBlobsWithErrno().

Finally, convert DirOK and DirSalvage to use the new variants and
replace DErrno with equivalent logic.  Remove all other references to
DErrno.

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

12 months agoafs: Log pid with disk cache read errors 16/14416/2
Andrew Deason [Mon, 26 Oct 2020 17:19:19 +0000]
afs: Log pid with disk cache read errors

Log the current pid (and procname) when we complain about an error
when reading from CacheItems in afs_UFSGetDSlot. These errors can
result in confusing situations, so it can be helpful to know at least
what process saw the error.

Our logic for logging this information is getting a bit large, so also
move this to a new function, LogCacheError.

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

12 months agoroken: use strtok_r from roken 91/13891/14
Cheyenne Wills [Tue, 8 Oct 2019 17:54:58 +0000]
roken: use strtok_r from roken

Windows standard library doesn't provide strtok_r.  Use the strtok_r
that is provided from roken.

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

12 months agoImport of code from heimdal 90/13890/14
Heimdal Developers [Tue, 8 Oct 2019 16:47:05 +0000]
Import of code from heimdal

This commit updates the code imported from heimdal to
5dfaa0d10b8320293e85387778adcdd043dfc1fe (git2svn-syncpoint-master-311-g5dfaa0d10)

New files are:
roken/strtok_r.c

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

12 months agoroken: add strtok_r to the imported file list 89/13889/14
Cheyenne Wills [Tue, 8 Oct 2019 17:02:59 +0000]
roken: add strtok_r to the imported file list

Import the strtok_r function which is needed by audit for parsing
command line options.

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

12 months agoDetect realloc failure 13/13313/3
Benjamin Kaduk [Sun, 2 Sep 2018 02:47:39 +0000]
Detect realloc failure

While reviewing other commits, a call to realloc() was discovered that
would leak memory on failure (by virtue of always assigning the realloc()
return value to the pointer holding the input address, even when the
return value is NULL).  Check for failure and return early in that case
(giving an incomplete list of events).

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

12 months agoptserver: move IDToName, NameToID to ptprocs.c and make static 19/13319/3
Benjamin Kaduk [Sun, 2 Sep 2018 22:03:38 +0000]
ptserver: move IDToName, NameToID to ptprocs.c and make static

These two helpers are only used in implementing server-side RPC handlers,
and having to track the codeflow across files is unhelpful.  Move them
into the file where they're used, make them static, and remove the
prototypes from ptrototypes.h (which is not an installed header, so
there is no API/ABI breakage).

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

12 months agoAssign explicit opcodes to butc RPCs 17/13317/2
Benjamin Kaduk [Sun, 2 Sep 2018 21:35:42 +0000]
Assign explicit opcodes to butc RPCs

This should prevent inadvertent reassignment if additional RPCs
are introduced in the future.

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

13 months agovlserver: Return VL_DBBAD on unhash failure 84/13384/4
Andrew Deason [Mon, 12 Nov 2018 21:06:09 +0000]
vlserver: Return VL_DBBAD on unhash failure

If we try to delete a vlentry, and the vlentry cannot be found on one
of its hash chains, we cannot unhash the vlentry properly and the
operation fails with VL_NOENT. This results in the following error
messages to the user:

        $ vos delentry 123456
        Could not delete entry for volume 123456
        You must specify a RW volume name or ID (the entire VLDB entry will be deleted)
        VLDB: no such entry
        Deleted 0 VLDB entries

This is confusing, because VL_NOENT can also occur if the user
specifies a volume that does actually not exist. This situation is
indicative of database corruption, usually because of a ubik
transaction that was only half-applied, or because of other ubik bugs
in the past.

The situation can only really be fixed by repairing the database, so
return VL_DBBAD in this case instead, to more clearly indicate that
something is wrong with the database, and not a problem with the
arguments the caller provided.

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

13 months agovlserver: Add VL_DBBAD error code 83/13383/4
Andrew Deason [Mon, 12 Nov 2018 20:41:44 +0000]
vlserver: Add VL_DBBAD error code

The VL_ error table currently doesn't have an error code to indicate
that an operation cannot succeed because the database is corrupted.
There are a few error codes for specific cases of errors that are
probably the result of corruption (like VL_IDALREADYHASHED, or
VL_EMPTY), but these are only for specific cases and indicate rather
low-level internal problems.

There are some instances where the real problem preventing an
operation from succeeding is that the database is just corrupt or
inconsistent in some way, and the administrator must repair the
database before it can succeed. And we currently don't have any way of
indicating that situation via an error code.

So, introduce the VL_DBBAD code, to indicate this situation. Error
codes already exist in other tables for similar situations, such as
PRDBBAD, and KADATABASEINCONSISTENT.

This commit does not use the new error code anywhere; we just
introduce it into the VL_ error table, so comerr-using applications
will be able to interpret it.

Note that the VL_DBBAD error code has been recognized by the AFS
Assigned Numbers Registry as recorded in the ticket history of
<https://rt.central.org/rt/Ticket/Display.html?id=134817>

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

13 months agoptserver: move allocation out of put_prentries() into listEntries() 15/13315/3
Benjamin Kaduk [Thu, 30 Aug 2018 14:54:23 +0000]
ptserver: move allocation out of put_prentries() into listEntries()

put_prentries() is a helper function for listEntries(), but the contract
between the two is rather odd -- put_prentries() is expected to notice
when the backing store has not yet been allocated and silently allocate
it, even though there is only the single caller and the allocation could
be done in the caller.

Move the allocation to the caller and adjust the "buffer is full"
logic accordingly, and normalize the initialization of the output
array to just use calloc() instead of individual memset()s when
populating each entry.

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

13 months agovolser: Avoid calling osi_audit before audit init 72/13772/20
Cheyenne Wills [Thu, 6 Jun 2019 20:08:53 +0000]
volser: Avoid calling osi_audit before audit init

volmain.c calls osi_audit before the audit facility is fully
initialized.

Commit 16d67791 (auditlogs-for-everyone-20050702) introduced the
-auditlog parameter; it appears that it didn't remove the call
to osi_audit (right after osi_audit_init) that was called before command
line argument processing. This resulted in calling the audit facility
before it was fulling initialized with the -auditlog and
-audit-interface parameters.  The 16d67791 commit replicated the
osi_audit call after command line processing.

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

13 months agoaudit: Always call pthread_once in osi_audit_init 03/14403/4
Andrew Deason [Mon, 19 Oct 2020 21:07:44 +0000]
audit: Always call pthread_once in osi_audit_init

Currently, we skip the pthread_once call in osi_audit_init if
audit_lock_initialized is set. But this is somewhat pointless, since
pthread_once will effectively do this check itself, and better (it
will wait if osi_audit_init is actively running in another thread).

So just get rid of audit_lock_initialized, and replace the other
assert for audit_lock_initialized with another plain pthread_once
call.

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