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>
CVE-2024-10397
Various RPCs return a variable-length array in an OUT argument, but
are only supposed to return specific sizes. A few instances of this
include the following (but this is not an exhaustive list):
- AFSVolListOneVolume should only return a single volintInfo.
- PR_NameToID should return the same number of IDs as names given.
- VL_GetAddrsU should return the same number of addresses as the
'nentries' OUT argument.
Some callers of these RPCs just assume that the server has not
violated these rules. If the server responds with a nonsensical array
size, this could cause us to read beyond the end of the array, or
cause a NULL dereference or other errors.
For example, some callers of VL_GetAddrsU will iterate over 'nentries'
addresses, even if the 'blkaddrs' OUT argument contains fewer entries.
Or with AFSVolListOneVolume, some callers assume that at least 1
volintInfo has been returned; if 0 have been returned, we can try to
access a NULL array.
To avoid all of this, add various sanity checks on the relevant
returned lengths of these RPCs. For most cases, if the lengths are not
sane, return an internal error from the appropriate subsystem (or
RXGEN_CC_UNMARSHAL if there isn't one). For VL_GetAddrsU, if
'nentries' is too long, just set it to the length of the returned
array.
FIXES 135043
Change-Id: Ibdc7837ab09b4765436fc4c0d780e695bba07128
Reviewed-on: https://gerrit.openafs.org/15921
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10397
When making an RPC call from a client, output arguments that use
arrays (or array-like objects like strings and opaques) can be
allocated by XDR, like so:
{
struct idlist ids;
ids.idlist_val = NULL;
ids.idlist_len = 0;
code = PR_NameToID(rxconn, names, &ids);
/* data inside ids.idlist_val[...] */
xdr_free((xdrproc_t) xdr_idlist, &ids);
}
With this approach, during XDR_DECODE, xdr_array() reads in the number
of array elements from the peer, then allocates enough memory to hold
that many elements, and then reads in the array elements.
Alternatively, the caller can provide preallocated memory, like so:
{
struct idlist ids;
afs_int32 ids_buf[30];
ids.idlist_val = ids_buf;
ids.idlist_len = 30;
code = PR_NameToID(rxconn, names, &ids);
/* data inside ids.idlist_val[...] */
}
With this approach, during XDR_DECODE, xdr_array() reads in the number
of array elements from the peer, and then reads in the array elements
into the supplied buffer. However, in this case, xdr_array() never
checks that the number of array elements will actually fit into the
supplied buffer; the _len field provided by the caller is just ignored.
In this example, if the ptserver responds with 50 elements for the 'ids'
output argument, xdr_array() will write 50 afs_int32's into
'ids.idlist_val', going beyond the end of the 30 elements that are
actually allocated.
It's also possible, and in fact very easy, to use xdr-allocated
buffers and then reuse them as a preallocated buffer, possibly
accidentally. For example:
{
struct idlist ids;
ids.idlist_val = NULL;
ids.idlist_len = 0;
while (some_condition) {
code = PR_NameToID(rxconn, names, &ids);
}
}
In this case, the first call to PR_NameToID can cause the buffer for
'ids' to be allocated by XDR, which will then be reused by the
subsequent calls to PR_NameToId. Note that this can happen even if the
first PR_NameToID call fails; the call can be aborted after the output
array is allocated.
Retrying an RPC in this way is effectively what all ubik_Call*
codepaths do (including all ubik_* wrappers, e.g. ubik_PR_NameToID).
Or some callers retry effectively the same RPC when falling back to
earlier versions (e.g. VL_ListAttributesN2 -> VL_ListAttributesN).
To prevent this for arrays and opaques, change xdr_array (and
xdr_bytes) to check if the _len field for preallocated buffers is
large enough, and return failure if it's not.
Also perform the same check for the ka_CBS and ka_BBS structures. These
are mostly the same as opaques, but they have custom serialization
functions in src/kauth/kaaux.c. ka_BBS also has two lengths: the actual
length of bytes, and a 'max' length. ka_CBS isn't used for any RPC
output arguments, but fix it for consistency.
For strings, the situation is complicated by the fact that callers
cannot pass in how much space was allocated for the string, since
callers only provide a char**. So for strings, just refuse to use a
preallocated buffer at all, and return failure if one is provided.
Note that for some callers using preallocated arrays or strings, the
described buffer overruns are not possible, since the preallocated
buffers are larger than the max length specified in the relevant
RPC-L. For example, afs_DoBulkStat() allocates AFSCBMAX entries for
the output args for RXAFS_InlineBulkStatus, which is the max length
specified in the RPC-L, so a buffer overrun is impossible. But since
it is so easy to allow a buffer overrun, enforce the length checks for
everyone.
FIXES 135043
Change-Id: Iaf913e2156af2081c72125ec066d9636086c7d14
Reviewed-on: https://gerrit.openafs.org/15920
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10397
Currently, a few RPCs with arrays or opaque OUT arguments are called
with preallocated memory for the arg, but also provide a _len of 0 (or
an uninitialized _len). This makes it impossible for the xdr routine to
tell whether we have allocated enough space to actually hold the
response from the server.
To help this situation, either specify an appropriate _len for the
preallocated value (cm_IoctlGetACL, fsprobe_LWP), or don't provide a
preallocated buffer at all and let xdr allocate a buffer for us
(PGetAcl).
Note that this commit doesn't change xdr to actually check the value of
the given _len; but now a future commit can do so without breaking
callers.
FIXES 135043
Change-Id: Ieb50aaa5ae9a1bde027999ce1c668e0c99b4d82b
Reviewed-on: https://gerrit.openafs.org/15919
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10397
Currently, several callers call RPCs with string OUT arguments, and
provide preallocated memory for those arguments. This can easily allow a
response from the server to overrun the allocated buffer, stomping over
stack or heap memory.
We could simply make our preallocated buffers larger than the maximum
size that the RPC allows, but relying on that is error prone, and
there's no way for XDR to check if a string buffer is large enough.
Instead, to make sure we don't overrun a given preallocated buffer,
avoid giving a preallocated buffer to such RPCs, and let XDR allocate
the memory for us.
Specifically, this commit changes several callers to
RXAFS_GetVolumeStatus(), and one caller of BOZO_GetInstanceParm(), to
avoid passing in a preallocated string buffer.
All other callers of RPCs with string OUT args already let XDR allocate
the buffers for them.
FIXES 135043
Change-Id: If42e2cc983903cff9766e1bab487142d4d493a17
Reviewed-on: https://gerrit.openafs.org/15918
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
Several places in the tree parse ACLs using sscanf() calls that look
similar to this:
sscanf(str, "%d dfs:%d %s", &nplus, &dfs, cell);
sscanf(str, "%100s %d", tname, &trights);
Some callers check whether the scanf() returns negative or 0, but some
callers do not check the return code at all. If only some of the fields
are present in the sscanf()'d string (because, for instance, the ACL is
malformed), some of the arguments are left alone, and may be set to
garbage if the relevant variable was never initialized.
If the parsed ACL is copied to another ACL, this can result in the
copied ACL containing uninitialized memory.
To avoid this, make sure all of the variables passed to sscanf() and
similar calls are initialized before parsing. This commit does not
guarantee that the results make sense, but at least the results do not
contain uninitialized memory.
Change-Id: Ib0becffbce5a6e15f91ac4390bb9c422d478bea6
Reviewed-on: https://gerrit.openafs.org/15917
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
After the preceding commits, the data returned by the VIOCGETAL
pioctl (a RXAFS_FetchAcl wrapper) will safely be NUL-terminated.
However, the callers that attempt to parse the ACL string make
assumptions that the returned data will be properly formatted,
and implement a "skip to next line" functionality (under various
names) that blindly increments a char* until it finds a newline
character, which can read past the end of even a properly
NUL-terminated string if there is not a newline where one is
expected.
Adjust the various "skip to next line" functionality to keep
the current string pointer at the trailing NUL if the end of the
string is reached while searching for a newline.
Change-Id: I7fb7f23d7d6f68608f3e656a1530a7fc40b4a567
Reviewed-on: https://gerrit.openafs.org/15916
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
Supplement the previous commit by additionally verifying that
the returned ACL string occupies the entire XDR opaque, rejecting
any values returned that have an internal NUL prior to the end
of the opaque.
Change-Id: Iefa3d00a9a0e25ef66b7166fe952aae0603ee3d7
Reviewed-on: https://gerrit.openafs.org/15915
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
Analogously to how a call to RXAFS_StoreACL() with a malformed
ACL string can cause a fileserver to perform invalid memory operations,
a malformed ACL string returned in response to a call to RXAFS_FetchACL()
can cause a client to perform invalid memory operations.
Modify all the in-tree callers of the RPC to verify that the ACL
data, which is conveyed as an XDR 'opaque' but whose contents
are actually expected to be a string, is a valid C string. If
a zero-length opaque or one without a trailing NUL is received,
treat that as an error response from the fileserver rather than
returning success.
The Unix cache manager's pioctl handler already has logic to cope with a
zero-length reply by emitting a single NUL byte to userspace. This
special-casing seems to have been in place from the original IBM import,
though it does so by confusingly "skipping over" a NUL byte already put
in place. For historical compatibility, preserve that behavior rather
than treating the zero-length reply as an error as we do for the other
callers. It seems likely that this location should treat a zero-length
reply as an error just as the other call sites do, but that can be done
as a later change.
Change-Id: Ibf685e54e7e3fca6a4caac63c961cfcfb2f4732a
Reviewed-on: https://gerrit.openafs.org/15914
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
Currently in SRXAFS_StoreACL, if CallPreamble() or check_acl() fail, we
will jump to Bad_StoreACL, which will pass the ACL string from the
client to osi_auditU. Since check_acl() hasn't yet checked if the given
ACL contains a NUL byte, the ACL may be an unterminated string. If
auditing is enabled, this can cause garbage to be logged to the audit
log, or cause the fileserver to crash.
To avoid this, set 'rawACL' to NULL at first, only setting it to the
actual ACL string after check_acl() has succeeded. This ensures that all
code accessing 'rawACL' is guaranteed to be using a terminated string.
This may mean that we pass a NULL AUD_ACL to osi_auditU. Our auditing
code explicitly checks for and handles handles NULL strings, so this is
fine.
FIXES 135445
Change-Id: Iecde5677805a28d55c833b135732a14fd86cc985
Reviewed-on: https://gerrit.openafs.org/15913
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
Change our StoreACL implementation to refer to the 'AccessList' argument
via a new local variable called 'rawACL'. This makes it clearer to
users that the data is a string, and makes it easier for future commits
to make sure we don't access the 'AccessList' argument in certain
situations.
Update almost all users in StoreACL to refer to 'rawACL' instead of
'AccessList'. Change the name of 'AccessList' to 'uncheckedACL' to make
sure we don't miss any users. Update our check_acl() call to use
'uncheckedACL' (and not 'rawACL'), because it must use an AFSOpaque to
check the ACL.
Change RXStore_AccessList() and printableACL() to accept a plain char*
instead of a struct AFSOpaque.
This commit should not incur any noticeable behavior change. Technically
printableACL() is changed to run strlen() on the given string, but this
should not cause any noticeable change in behavior:
This change could cause printableACL() to process less of the string
than before, if the string contains a NUL byte before the end of the
AFSOpaque buffer. But this doesn't matter, since the all of our code
after this treats the ACL as a plain string, and so doesn't look at any
data beyond the first NUL. It's not possible for printableACL() to
process more data than before, because check_acl() has already checked
that the ACL string contains a NUL byte, so we must process
AFSOpaque_len bytes or fewer.
FIXES 135445
Change-Id: I26229a39528fec788d96c77012c042c278a214c9
Reviewed-on: https://gerrit.openafs.org/15912
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
In acl_Internalize_pr(), each line in an ACL granting rights (positive
or negative) is sscanf()'d with "%63s\t%d\n", and then we try to
advance 'nextc' beyond the next newline character.
However, sscanf()'ing "%63s\t%d\n" does not guarantee that there is a
newline in the given string. Whitespace characters in sscanf() are not
matched exactly, and may match any amount of whitespace (including
none at all). For example, a string like "foo 4" may be parsed by
sscanf(), but does not contain any newlines.
If this happens, strchr(nextc, '\n') will return NULL, and we'll
advance 'nextc' to 0x1, causing a segfault when we next try to
dereference 'nextc'.
To avoid this, check if 'nextc' is NULL after the strchr() call, and
return an error if so.
FIXES 135445
Change-Id: I6bcbbaf88a16202fb84c0932578dd8d5712726dd
Reviewed-on: https://gerrit.openafs.org/15911
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
The early parsing code in acl_Internalize_pr() tries to advance
'nextc' to go beyond the first two newlines in the given ACL string.
But if the given ACL string has no newlines, or only 1 newline, then
'nextc' will point beyond the end of the ACL string, potentially
pointing to garbage.
Intuitively, it may look like the ACL string must contain at least 2
newlines because we have sscanf()'d the string with "%d\n%\d".
However, whitespace characters in sscanf() are not matched exactly
like non-whitespace characters are; a sequence of whitespace
characters matches any amount of whitespace (including none). So, a
string like "1 2" will be parsed by "%d\n%d\n", but will not contain
any newline characters.
Usually this should result in a parse error from acl_Internalize_pr(),
but if the garbage happens to parse successfully, this could result in
unrelated memory getting stored to the ACL.
To fix this, don't advance 'nextc' if we're already at the end of the
ACL string.
FIXES 135445
Change-Id: Ie009b59bec9a75afc81fee201c2fca6955f484e4
Reviewed-on: https://gerrit.openafs.org/15910
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10394
Currently, we only throttle PAG creation in afs_setpag(). But there
are several callers that call setpag() directly, not via afs_setpag;
notably _settok_setParentPag in afs_pioctl.c. When setpag() is called
with a PAG value of -1, it generates a new PAG internally without any
throttling. So, those callers effectively bypass the PAG throttling
mechanism, which allows a calling user to create PAGs without any
delay.
To avoid this, move our afs_pag_wait call from afs_setpag() to
afs_genpag(), which all code uses to generate a new PAG value. This
ensures that PAG creation is always throttled for unprivileged users.
FIXES 135062
Change-Id: Ic4cb352edaa693984995fbdb6dc35b89686e8470
Reviewed-on: https://gerrit.openafs.org/15907
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10394
Currently, several areas in the code call genpag() to generate a new
PAG id, but the signature of genpag() is very limited. To allow for
the code in genpag() to return errors and to examine the calling
user's credentials, introduce a new function, afs_genpag(), that does
the same thing as genpag(), but accepts creds and allows errors to be
returned.
Convert all existing callers to use afs_genpag() and to handle any
errors, though no errors are ever returned in this commit on its own.
To ensure there are no old callers of genpag() left around, change the
existing genpag() to be called genpagval(), and declare it static.
FIXES 135062
Change-Id: I5c96c0134db901f21ca30c8b3f57aeec1eb67aa5
Reviewed-on: https://gerrit.openafs.org/14090
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
Currently, we don't free 'newACL' if acl_Internalize_pr() fails. If
acl_Internalize_pr() has already allocated 'newACL', then the memory
associated with newACL will be leaked. This can happen if parsing the
given ACL fails at any point after successfully parsing the first
couple of lines in the ACL.
Change acl_FreeACL() to make freeing a NULL acl a no-op, to make it
easier to make sure the acl has been freed.
FIXES 135445
Change-Id: I87745fa9b6285574acdd5ecb613e80fa1ea37ae8
Reviewed-on: https://gerrit.openafs.org/15909
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
CVE-2024-10396
Currently, the fileserver treats the ACL given in RXAFS_StoreACL as a
string, even though it is technically an AFSOpaque and could be not
NUL-terminated.
We give the ACL opaque/string to acl_Internalize_pr() to parse, which
will run off the end of the allocated buffer if the given ACL does not
contain a '\0' character. Usually this will result in a parse error
since we'll encounter garbage, but if the partially-garbage ACL
happens to parse successfully, some uninitialized data could make it
into the stored ACL.
In addition, if the given ACL is an opaque of length 0, we'll still
give the opaque pointer to acl_Internalize_pr(). In this case, the
pointer will point to &memZero, which happens to contain a NUL byte,
and so is treated like an empty string (which is not a valid ACL). But
the fact that this causes no problems is somewhat a coincidence, and
so should also be avoided.
To avoid both of these situations, just check if the given ACL string
contains a NUL byte. If it doesn't, or if it has length 0, refuse to
look at it and abort the call with EINVAL.
FIXES 135445
Change-Id: If55f72d6556bc7b1704a3848865bfb902ee9f92a
Reviewed-on: https://gerrit.openafs.org/15908
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
Tested-by: Benjamin Kaduk <kaduk@mit.edu>
Commit de46edef69 (Remove lowercase min/max macro definitions)
inadvertently removed a definition of max() that is required by
_krb5_n_fold() (src/external/heimdal/krb5/n-fold.c) if no system
definition of max() is available. For example, on Solaris this results
in a kernel module with an undefined external symbol 'max', which
prevents the kernel module from loading:
genunix: [ID 819705 kern.notice] /kernel/drv/amd64/afs: undefined symbol
genunix: [ID 826211 kern.notice] 'max'
genunix: [ID 472681 kern.notice] WARNING: mod_load: cannot load module 'afs'
Restore the required max() macro definition.
Change-Id: I6bca2bb2b90d7cdbe90a3b769997cdc153f59f6c
Reviewed-on: https://gerrit.openafs.org/15874
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
During parallel builds, we can run the final 'libtool --mode=link'
step for, say, pam_afs.la and pam_afs.krb.la in parallel. When libtool
runs --mode=link for these libraries, it effectively does the
following (amongst many other steps):
- rm -rf .libs/"$libname".*
- mkdir .libs/"$libname".lax
- mkdir .libs/"$libname".lax/libopr_pic.a
The 'rm -rf' cleans up any lingering files from previous runs in the
.libs dir (e.g. .libs/pam_afs.a, .libs/pam_afs.lai, etc etc). The
'.lax' directory is used as a temporary space to extract object files
in dependant libs. And of course it creates various other files in
there named like .libs/pam_afs.lai.
But if we're building a library called pam_afs.krb.la, then some of
our temporary files will be named like .libs/pam_afs.krb.lax, which
get deleted when building pam_afs.la when it deletes
".libs/pam_afs.*". So, if we build pam_afs.la while the libtool
command for pam_afs.krb.la is using .libs/pam_afs.krb.lax, the
temporary files can get deleted out from under us, causing rare
confusing libtool errors. For example:
/bin/sh ../../libtool --quiet --mode=link --tag=CC gcc -rpath /usr/local/lib \
-module -no-undefined -o pam_afs.la [...]
/bin/sh ../../libtool --quiet --mode=link --tag=CC gcc -rpath /usr/local/lib \
-module -no-undefined -o pam_afs.krb.la [...]
../../libtool: line 1719: cd: .libs/pam_afs.krb.lax/libopr_pic.a: No such file or directory
make[3]: *** [Makefile:76: pam_afs.krb.la] Error 1
make[3]: *** Waiting for unfinished jobs....
Fortunately, most of the libraries we build don't have dots in their
name, and so avoid this issue. But we do have a few libraries that
build a ".krb" variant, and so can be impacted by this. This includes
pam_afs.krb, libauth.krb, and libkauth.krb.
To make sure these libraries don't encounter this problem, make the
.krb variant depend on the non-.krb variant, so the libraries aren't
built in parallel.
Change-Id: Icb96a721b481bc3d99c9e24cf81fcfbbf7d498f6
Reviewed-on: https://gerrit.openafs.org/15328
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Since the original IBM code import, the 'bos' utility source code has
contained an unused pre-processor conditional 'OPBOS'. If OPBOS is
defined, it builds a bos binary that implements only the following bos
commands: status, start, stop, restart, startup, shutdown.
There is no configure option or any other logic that defines 'OPBOS',
nor is it documented. Remove the unused conditional.
Change-Id: I4d32084e6a6b4ca8cadb0aa0ef06c74fb6edb770
Reviewed-on: https://gerrit.openafs.org/15875
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Modify OpenAFS to respect user-specified ARCHFLAGS (-m32 or -m64) when
building on Solaris 10 and 11 SPARC platforms. If ARCHFLAGS is
unset/unspecified, we will continue to build 32-bit userspace by
default.
Implement -m64 SPARC LWP via USE_UCONTEXT; leave the -m32 SPARC LWP
(default) implementation unchanged.
Kernelspace builds remain 64-bit, regardless of ARCHFLAGS value.
Change-Id: Ibb8e145d0cb8dfbe05de896b84c1d1135664b845
Reviewed-on: https://gerrit.openafs.org/14897
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Currently, the ReclaimedVCList is only fed on Darwin. As a result, the
logic present in afs_FlushReclaimedVcaches() is never used on all other
platforms and should be wrapped in #ifdef AFS_DARWIN_ENV. While here,
also reorganize this function in an effort to make it more readable.
No functional change is incurred by this commit.
Change-Id: I1097a2b07fcb48dec2b31385477c34e974144b51
Reviewed-on: https://gerrit.openafs.org/15027
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Because the afs_vhashT hash table uses singly-linked lists in its hash
buckets, to remove an entry we have to:
1. Find the bucket where the entry to be removed should be present.
2. Go through each element of this bucket until we find this entry.
3. Remove it.
Due to step number 2, the time required to remove an entry from a bucket
increases as the linked list associated with this bucket gets longer.
This can get particularly bad when evicting vcaches (ShakeLooseVCaches),
as each entry from the least recently used queue (VLRU) has to be found
in our afs_vhashT hash table. This problem is exacerbated when files are
repeatedly deleted and recreated in the same volume, resulting in many
vcaches with the same volume id and vnode number (but different unique
ids) in the same bucket.
To avoid this problem, build afs_vhashT using doubly-linked lists with
'struct afs_q', like we did for afs_vhashTV in commit 4fc48af8
(vc-hashing-be-less-expensive-20050728). Now, removing a vcache from
afs_vhashT is an O(1) operation (QRemove).
Change-Id: I5a1a9a090f9aa3d8884e2bb12aca1f1acc3b902d
Reviewed-on: https://gerrit.openafs.org/14949
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Replace strcpy, strcat, and sprintf with safer string functions in the
tests/torture/DumpAfsLog program. Add checks for string truncation
where needed.
Change-Id: I4eac12e73bbeaeadc0a8a6b5ebded664ca91b7f8
Reviewed-on: https://gerrit.openafs.org/15489
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Gaurav Saxena <gsaxena@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Michael Meffie <mmeffie@sinenomine.net>
Remove the commented out code in the DumpAfsLog test program.
Change-Id: Ie7b380ec37d18cfc7b1be8ce9904b55ad101b263
Reviewed-on: https://gerrit.openafs.org/15871
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>
Several static analysis tools have identified various problems:
- files left open from early returns/exits (infer)
- missing checks to ensure *alloc was successful (infer)
- memory leaks due to not freeing storage before
a return (infer)
- unused variables (cppcheck)
To resolve the above problems:
- close files before returning/exiting
- add checks to ensure *alloc was successful before using the memory
- free storage before returning
- remove unused variables
This commit is a reorganization of commits submitted by Pat Riehecky,
who ran the static analysis tools and developed the fixes.
Change-Id: If45d64c370ad68d1e844b6aa5881f9dbd75300c9
Reviewed-on: https://gerrit.openafs.org/14671
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
There are several large #if/#endif blocks that do not have a comment on
the #endif, and some of the nested #if/#else/#endif blocks are not
indented.
In addition, there are violations of the CODING style such as
indentation errors, trailing whitespace, braces for functions that are
not on their own line, etc.
Add comments to the #endif statements for the larger blocks.
Use indent with parameters from CODING to fix the remaining style
issues, and fix up the preprocessor indentation.
There are no functional changes
Change-Id: I89a13a1f43aa08728777a71a41984613ac163b95
Reviewed-on: https://gerrit.openafs.org/15669
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-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>
Commit de46edef69 (Remove lowercase min/max macro definitions) removed
the min() macro definition from the util/ktime.h header. Unfortunately,
that min() definition is still required for some of the Windows source
files.
Later, the windows sources can be converted to use the new in-tree
opr_min and opr_max macros, but for now reinstate the min() macro
definition in ktime.h.
Fixes the build errors on Windows:
msrpc.obj : error LNK2001: unresolved external symbol _min
RDRFunction.obj : error LNK2001: unresolved external symbol _min
cm_server.obj : error LNK2019: unresolved external symbol _min referenced
in function _cm_SetServerIPRank
cm_dcache.obj : error LNK2001: unresolved external symbol _min
cm_vnodeops.obj : error LNK2001: unresolved external symbol _min
smb3.obj : error LNK2001: unresolved external symbol _min
Change-Id: I043e23e72977ef06330b53f05044cd3be93cc00e
Reviewed-on: https://gerrit.openafs.org/15870
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Commit 8e1e242ce4 (Replace lowercase min/max macros with opr_min/max)
converted the min() invocation in auth/ktc_nt.c with our in-tree
opr_min() macro, but missed adding the opr.h header to that source file.
Add the opr.h include to fix the build error on windows:
afsauth.lib(ktc_nt.obj) : error LNK2019: unresolved external symbol
_opr_min referenced in function _GetLocalToken
Change-Id: I96dbe377a8a85921fa1833cd27638cd853e26b9c
Reviewed-on: https://gerrit.openafs.org/15869
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: Michael Meffie <mmeffie@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
The commit:
"afsd: Move to newer cmd syntax" (fbe6f0f4ad)
removed setting the flag variable "sawBiods" when the -nbiods option is
used, this results in the -nbiods value being ignored and the default
value being used.
This problem was identified by the cppcheck static analysis tool.
Note: The -nbiods option is only for AIX systems.
Change-Id: I9a8d199a212a164660c1e9f8b4151792d9c9d0e0
Reviewed-on: https://gerrit.openafs.org/15212
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Since introduced in commit 53986479ee (ptutil-initial-20001219),
pt_util has used a statically allocated buffer to read pages from the
database. In several places the buffer is cast to a struct ubik_header.
Memory allocated dynamically via malloc()/calloc() is guaranteed to be
properly aligned for objects of any type at run time, however buffers
that are not allocated dynamically have no such guarantee. So, instead
of using a statically allocated buffer, dynamically allocate it with
calloc() to ensure it is aligned for struct ubik_header objects.
This fixes a SIGBUS error whenever the buffer happens to be built with a
non-word alignment. This failure was observed while running the
tests/ptserver/pt_util-t unit test on Solaris SPARC 64-bit.
Note: By inspection, the same potential problem exists in
src/kauth/ka_util.c. However, kaserver is deprecated and ka_util has
never been built by default.
Change-Id: I00372ed6adede32448b5e0df1c1d74689286a218
Reviewed-on: https://gerrit.openafs.org/15352
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>
rxkad/fcrypt defines 2 macros: ENCRYPT and DECRYPT. The names are
pretty generic and do not clearly associate with fcrypt.
In addition, the name ENCRYPT collide with the Microsoft provided header
'ntddndis.h' that uses ENCRYPT as an ENUM (on newer versions of the
Windows development kits):
typedef enum _OFFLOAD_OPERATION_E
{
AUTHENTICATE = 1,
ENCRYPT
}
This results in a build error on Windows when fcrypt.h is included with
ntddndis.h:
... ntddndis.h(2212): error C2059: syntax error: 'constant'
Rename the ENCRYPT/DECRYPT macros to FCRYPT_ENCRYPT/FCRYPT_DECRYPT in
fcrypt.h to a name that relates the macros back to fcrypt.
Note: The ENCRYPT/DECRYPT symbols are part of a public interface
installed in fcrypt.h, but keeping the old names are impractical, so we
are changing them anyway.
Change-Id: I9c51c81fab7fcbf0dae01569852ca94c0e6a0439
Reviewed-on: https://gerrit.openafs.org/15868
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Benjamin Kaduk <kaduk@mit.edu>
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: I9396b8327f8283c60821050c75de1c36e0ebd12e
Reviewed-on: https://gerrit.openafs.org/14684
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
CheckVolume accepts an optional nvldbentry as an argument. When this
argument is NULL, CheckVolume correctly locks the nvldbentry before
obtaining the entry and making changes. However, if an nvldbentry is
passed, CheckVolume merely copies it to a work entry; it also neglects
to obtain the current state of the entry after locking it for
modification.
Change CheckVolume to obtain a locked copy of the nvldbentry if
modifications are required.
Change-Id: Ie98bc57af1cc2955d10982f44abd4c55755c6244
Reviewed-on: https://gerrit.openafs.org/14356
Tested-by: BuildBot <buildbot@rampaginggeek.com>
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>
In preparation for a future commit, refactor some large chunks of logic
out of CheckVolume and into new static routines to handle the specific
cases for RW, RO, and BK volumes. Also provide Doxygen documentation for
CheckVolume.
No functional change should be incurred by this commit.
Change-Id: Iac6a773d29ed530e9676f02726db031b1f2a2526
Reviewed-on: https://gerrit.openafs.org/14355
Tested-by: BuildBot <buildbot@rampaginggeek.com>
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>
Since the original IBM code import, UV_RestoreVolume (later renamed to
to the current name UV_RestoreVolume2) has called VLDB_GetEntryByID to
obtain the VLDB entry of the target/destination volume. This entry is
then modified to match the restore changes, locked via VL_SetLock, and
replaced via VL_ReplaceEntryN.
Unfortunately, this allows for a race with other volume operations
against the same volume, if another operation alters the entry after we
call VLDB_GetEntryByID but before ubik_VL_SetLock. To avoid this, call
GetLockedEntry to ensure that the entry is locked while we are reading
and modifying it.
Change-Id: I772ef80158205f45c04f8ef0aa95726f1f2c4ac4
Reviewed-on: https://gerrit.openafs.org/14354
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>
Since the original IBM code import, UV_RestoreVolume2 (originally
UV_RestoreVolume) has checked for VL_ENTDELETED from VLDB_GetEntryByID.
However, if VL_ENTDELETED is received, we will act like we retrieved the
vlentry successfully. If the -verbose flag is given, we'll try to print
the entry (even though one was not returned), and we'll call VL_SetLock
for the volume, which will almost certainly also fail with VL_ENTDELETED
and bail out.
Furthermore (also since the original IBM import), there is no longer any
code path to set the volume entry flag VLDELETED. So it should be rare
for any VL_* RPC to return VL_ENTDELETED; that would require an entry to
have the VLDELETED flag carried over from a very old database.
Therefore, it is safe and reasonable to simply remove the check for
VL_ENTDELETED from UV_RestoreVolume2. If we do somehow receive the
VL_ENTDELETED error code, vos will terminate with an error instead of
possibly printing an uninitialized entry.
In addition, remove the same check for VL_ENTDELETED in the equivalent
code path in libadmin's UV_RestoreVolume.
Change-Id: I11d1c3306f67d68de54780f6aac75e4c27779db4
Reviewed-on: https://gerrit.openafs.org/14357
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: Andrew Deason <adeason@sinenomine.net>
In several routines in vsprocs.c, we declare and clear nvldbentry
storeEntry long before it is actually needed.
Instead, where possible, move the declaration and memset of storeEntry
closer to the actual point of use.
No functional change is incurred by this commit.
Change-Id: Iffbf5ff56fab4d221586dfb93bbbbfde063c856e
Reviewed-on: https://gerrit.openafs.org/14353
Reviewed-by: Cheyenne Wills <cwills@sinenomine.net>
Reviewed-by: Marcio Brito Barbosa <mbarbosa@sinenomine.net>
Reviewed-by: Andrew Deason <adeason@sinenomine.net>
Tested-by: BuildBot <buildbot@rampaginggeek.com>
Reviewed-by: Michael Meffie <mmeffie@sinenomine.net>
Reviewed-by: Mark Vitale <mvitale@sinenomine.net>
