On Linux and possibly other platforms, if configure is run with the
--disable-static option, anything linking against liboafs_rxkad.la will
fail:
gcc [...] -o afsd [...] .../src/auth/liboafs_auth.la [...] -lafshcrypto -lrokenafs -lpthread -lresolv
/home/build/openafs/src/rxkad/.libs/liboafs_rxkad.so: undefined reference to `rxs_Ref'
/home/build/openafs/src/rxkad/.libs/liboafs_rxkad.so: undefined reference to `rxs_SetRefs'
/home/build/openafs/src/rxkad/.libs/liboafs_rxkad.so: undefined reference to `rxs_DecRef'
collect2: error: ld returned 1 exit status
make: *** [afsd] Error 1
The references to these symbols were added in commit 9d7b94493c (rx: Use
atomics for rx_securityClass refcounts), but that commit didn't add them
to the liboafs_rx.la.sym symbol file. Add them to allow the build to
complete with --disable-static.
Change-Id: I14b348d1be6bf6e8f512d1cac6a70a58fd6f39a1
Reviewed-on: https://gerrit.openafs.org/15988
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Currently, our HashTable for FileEntry structs is a fixed size
(FEHASH_SIZE, 512). If we have a large number of callbacks, this can
lead to very long hash chains, which can cause the fileserver to
consume high amounts of CPU when finding, deleting, or especially
adding new callbacks. This is especially bad since callbacks are
managed under the global H_LOCK.
To improve this, use our configured callback limit (-cb) to build an
appropriately-sized HashTable at initialization, instead of using a
hard-coded size. We compute this by dividing the -cb value by the
desired hash chain length (FE_CHAIN_TARGET, 16), then rounding up to
the nearest power of 2 at least as big as the old FEHASH_SIZE, 512.
For DAFS, a copy of our HashTable is included in the fsstate.dat file
on disk, so changing our hashtable size potentially changes the data
in fsstate.dat. However, we currently do not read in the hashtable
data from fsstate.dat, since it's not very useful; we only write it
out in case other utilities or older fileservers need it.
Older fileservers, however, will not read an fsstate.dat with a
hashtable that does not have exactly FEHASH_SIZE. So if we have a
differently-sized hash table, we're breaking compatibility with those
fileservers anyway, so don't write out our hashtable data at all.
This commit also changes the format of the callback.dump file, since
it did not allow for a variable-length hashtable. We create a new
MAGICV3 magic to define the new format, and add some logic to the
'cbd' utility to understand it. We still write out our hashtable to
this file since it's for debugging, and the hashtable data may be
useful in that context.
Move FEHash to callback.c, since it now relies on the callback.c-only
FEhashsize, and move FEHASH_SIZE_OLD along with it.
[adeason@sinenomine.net: Don't write out non-old-sized hashtable. Move
FEHash &co to callback.c. Various other minor edits.]
Change-Id: I54de91c54c5fcb526f880bc63ba10c1b3eb0aaf0
Reviewed-on: https://gerrit.openafs.org/14731
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Currently, if the fileserver tries to restore fsstate.dat and the
included FE hashtable does not match our in-memory size (FEHASH_SIZE),
we'll discard the entire state file. This is unnecessary, since we
could just add the FEs to our hashtable ourselves; the on-disk
hashtable data is not required.
Furthermore, loading the hashtable from disk isn't even terribly
efficient when the hashtable sizes do match, since any indices need to
be converted from the on-disk values to in-memory values
(fe_OldToNew). So if we're processing the FE hashtable indices anyway,
we might as well just add them to the hashtable like normal, and
ignore the values on disk.
So, ignore loading the "fehash" data from the fsstate file, and just
add file entries to our hashtable as we read them in. This ends up
being slightly faster, simpler, and more flexible, since we don't need
to worry about reflecting any changes to our hashtable in the
fsstate.dat file.
At least for now, we still write our hashtable data to fsstate.dat, in
case other utilities (or older fileservers) need it.
[adeason@sinenomine.net: Converted to always ignore on-disk hashtable
data.]
Change-Id: Ie53a24a5d24906ac55d13eb22713b73dc4bfaab2
Reviewed-on: https://gerrit.openafs.org/14738
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Log how long it took to restore our fsstate.dat data, and how many
FEs/CBs we loaded. This may be of interest to anyone looking at the
impact of different relevant options (such as -fs-state-verify), and
how much callback state is being restored.
Note that we're not adding any new messages, just adding some
additional info to an existing message.
Change-Id: I86bc384919d1c6c141558c23d2c53def2e8916f8
Reviewed-on: https://gerrit.openafs.org/14737
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Some code paths in our state-restoring logic don't log the specific
reason why restoring state failed. These cases shouldn't happen often,
but try to at least log something for them, to try to make sure we
always give a reason.
[adeason@sinenomine.net: Added more messages.]
Change-Id: Ib7c082c4ec221460f7c07be577e78c700f8850a5
Reviewed-on: https://gerrit.openafs.org/14728
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
When verifying the sanity of host and callback state at startup and
shutdown, dafileserver attempts to detect loops in various structures
by imposing arbitrary hard-coded limits (e.g.
FS_STATE_HCB_MAX_LIST_LEN, 100000 callbacks per host).
However, it is possible for AFS clients with heavy RW workloads to
legitimately exceed some of these limits, if the dafileserver itself
is configured with -cb higher than 100000, for instance.
When this occurs, the dafileserver will falsely invalidate the
fsstate.dat file at shutdown:
"cb_stateVerifyFCBList: error: list length exceeds 100000 (h->index=1);
assuming there's a loop"
The dafileserver will then refuse to load the invalid fsstate.dat file
at startup, defeating the callback preservation feature just when it
is most sorely needed - when there are hundreds of thousands of
callbacks.
These arbitrary limits are unnecessary, since we already know the
number of structures we're processing for all of these cases (CBs,
FEs, and hosts). So just get rid of the hard-coded limits, and use the
actual limit for the relevant structure instead:
For CBs and FEs, we can never have more than 'cbstuff.nblks' entries
whether we're loading or saving state, so just use that limit for
simplicity.
For hosts, the number of entries is either in the dump file (when
restoring state), or just in our 'hostCount' global (when saving
state). To make sure a corrupted state file doesn't cause the host
limit to be unreasonably high, impose a new, higher, arbitrary limit
on the number of hosts (FS_STATE_H_MAX_LIST_LEN, set to 100M).
[adeason@sinenomine.net: Raise limits for _all_ structures.]
Change-Id: I525916dc6189b887e4625371e81afac98f12a651
Reviewed-on: https://gerrit.openafs.org/14727
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
When the fileserver saves its callback state to disk on shutdown,
fs_stateCreateDump sets state->mode to FS_STATE_DUMP_MODE. But this is
after some code paths have already looked at state->mode, such as
fs_stateSave -> h_stateVerify -> h_Enumerate_r -> h_stateVerifyHost ->
h_stateVerifyAddrHash. This doesn't cause any problems right now,
since the value of FS_STATE_DUMP_MODE is 0, and we zero the whole
'state' struct when it is initialized.
This is confusing, though, so move the assignment of
FS_STATE_DUMP_MODE to earlier in the state-saving process, before
anything looks at it.
To try to avoid possible uninitialized use of state->mode in the
future, change the FS_STATE_*_MODE constants so that none of them are
0, and make sure that code checking the 'mode' checks that the mode is
a valid constant.
[adeason@sinenomine.net: Added state mode checking.]
Change-Id: Ifa5e5aabdb7c7b4a50a1b397620aafa7c48c223e
Reviewed-on: https://gerrit.openafs.org/14726
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
In a few perl scripts (that happen to be for macOS), we check whether
'$? >> 8' is zero to see whether a command failed. But the value of
'$? >> 8' is just the exit code of the process. If it instead, for
example, was killed by a signal, there is no exit code, and '$? >> 8'
may not be accurate (the terminating signal is in '$? & 127'). We should
check if $? is nonzero at all to see if an error happened.
To avoid any possible issues, update all of our checks for command
failure to check if $? is nonzero, instead of '$? >> 8'. In notarize.pl,
print out the whole status in addition to the exit code, just to be
clear in case the command somehow terminates from a signal.
Change-Id: I07ed145fae45d0bdc77c1249cf4d89bd5ba5cc56
Reviewed-on: https://gerrit.openafs.org/15980
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
To create the OpenAFS client for macOS, the current process involves
building the code, signing the binaries, creating the package, and
notarizing it. Each step is typically performed separately and requires
distinct parameters and credentials, making this process cumbersome and
difficult to follow.
To simplify this process, introduce the following '--with' options:
--with-macos-app-key
--with-macos-inst-key
--with-macos-keychain-profile
These options allow users to specify the credentials needed for signing
and notarizing the package upfront.
With these enhancements, users will be able to perform the entire
workflow - building, signing, creating, and notarizing the package -
with a single 'make packages' command, significantly simplifying this
process.
Change-Id: Ibf114f4f5bbe9bc72f37adc487c046e5243f5a97
Reviewed-on: https://gerrit.openafs.org/15977
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
At WWDC 2021, Apple introduced notarytool, a new utility for interacting
with the Apple notary service. Concurrently, Apple deprecated altool for
notarization purposes, with plans to discontinue its functionality on
November 1, 2023.
Currently, notarize.pl relies on the deprecated altool, which is no
longer supported. This commit updates this script to use notarytool,
ensuring it remains fully functional for notarizing the OpenAFS package.
Note that the arguments for notarize.pl have changed. Users can no
longer provide a plain password or keychain reference using the
@keychain prefix. Instead, a keychain profile for the credentials must
be created first, with the profile name provided as an argument.
This change is mandated by the new notarytool workflow.
Additionally, update pkgbuild.sh.in, as this script calls notarize.pl
if the --apple-id option is provided. Since notarize.pl now requires a
keychain profile name instead of an Apple ID and password, remove the
--apple-id option from pkgbuild.sh.in and introduce a new option,
--keychain-profile, which specifies the name of the keychain profile to
be used by notarize.pl.
Change-Id: I8b513e3eebb38e49f0d7c5ff9ea4e1d46e87aa3f
Reviewed-on: https://gerrit.openafs.org/15976
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Since commit f7ddab6606 (Merge pam into the kauth configure option), we
set enable_pam="yes" if --enable-kauth was given to configure. That
makes sense, but we also do that if --disable-kauth was given to
configure. This doesn't make any sense, since we disable enable_pam and
enable_kauth if no --enable-kauth/--disable-kauth option was given at
all.
This doesn't affect much, since INSTALL_KAUTH will be disabled for
--disable-kauth, so the items in src/pam won't be installed in this case
regardless. But to make our behavior more consistent, and actually avoid
kauth stuff for --disable-kauth, set enable_pam to the same thing as
enable_kauth when an option is given, so pam is disabled when kauth is
disabled.
Change-Id: Ib36e9fdffb93d659e1fadab5f1bb1334770806b8
Reviewed-on: https://gerrit.openafs.org/15995
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Several build artifacts are missing from the gitignore rules when
building on macOS. For example:
$ git status --porcelain
?? src/libafs/afs.x86_darwin_210.plist
?? src/platform/DARWIN/PrivilegedHelper/org.openafs.privhelper
Update the gitignore rules to ignore generated darwin plist files and
the prefpane "privileged helper" binary.
Change-Id: I684ec9edde3da76cc83a644f0150da800f6f3db5
Reviewed-on: https://gerrit.openafs.org/16002
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
OpenAFS builds both shared and static libraries by default. On most
platforms, each library is built as a shared library libfoo.so.x.y.z,
and as a static library libfoo.a. libtool refers to this as the 'svr4'
style of libraries on AIX.
On AIX, traditionally libraries are built with the shared library,
static library, and various versions and 64-bit variants and such all
contained in the same libfoo.a. libtool refers to this as the 'aix'
style of libraries on AIX.
libtool defaults to the 'aix' style on AIX, and so this is how libtool
attempts to build our libraries on AIX. For many of our libraries, even
though the build completes successfully, this results in broken
binaries because our Makefile rules assume the shared and static library
files have different names.
For example, here is the install rule for librokenafs:
install: $(SHARED_LIBS) librokenafs.a
$(LT_INSTALL_DATA) librokenafs.la $(DESTDIR)$(libdir)/librokenafs.la
$(RM) $(DESTDIR)$(libdir)/librokenafs.la
$(INSTALL_DATA) librokenafs.a $(DESTDIR)$(libdir)/librokenafs.a
On AIX, the LT_INSTALL_DATA step will install the shared library
librokenafs.a into $(DESTDIR)$(libdir). Then the INSTALL_DATA step will
install the static library librokenafs.a into the same location,
deleting the shared library file.
When the user tries to run an executable, they get an error, because the
shared library librokenafs is not installed at all:
$ /opt/openafs/bin/pts
exec(): 0509-036 Cannot load program /opt/openafs/bin/pts because of the following errors:
0509-150 Dependent module /opt/openafs/lib/librokenafs.a(librokenafs.so.2) could not be loaded.
0509-152 Member librokenafs.so.2 is not found in archive
To avoid this, the user can run configure with --with-aix-soname=svr4 to
build all of our shared libraries in the 'svr4' style. In the above
example, this causes LT_INSTALL_DATA to install the shared librokenafs
as librokenafs.so.* into $(DESTDIR)$(libdir), and then the static
library is installed as librokenafs.a like normal. The resulting
binaries can then run without issue.
To make it so users don't need to specify --with-aix-soname=svr4 to get
a working build, change the default behavior to --with-aix-soname=svr4
by passing aix-soname=svr4 to LT_INIT.
However, just specifying LT_INIT([aix-soname=svr4]) alone does not work,
due to a bug in libtool: https://savannah.gnu.org/support/?111161
To workaround this bug, also explicitly turn on shared and static
libraries by default in LT_INIT, even though they are already on by
default. Using --disable-shared or --disable-static should still be
honored as normal; the LT_INIT arguments just specify the default
values.
Ideally, we would install our shared libraries in the 'aix' style, so a
single libfoo.a contains all of the necessary .o and .so files for that
library. However, changing our build system to do this is difficult. And
historically, OpenAFS has built shared libraries on AIX in the 'svr4'
style; that is how we built shared libraries in OpenAFS 1.6 and earlier,
before we converted to using libtool.
Change-Id: Ifd2538c635323c568f0d8b2e621cc0b8594721ae
Reviewed-on: https://gerrit.openafs.org/15983
Reviewed-by: Ben Huntsman <ben@huntsmans.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
When building the libafs kernel module, we pass various flags to the
compiler and linker, some of which no longer work with the clang-based
Open XL C 17.1 and newer:
On XL C 16.1 and below, the -M argument causes a file to be generated
with a listing of all the files upon which the .o is dependent. This
file is created with the same name as the .o file, but with a .u
extension. With the Open XL C 17.1+ compilers, the -M file acts
similarly, but if -o is specified, the dependency file is created
with the name specified by the -o argument. This causes the
dependency file to be created but not the actual object code file.
Because the compiler invocation succeeds, this repeats until it is
time to link, which fails because the .o files are not actually code,
but ascii files containing the dependency information. We don't
actually use the .u files for anything, so to avoid this issue, remove
the -M flag.
The -H8 flag is a linker argument that aligns the text, data, and loader
sections of the output file so that each section begins on a file offset
that is a multiple of 8. On XL C 16.1 and below, the compiler passes
unrecognized arguments to the linker, and so giving -H8 to the compiler
works fine. But with XL C 17.1+, the compiler does not pass unrecognized
arguments to the linker, and throws an error instead. To avoid this,
move -H8 from KDEFS to LDFLAGS, so it's given only to the linker and not
to the compiler.
The -q64 flag no longer exists with XL C 17.1+, and is replaced by -m64.
Use ${XCFLAGS64} instead of -q64, which will choose the correct flag to
use, like we did in commit aa82a8894f (export: Use XCFLAGS64 for -q64).
Change-Id: I2527a604000507ec27adb37338d98a6852ad3131
Reviewed-on: https://gerrit.openafs.org/15974
Reviewed-by: Ben Huntsman <ben@huntsmans.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
clang-19 is flagging a warning/error in rxkad/test/stress.c as
unused-but-set-variables
stress.c:51:9: error: variable 'nonoauth' set but not used [-Werror,-Wunused-but-set-variable]
51 | int nonoauth = 0;
| ^
1 error generated.
The variable nonoauth is initialized and conditionally incremented, but
is not used elsewhere.
Remove the nonoauth variable.
Change-Id: I9a97a3980e5b6ff462f860d7bdae4ee8d7b22971
Reviewed-on: https://gerrit.openafs.org/15984
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Convert the prefpane logic for writing to config files to use
privhelper, so it can work with macOS 10.8+. With this commit, the
"CellServDB Editor" and "Parameter" tabs in the prefpane should now
work.
Specifically, define the privhelper task "write". Define a new variant
to TaskUtil's executePrivTask() called executePrivTaskWrite() to use the
new task type, and convert the prefpane code to use it.
Most files are straightforward, but converting writeAfsdOption() is a
little awkward due to the repeated checking of useAfsdConfVersion. Just
remove the writeAfsdOption() method and make the caller choose between
writeNewAfsdOption() and writeOldAfsdOption().
The new "write" task takes a new argument (data) to indicate what data
to write to the config file in question. Like the "backup" task, it also
takes a "filename" argument that is checked against our list of config
files.
Change-Id: Ib93da5a75a92e3507c15f9ee03eb8f7fbad7d088
Reviewed-on: https://gerrit.openafs.org/15960
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Convert the prefpane logic to backup various config files to privhelper,
so it can work with macOS 10.8+.
Specifically, define the new privhelper task "backup". Define a new
variant to TaskUtil's executePrivTask() called executePrivTaskBackup()
to use the new task type, and convert the prefpane code to use it.
Remove the backupFile() method, since it's now unused.
The new "backup" task has an argument (filename), so we don't need to
define separate tasks for each file we want to backup. But check the
given filename against a short list of files we know we deal with, just
to make sure we don't accidentally mess with some other system file,
since we're running as root.
Change-Id: I066d8e95a50a52f7509487f9228ba03dd027ea36
Reviewed-on: https://gerrit.openafs.org/15959
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Convert the logic to "start/stop OpenAFS" in the prefpane to use
privhelper, so it can work with macOS 10.8+.
Specifically, define these new privhelper tasks:
- afsd_start
- afsd_stop
And convert our start/stop-related code in the prefpane to use them.
Change-Id: I4cc5b500fb09b83a4007e2c7c5045e93fe1f4ced
Reviewed-on: https://gerrit.openafs.org/15958
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Convert the logic for the "start at login" option in the prefpane to use
privhelper, so it can work with macOS 10.8+.
Specifically, define these new privhelper tasks:
- startup_enable
- startup_disable
- startup_check
And convert our startup-related logic in the prefpane to use them.
Get rid of our now-unused method executeTaskWithAuth() and related
methods.
Change-Id: I2cb4c31f964529ab1af43ab7828c14eba7354af0
Reviewed-on: https://gerrit.openafs.org/15957
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
The prefpane for macOS runs as the logged-in user, but needs root access
for some operations: starting/stopping the client, editing various
configuration files like CellServDB, etc. We currently use functions
like AuthorizationExecuteWithPrivileges() to run commands with root
privileges directly, but this approach no longer works as of macOS 10.8
(Mountain Lion); the relevant functions have been removed.
Instead, a new approach exists as of macOS 10.6 (Snow Leopard). The
prefpane application itself cannot gain root privileges, but we can
provide another daemon process that runs as root, and the PrefPane sends
requests to that process to perform the privileged operations we need.
In this commit, create a separate helper program called PrivilegedHelper
(privhelper for short) that serves this purpose. Define the
executePrivTask() method in TaskUtil to handle communicating with
privhelper over XPC.
This commit does not define any of the tasks that privhelper will
actually perform; this just implements privhelper itself. Later commits
will add and use various privileged tasks in privhelper.
In order for privhelper to be able to run as root, both privhelper and
the prefpane itself must be code signed and the relevant apple team id
must be specified in their Info.plist when they are built, as well as
inside privhelper.c. Currently, we have
no way of specifying code signatures info during the build, since all code
signing is done when generating packages (via pkgbuild.sh) after
binaries are built. For now, just put a commented-out section in
src/platform/DARWIN/AFSPreference/Info.plist and
src/platform/DARWIN/PrivilegedHelper/privhelper-info.plist and a
placeholder in src/platform/DARWIN/PrivilegedHelper/privhelper.c to show
how to add this information. The package builder must add their own team
id to these before privhelper can work properly.
The privhelper tool checks that the calling user has authorization to
run commands as root (via AuthorizationCopyRights()), and that the
calling process is either our AFSBackgrounder menu bar or the prefpane.
We use xpc_connection_set_peer_code_signing_requirement() for this where
available, but fallback to using SecCodeCheckValidity() with
SecCodeCreateWithXPCMessage() or
xpc_dictionary_get_audit_token()/SecCodeCopyGuestWithAttributes() if
needed.
Change-Id: I724b6d486ee5397c89c79e589ddcb2a5987a895b
Reviewed-on: https://gerrit.openafs.org/15956
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
The default Xcode compiler options are insufficient to fail the build if
a method is not found. For example, a typo in the name of method
'componentsJoinedByString' results in the following build warning:
/TaskUtil.m:70:62: warning: instance method '-componentsJoinedBySting:'
not found (return type defaults to 'id') [-Wobjc-method-access]
NSLog(@"Task failed: %@ %@ status:%d.", taskName,
[args componentsJoinedBySting:@", "], status);
Because this is only flagged as a warning, the build completes
successfully. When this code runs, the AFSBackgrounder merely logs a
runtime exception every time it passes through the erroneous code. This
is a silent failure, unless you happen to know how to check for
AFSBackgrounder log messages:
$ log show --predicate 'process=="AFSBackgrounder"'
Add compiler flag '-Werror=objc-method-access' to all of our xcode-built
project files to treat this case as an error instead of a warning, so
the build will fail.
[adeason@sinenomine.net: Update all xcode project files.]
Change-Id: I69a9f45deb6710a50590bd79daf07466332a6ad1
Reviewed-on: https://gerrit.openafs.org/14586
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Commit 91378d93b9 (rx: Don't send packets to localhost if -rxbind set)
added the rxi_GetLocalAddr() function, but missed the prototype. Add the
function prototype to fix a GCC missing-prototype warning when building
the linux kernel module.
Fixes the build error when building the kernel module for Linux 6.8
or later when the tree was configured with --enable-checking:
.../src/libafs/MODLOAD-6.8.0-49-generic-SP/rx.c:9693:1: error: no
previous prototype for ‘rxi_GetLocalAddr’ [-Werror=missing-prototypes]
9693 | rxi_GetLocalAddr(struct sockaddr_in *sin)
| ^~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
Change-Id: I43bd56fa28d258be509b1d1381e2f7d76ad5a532
Reviewed-on: https://gerrit.openafs.org/15978
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
The comments above VCHash claim that PFlush relies on the fid uniq not
being used in VCHash, but this doesn't appear to be true.
However, we do have some code that relies on VCHash not using uniq:
afs_GenFakeFid. Update the VCHash comment to point this out.
Change-Id: Ib1e0c1700c4cd13b95ac57522645ce73d2b24a08
Reviewed-on: https://gerrit.openafs.org/15042
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
For some platforms (SOLARIS, FBSD), shutting down the libafs client
currently involves sending a packet to localhost to wakeup the listener
thread. If rx is bound to a specific host address (the -rxbind option
was passed to afsd), this won't work, because rx won't receive packets
sent to localhost. This results in the client hanging forever when
trying to 'umount afs', until a packet is otherwise sent to the rx
socket.
To fix this, send the packet to the bound host address instead, if
-rxbind was given. Otherwise, send the packet to localhost, like before.
Introduce the small helper function rxi_GetLocalAddr() to consolidate
the logic of what address to use.
Change-Id: I26fa5194b726ed753779faa07142fec647228b44
Reviewed-on: https://gerrit.openafs.org/15906
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
Remember what local address we're bound to (as passed to rx_InitHost(),
which is given when -rxbind is passed to various programs), like we do
for our local port with rx_port. Store this in a new internal global
variable, rx_host.
This commit just introduces the new variable, but does not use it.
Future commits will add code that make use of this information.
Change-Id: I5ac1a60b10f6c28df1e6620e30515bcdfd749311
Reviewed-on: https://gerrit.openafs.org/15905
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
The indexGreaterThanIndex function returns the closest index from the
index set that is greater than a specified index. This function is
typically used in a loop to iterate through a list of items until it
returns NSNotFound, indicating the end of the list.
In AFSCommanderPref.m, the following pattern is being used:
int index = 0;
do {
...
} while((index = [... indexGreaterThanIndex:index]) != NSNotFound);
The issue arises because indexGreaterThanIndex returns an NSUInteger,
while the loop uses an int for index. If NSNotFound is cast to an int,
it becomes -1, causing the loop to never terminate and leading to a
crash.
To fix this problem, change the type of index from int to NSUInteger to
ensure proper comparison and termination of the loop when NSNotFound is
returned.
Change-Id: I0b8b38d201b500c2d6d7778f0132ce17f7c04ac8
Reviewed-on: https://gerrit.openafs.org/15961
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Address potential memory issues by setting workIPArray to nil after
releasing it in commitModify. This prevents dangling pointers and
ensures consistency with rollbackModify, which already includes this
safeguard. Without this change, PrefPane can crash upon closing the
window used for setting IP addresses of cells in the CellServDB.
Change-Id: Iea56343d734f854b76b74c961d2df05bcf4a9332
Reviewed-on: https://gerrit.openafs.org/15962
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
On FreeBSD, we commonly generate some kernel header files in
src/libafs/kconf-GENERIC, ever since commit a9c1939eeb (FBSD: Use
GENERIC kernel headers by default). Ignore it.
Change-Id: I2ce14361b130f20861133a8358aaa4085073e897
Reviewed-on: https://gerrit.openafs.org/15902
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Ben Huntsman <ben@huntsmans.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Most of our binaries link with -lhcrypto or our internally-built
-lafshcrypto for heimdal-provided crypto code. But we also build an LWP
variant, libafshcrypto_lwp.a, which avoids any pthread calls and is to
be used when linking against LWP instead of pthreads.
Currently, a few binaries link against LWP, but also link against
$(LIB_hcrypto) (which gets expanded to -lhcrypto or -lafshcrypto). This
includes most binaries in src/tests, src/rx/test, and src/rxkad/test,
and has been the case since these commits:
236cb51b83 rx: Cleanup and build src/rx/test
1fe1dac4c5 rxkad: Cleanup and build src/rxkad/test
80c23d958c tests: Make src/tests buildable
On most platforms, this isn't so noticeable; the binaries just get built
for LWP, but have a pthreaded hcrypto and contain references to
pthreads. (hcrypto itself just uses a single global mutex that is
briefly held, and so tends to not cause cause any problems running with
LWP.)
On AIX, this causes a build error when building with --disable-shared,
because we build these programs without linking to the pthreads library:
/opt/IBM/xlC/16.1.0/bin/cc [...] -o test-setgroups test-setgroups.o [...]/lib/libopr.a -lafshcrypto -lrokenafs
ld: 0711-317 ERROR: Undefined symbol: .pthread_mutex_lock
ld: 0711-317 ERROR: Undefined symbol: .pthread_mutex_unlock
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
make: *** [test-setgroups] Error 8
(Without --disable-shared, this succeeds, presumably because we link
against the libtool libafshcrypto.la library, which is smart enough to
know it depends on pthreads.)
To fix this, link against libafshcrypto_lwp.a for these LWP binaries
instead of $(LIB_hcrypto), like other LWP binaries do. We could maybe
change $(LIB_hcrypto) to expand to libafshcrypto_lwp.a for Makefile.lwp,
but that can be confusing, especially for directories that build both
LWP and pthreaded binaries.
Written in collaboration with ben@huntsmans.net, who noticed the issue
and provided testing.
Change-Id: Ic4eef01c40e3ecdc4a8dc999b21273d7da364d34
Reviewed-on: https://gerrit.openafs.org/15904
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Ben Huntsman <ben@huntsmans.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
scan-build identified these as unused operations that can be removed.
Change-Id: I6fe7c3c23cfcbecbe8fa539131aec779c42876c2
Reviewed-on: https://gerrit.openafs.org/13305
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Several static analysis tools have identified various problems:
- files left open from early returns/exits (infer)
- possible memory leaks (scan-build, cppcheck)
To resolve the above problems:
- close files before returning/exiting
- fix possible memory leaks by freeing memory
This commit is a reorganization of commits developed by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Change-Id: Id8d25bacd2d1e1dee32354248519e3821ffe726c
Reviewed-on: https://gerrit.openafs.org/14681
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Several static analysis tools have identified various problems:
- possible memory leaks (scan-build, cppcheck)
To resolve the above problems:
- fix possible memory leaks by freeing memory
This commit is a reorganization of commits developed by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Change-Id: Ica9e5a33bf33ae802fed0f1439fc3561d0406713
Reviewed-on: https://gerrit.openafs.org/14685
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: PatRiehecky <jcpunk@gmail.com>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Several static analysis tools have identified various problems:
- use of uninitlized variables (scan-build)
- missing checks to ensure *alloc was successful (infer)
- possible memory leaks (scan-build, cppcheck)
To resolve the above problems:
- ensure variables are initialized to safe defaults
- add checks to ensure *alloc was successful before using the memory
- fix possible memory leaks by freeing memory
This commit is a reorganization of commits developed by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Change-Id: Ic5e222e344df4d5a3e317fcbb2f30a09dce93793
Reviewed-on: https://gerrit.openafs.org/14679
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Several static analysis tools have identified various problems:
- missing checks to ensure *alloc was successful (infer)
To resolve the above problems:
- Add asserts to verify that the *alloc was successful before using the
memory.
This commit is a reorganization of commits developed by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Note: Most of the callers to cmd_AddParmAlias and cmd_OptionAsList do
not check the returned value, so asserts are used instead of returning
ENOMEM.
Change-Id: Ic44f6d34240c13c2626567f9d698d2480d2e8672
Reviewed-on: https://gerrit.openafs.org/14680
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Several static analysis tools have identified various problems:
- printf conversion specifiers (cppcheck)
- sscanf string overflows (cppcheck)
- files left open from early exits (infer)
- dereference of NULL pointer (scan-build)
- missing checks to ensure *alloc was successful (infer)
- possible memory leaks (scan-build, cppcheck)
To resolve the above problems:
- update printf conversion specifiers
- update sscanf format strings to include the size of the buffer
- close files before exiting
- ensure pointers are non-NULL prior to use
- add checks to ensure *alloc was successful before using the memory
- fix possible memory leaks by freeing memory
This commit is a reorganization of commits developed by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Change-Id: I864b2c6be0c7dec1e93b49231ad7876d38c20435
Reviewed-on: https://gerrit.openafs.org/14677
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Clang's scan-build static analysis tool flagged a 'Use of memory after
it is freed'
The function _DirUpdate can reallocate the storage that is pointed to
by the dirbuffer member of the DirHeader structure.
The code in afscp_DirLookup initializes a local variable to d->dirbuffer
before calling _DirUpdate.
Update afscp_dir.c to initialize the local variable 'h' after calling
_DirUpdate.
Change-Id: Ibfc8b25ac31998f8581185f896737ca4bc27da7e
Reviewed-on: https://gerrit.openafs.org/14713
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
In the project root .gitignore, fix a typo which was causing the
built products directory to not be properly ignored.
Change-Id: Iea83a135b6f5a4c60c76df7c8bfcc65df90e1b39
Reviewed-on: https://gerrit.openafs.org/15903
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Several static analysis tools have identified various problems:
- missing checks to ensure *alloc was successful (infer)
To resolve the above problems:
- add checks to ensure *alloc was successful before using the memory
This commit is a reorganization of commits developed by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Change-Id: I48e053e73131c3f86928a4658dbc69e56abd2b20
Reviewed-on: https://gerrit.openafs.org/14683
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Currently, if the dynamic vcaches feature is enabled, the cache manager
deallocates batches of vcaches (allocated on demand) in the background.
Since this procedure is executed once every 5 minutes (at most), the
number of vcaches can grow unlimitedly between executions, increasing
the cost of using and maintaining them during this timeframe.
To prevent this from happening, run afs_ShakeLooseVCaches() sooner if we
have way more vcaches than the configured threshold.
Change-Id: I10ce7487441c659463ec85b4c8390274c11ef43f
Reviewed-on: https://gerrit.openafs.org/15044
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
On some platforms (such as LINUX), unlinked vcaches are not destroyed
until the next execution of afs_ShakeLooseVCaches(). Since the hash for
a given vcache is computed from its volume id and vnode number, keeping
unlinked vcaches around can be a problem if files are repeatedly deleted
and recreated in the same volume. In this scenario, the same vnode
number will be reused several times, resulting in many vcaches with the
same volume id and vnode number (but different unique ids) in the same
bucket. Consequently, finding a given vcache in the bucket in question
can be time consuming (and cause extra cpu processing), as we would have
to traverse a long linked list.
To mitigate this problem, move vcaches associated with unlinked files to
the least recently used position in our VLRU, prioritizing their removal
the next time afs_ShakeLooseVCaches() is executed.
Introduce the QAddEnd() macro to make it easier to add a vcache to the
end of the VLRU.
Change-Id: Idc74da0c3c0b27bfdf2cd491b003bdd42c889a95
Reviewed-on: https://gerrit.openafs.org/14961
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Linux-6.12 commit 'fs: Convert aops->write_begin to take a folio'
(1da86618bdce3) changed the address_space_operations's members
write_begin and write_end to use a folio instead of a page.
Add configure check to test the signature for aop's write_begin and
write_end members to see if they take a folio parameter.
Update the afs_linux_write_begin and afs_linux_write_end functions to
use a folio instead of a page.
Change-Id: Idd38aa3a9871e188580aae86f5b22621a5511bb2
Reviewed-on: https://gerrit.openafs.org/15898
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
The function afs_linux_write_begin() has 2 preprocessor selected
implementations, one to handle the case where write_begin has a flag
parameter and the other where it doesn't.
Refactor the code to combine the 2 implementations using preprocessor
conditionals for the function declaration and within the body of the
function as needed.
There are no functional changes.
This refactoring is in preparation for additional changes that will be
made to the afs_linux_write_begin() function.
Change-Id: I5389ea562d834d847bc7413e6f58dc3be2e5aa31
Reviewed-on: https://gerrit.openafs.org/15897
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
The Linux 6.12 commit 'mm: remove PG_error' (09022bc196d23) removed
the PG_error page flag and the associated ClearPageError() and
SetPageError() functions (via removing the PAGEFLAG(Error, ...) macro).
The PG_error flag has not been used by core VFS/MM Linux code for some
time, possibly ever, and so our calls to these functions do not have any
practical effect, since we also do not check for the PG_error flag.
While we could simply remove these calls, play it safe and keep them
around until ClearPageError()/SetPageError() are removed.
The specific semantics of the PG_error flag are not completely well
defined in the Linux kernel, which appears to be one of the motivations
for its removal. Some Linux commits that mention some details on how the
flag is not useful for read errors include:
7edf1ec5b249 ceph: don't SetPageError on readpage errors
41a638a1b3fc affs: convert affs_symlink_read_folio() to use the folio
2b2553f12355 btrfs: stop setting PageError in the data I/O path
Add a configure test to see if ClearPageError()/SetPageError() are
available in the Linux kernel; if they are not, define
ClearPageError()/SetPageError() as no-ops.
Change-Id: I2d3235d81030aa0b30907336f58a7d9c5c7b8026
Reviewed-on: https://gerrit.openafs.org/15876
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Tested-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Commit 1f5e1ef9e3 (OPENAFS-SA-2024-003: Run xdr_free for retried RPCs)
added a couple of references to xdr_namelist, which currently causes a
build failure on AIX:
/bin/sh ../../libtool --quiet --mode=link --tag=CC xlc_r [...] -o pts pts.o ../../src/ptserver/liboafs_prot.la [...]
ld: 0711-317 ERROR: Undefined symbol: xdr_namelist
ld: 0711-345 Use the -bloadmap or -bnoquiet option to obtain more information.
make: 1254-004 The error code from the last command is 8.
To avoid this, add xdr_namelist to liboafs_prot.la.sym.
Change-Id: I8c9a7a64efdb9107e27a3b3502e5d96272f5341e
Reviewed-on: https://gerrit.openafs.org/15954
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Update version strings for the third 1.9.x development release.
Change-Id: I203a9d3487d1344882afb02ded094e01e7672753
Reviewed-on: https://gerrit.openafs.org/15926
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10397
Currently, there are a few callers of RPCs that specify some data for
an INOUT parameter, but do not initialize the memory for that data.
This can result in the uninitialized memory being sent to the peer
when the argument is processed as an IN argument. Simply clear the
relevant data before running the RPC to avoid this.
The relevant RPCs and arguments are:
- For RMTSYS_Pioctl, the 'OutData' argument.
- For BUDB_GetVolumes, the 'volumes' argument.
-- via DBLookupByVolume -> bcdb_LookupVolume -> ubik_BUDB_GetVolumes
-- and via bc_Restorer -> bcdb_FindVolumes -> ubik_BUDB_GetVolumes
- For KAA_Authenticate_old / KAA_Authenticate, this can happen with
the 'answer' argument in ka_Authenticate if KAA_AuthenticateV2 or
KAA_Authenticate return RXGEN_OPCODE, but the server manages to
populate oanswer.SeqLen with non-zero.
For all of these, make sure the memory is blanked before running the
relevant RPC. For ka_Authenticate, reset oanswer.SeqLen to 0 to avoid
sending any data, but still blank 'answer' and 'answer_old' just to be
safe.
FIXES 135043
Change-Id: I95ee91a1710d66e2e88c3086d57c279a376f7dc6
Reviewed-on: https://gerrit.openafs.org/15925
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10397
Here, 'OutData' only has OutData.rmtbulk_len bytes in it. We know that
OutData.rmtbulk_len is at most data->out_size, but it could be
smaller. So, only copy OutData.rmtbulk_len bytes, not data->out_size,
since data->out_size could be more than the number of bytes we have
allocated in OutData.
FIXES 135043
Change-Id: I6f87fc8cb5df0298061f419112200f6c7e1974ba
Reviewed-on: https://gerrit.openafs.org/15924
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10397
A few areas of code retry the same RPC, like so:
do {
code = VL_SomeRPC(rxconn, &array_out);
} while (some_condition);
xdr_free((xdrproc_t) xdr_foo, &array_out);
Or try a different version/variant of an RPC (e.g.
VLDB_ListAttributesN2 -> VLDB_ListAttributes).
If the first RPC call causes the output array to be allocated with
length N, then the subsequent RPC calls may fail if the server
responds with an array larger than N.
Furthermore, if the subsequent call responds with an array smaller
than N, then when we xdr_free the array, our length will be smaller
than the actual number of allocated elements. That results in two
potential issues:
- We'll fail to free the elements at the end of the array. This is
only a problem if each element in the array also uses
dynamically-allocated memory (e.g. each element contains a string or
another array). Fortunately, there are only a few such structures in
any of our RPC-L definitions: SysNameList and CredInfos. And neither
of those are used in such a retry loop, so this isn't a problem.
- We'll give the wrong length to osi_free when freeing the array
itself. This only matters for KERNEL, and only on some platforms
(such as Solaris), since the length given to osi_free is ignored
everywhere else.
To avoid these possible issues, change the relevant retry loops to
free our xdr-allocated arrays on every iteration of the loop, like
this:
do {
xdr_free((xdrproc_t) xdr_foo, &array_out);
code = VL_SomeRPC(rxconn, &array_out);
} while (some_condition);
xdr_free((xdrproc_t) xdr_foo, &array_out);
Or like this:
do {
code = VL_SomeRPC(rxconn, &array_out);
xdr_free((xdrproc_t) xdr_foo, &array_out);
} while (some_condition);
FIXES 135043
Change-Id: I4e058eb52d277e6d3817df8b703c5e5b894c0b5a
Reviewed-on: https://gerrit.openafs.org/15923
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10397
Currently, if a caller calls an RPC with a string output argument,
like so:
{
char *str = NULL;
code = RXAFS_SomeCall(&str);
/* do something with 'str' */
xdr_free((xdrproc_t) xdr_string, &str);
}
Normally, xdr_free causes xdr_string to call osi_free, specifying the
same size that we allocated for the string. However, since we only
have a char*, the amount of space allocated for the string is not
recorded separately, and so xdr_string calculates the size of the
buffer to free by using strlen().
This works for well-formed strings, but if we fail to decode the
payload of the string, or if our peer gave us a string with a NUL byte
in the middle of it, then strlen() may be significantly less than the
actual allocated size. And so in this case, the size given to osi_free
will be wrong.
The size given to osi_free is ignored in userspace, and for KERNEL on
many platforms like Linux and DARWIN. However, it is notably not
ignored for KERNEL on Solaris and some other less supported platforms
(HPUX, Irix, NetBSD). At least on Solaris, an incorrect size given to
osi_free can cause a system panic or possibly memory corruption.
To avoid this, change xdr_string during XDR_DECODE to make sure that
strlen() of the string always reflects the allocated size. If we fail
to decode the string's payload, replace the payload with non-NUL bytes
(fill it with 'z', an arbitrary choice). And if we do successfully
decode the payload, check if the strlen() is wrong (that is, if the
payload contains NUL '\0' bytes), and fail if so, also filling the
payload with 'z'. This is only strictly needed in KERNEL on certain
platforms, but do it everywhere so our behavior is consistent.
FIXES 135043
Change-Id: I90c419a7ef0ede247187172a182863dcb4250578
Reviewed-on: https://gerrit.openafs.org/15922
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
