5 years agovolser: detect eof in dump stream while reading acl 03/11703/7
Michael Meffie [Fri, 30 Jan 2015 17:20:10 +0000]
volser: detect eof in dump stream while reading acl

Detect an EOF condition while reading the ACL in a dump stream
and return a restore error, instead of filling the ACL with
0xFF and then failing the restore due to an invalid tag.

Change-Id: If7a71946c81f47ac68ed2f7afdfca1023bad3baf
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agocellconfig: check for invalid dotted quads 09/12109/4
Benjamin Kaduk [Sun, 22 Nov 2015 20:23:49 +0000]
cellconfig: check for invalid dotted quads

IP addresses entered into the CellServDB with components larger
than 255 would silently be trucated down to 8-bit unsigned integer
representations.  This could cause confusing behavior with
occasional hangs.

FIXES 131794

Change-Id: I44834cb4662e178fdb4be2eeb03ad58d2fa7c556
Reviewed-by: Chas Williams <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoafs: Log abnormally large chunk files 31/11831/5
Andrew Deason [Sun, 12 Apr 2015 01:51:09 +0000]
afs: Log abnormally large chunk files

Any chunk in our cache for a regular file should be smaller than or
equal to our configured chunksize. If someone sets a chunk to be
larger than that, it is very strange and may cause other confusing
issues. Specifically, afs_DoPartialWrite determines if our cache is
"too full" by counting the number of dirty chunks. If we have a dirty
chunk that is much larger than the chunksize, it can throw off the
afs_DoPartialWrite calculation.

This is only true for dcaches backing regular files, though. For
directories, we fetch the entire directory into a single chunk file,
and the size of a directory blob can easily exceed the chunksize
without issues. The aforementioned issue with afs_DoPartialWrite does
not apply, since directory chunks cannot be dirty (we only locally
modify the chunk if we modify the dir on the server, and the DVs

Anyway, it should not be possible to get a chunk for a regular file
larger than the chunksize. Log a message if it does occur, to help
assist anyone in tracking down issues when this does occur.

[ remove unnecessary casts in afs_warn args.]

Change-Id: I5cf58e3659dc04255c62fa56b044d5bc1c7ce877
Reviewed-by: Michael Meffie <>
Reviewed-by: Chas Williams <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoopr: Use opr_Assert() instead of silently failing 53/11853/4
Chas Williams [Sat, 25 Apr 2015 20:53:43 +0000]
opr: Use opr_Assert() instead of silently failing

These routines should never be passed a NULL.  If this happens it
is a serious issue that needs to be addressed.

Change-Id: I9728dcd67bc9f8e9927bed1674fc0ee83567df1a
Tested-by: BuildBot <>
Tested-by: Benjamin Kaduk <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

5 years agoopr: Disable some warnings during opr assertions 52/11852/5
Chas Williams [Sat, 25 Apr 2015 20:38:12 +0000]
opr: Disable some warnings during opr assertions

Detect _Pragma(), a C99 extension for inline #pragma's, and use it to
disable to certain warnings during the use of opr_Verify() and

Because some versions of clang support _Pragma, do not have support
for -Wtautological-pointer-compare, and do set -Werror and -Wunknown-pragmas,
we must explicitly check for pragma support for -Wtautological-pointer-compare
as well.

Change-Id: Id3d5ee347f320a366a0571572b58414aa7044bf7
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoafs: Log weird 'size' fetchdata errors 30/11830/5
Andrew Deason [Fri, 10 Apr 2015 02:26:25 +0000]
afs: Log weird 'size' fetchdata errors

There are a couple of situations that should never happen when issuing
a fetchdata, but cause errors when they do:

 - The fileserver responds with more than 2^32 bytes of data
 - The fileserver responds with more data than requested (but still
   smaller than 2^32)

While these should normally never be encountered, it can be very
confusing when they do, since they cause file fetches to fail. To give
the user or investigating developer some hope of figuring out what is
going on, at least log a warning in these situations, to at least
indicate this is the area in which something is breaking.

Only log these once, in case something causes these conditions to be
hit, e.g., every fetch. Once is at least enough to say this is

[ remove unneeded casts in afs_warn args and
explicit static initializers.]

Change-Id: I7561a9ecc225386f9b140e633912b900c591a9bb
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoafs: Fix fetchInit for negative/large lengths 29/11829/4
Andrew Deason [Wed, 8 Apr 2015 03:10:53 +0000]
afs: Fix fetchInit for negative/large lengths

Currently, the 'length64' variable in rxfs_fetchInit is almost
completely unused (it just goes into an icl logging function). For the
length that we actually use ('*alength'), we just take the lower 32
bits of the length that the fileserver told us. This method is
incorrect in at least the following cases:

 - If the fileserver returns a length that is larger than 2^32-1,
   we'll just take the lower 32 bits of the 64-bit length the
   fileserver told us about. The client currently never requests a
   fetch larger than 2^32-1, so this would be an error, but if this
   occurred, we would not detect it until much later in the fetch.

 - If the fileserver returns a length that is larger than 2^31-1, but
   smaller than 2^32, we'll interpret the length as negative (which we
   assume is just 0, due to bugs in older fileservers). This is also

 - If the fileserver returns a negative length smaller than -2^31+1,
   we may interpret the give length as a positive value instead of a
   negative one. Older fileservers can do this if we fetch data beyond
   the file's EOF (this was fixed in the fileserver in commit
   529d487d65d8561f5d0a43a4dc71f72b86efd975). This positive length
   will cause an error (usually), instead of proceeding without error
   (which is what would happen if we correctly interpreted the length
   as negative).

On Solaris, this can manifest as a failed write, when writing to a
location far beyond the file's EOF from the fileserver's point of
view, because Solaris writes can trigger a fetch for the same area.
Seeking to a location far beyond the file's EOF and writing can
trigger this, as can a normal copy into AFS, if the file is large
enough and the cache is large enough. To explain in more detail:

When copying a file into AFS, the cache manager will buffer the dirty
data in the disk cache until the file is synced/closed, or we run out
of cache space. While this data is buffering, the application will
write into an offset, say, 3GiB into the file. On Solaris, this can
trigger a read for the same region, which will trigger a fetch from
the fileserver at the offset 3GiB into the file. If the fileserver
does not contain the fix in commit
529d487d65d8561f5d0a43a4dc71f72b86efd975, it will respond with a large
negative number, which we interpret as a large positive number; much
larger than the requested length. This will cause the fetch to fail,
which then causes the whole write() call to fail. Specifically this
will fail with EINVAL on Solaris, since that is the error code we
return from afs_GetOnePage when we fail to acquire a dcache. If the
cache is small enough, this will not happen, since we will flush data
to the fileserver before we have a large amount of dirty data,
e.g., 3GiB. (The actual error occurs closer to 2GiB, but this is just
for illustrative purposes.)

To fix this, detect the various ranges of values mentioned above, and
handle them specially. Lengths that are too large will yield an error,
since we cannot handle values over 2^31-1 in the rxfs_* framework

For lengths that are negative, just act as if we received a length of
0. Do this for both the 64-bit codepath and the non-64-bit codepath,
just so they remain identical.

[ directly use 64 bit comparisons, don't mask
end call error code, commit nits.]

Change-Id: I7e8f2132d52747b7f0ce4a6a5ba81f6641a298a8
Reviewed-by: Chas Williams <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoafs: Avoid incorrect size when fetching beyond EOF 28/11828/3
Andrew Deason [Fri, 10 Apr 2015 00:58:51 +0000]
afs: Avoid incorrect size when fetching beyond EOF

Currently, afs_GetDCache contains a couple of calculations that look
similar to this:

    if (position + size > file_length) {
        size = file_length - position;
    if (size < 0) {
        size = 0;

Most of the time, this is fine. However, if 'position' is more than
2GiB greater than file_length, 'size' will calculated to be smaller
than -2GiB. Since 'size' in this code is a signed 32-bit integer, this
can cause 'size' to underflow, and result in a value closer to
(positive) 2GiB.

This has two potential effects:

The afs_AdjustSize call in afs_GetDCache will cause the underlying
cache file for this dcache to be very large (if our offset is around
2GiB larger than the file size). This can confuse other parts of the
client, since our cache usage reporting will be incorrect (and can be
even way larger than the max configured cache size).

This will also cause a read request to the fileserver that is larger
than necessary. Although 'size' will be capped at our chunksize, it
should be 0 in this situation, since we know there is no data to
fetch. At worst, this currently can just result in worse performance
in rare situations, but it can also just be very confusing.

Note that an afs_GetDCache request beyond EOF can currently happen in
non-race conditions on at least Solaris when performing a file write.
For example, with a chunksize of 256KiB, something like this will
trigger the overflow in 'size' in most cases:

    $ printf '' > smallfile && printf b | dd of=smallfile bs=1 oseek=2147745793

But there are probably other similar scenarios.

To fix this, just check if our offset is beyond the relevant file
size, and do not depend on 'size' having sane values in edge cases
such as this.

Change-Id: Ie36f66ce11fbee905062b3a787871ec077c15354
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Michael Meffie <>
Tested-by: BuildBot <>
Reviewed-by: Chas Williams <>

5 years agodoc: afsd -settime and -nosettime are obsolete 75/12175/3
Michael Meffie [Thu, 21 Jan 2016 22:55:37 +0000]
doc: afsd -settime and -nosettime are obsolete

Update the afsd man page -settime and -nosettime options, which are obsolete
and no longer have any effect.  Use the same wording as the other obsolete
options in the afsd man page.  Keep the recommendations to use the time keeping
daemons provided by the operating system to maintain the system time.

Change-Id: I08a1bd5ae0b2d6618b3e212ebcbb98f470e33820
Reviewed-by: Michael Laß <>
Tested-by: BuildBot <>
Reviewed-by: Stephan Wiesand <>
Reviewed-by: Benjamin Kaduk <>

5 years agorxinit_status needs to be global for the kext since 73/12073/3
Dave Botsch [Wed, 28 Oct 2015 15:53:26 +0000]
rxinit_status needs to be global for the kext since
RXK_UPCALL_ENV is defined in src/rx/DARWIN/rx_knet.c

Change-Id: I23b535f0cd6b45c3e186319c4bacf5b6c5a93681
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Michael Meffie <>
Tested-by: BuildBot <>

5 years agoInitial set of changes for El Capitan OS X 10.11 . 72/12072/3
Dave Botsch [Wed, 28 Oct 2015 15:28:01 +0000]
Initial set of changes for El Capitan OS X 10.11 .

Mainly new El Capitan specific config files and defitions of
Darwin 15 variables and config tests/etc.

Change-Id: I87b926109561f41ee95a2f3f94fbdbcf2903691a
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

5 years agoFix optimized IRIX kernel module builds 11/12111/2
Benjamin Kaduk [Mon, 23 Nov 2015 00:22:58 +0000]
Fix optimized IRIX kernel module builds

Commit 9f94892f8d996a522e7801ef6088a13769bee7c2 (from 2006)
introduced per-file CFLAGS, using $(CFLAGS-$@); this construct
is not parsed well by IRIX make, which ends up attempting to
expand '$@)' and finding mismatched parentheses.

Commit 5987e2923a2670a27a801461dc9668ec88ed7d2a (from 2007) followed,
fixing the IRIX build but only for the NOOPT case.  This left the
problematic expression in CFLAGS_OPT until 2013, when another RT
ticket was filed reporting the continued breakage.  That ticket
was then ignored until 2015 (now) with no particular cries of
outrage on the mailing lists.  Perhaps this gives some indication
of the size and/or mindset of the IRIX userbase.  (There have
been successful IRIX installations during this time period, so
presumably it was discovered that disabling optimizations helped
the build along.)

FIXES 131621

Change-Id: Id5298103221b016239723aa08ebe0dc54bdadc5e
Tested-by: BuildBot <>
Reviewed-by: Chas Williams <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

5 years agoLINUX: don't cache negative entries for dynroot 40/12140/4
Chas Williams [Thu, 24 Dec 2015 22:58:32 +0000]
LINUX: don't cache negative entries for dynroot

The dynroot volume lacks any callbacks that would invalidate the directory
or change the data version.  Further, the data version for the dynroot
is only updated for when a new cell is found or added (a positive lookup).

Change-Id: If0b022933de7335d3d94aafc77c50b85b99f4116
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoTypo fix in comment 33/12133/3
Benjamin Kaduk [Sat, 19 Dec 2015 07:08:06 +0000]
Typo fix in comment

afsd -shutdown takes only a single dash, as with all cmd-style

Improve the grammar a bit while we're here.

Change-Id: Ie96c80dba1770e735617e5c93fe3d4c1e3afd3a9
Reviewed-by: Michael Meffie <>
Reviewed-by: Chas Williams <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoafs: do not allow two shutdown sequences in parallel 16/12016/8
Marcio Barbosa [Tue, 29 Dec 2015 13:31:43 +0000]
afs: do not allow two shutdown sequences in parallel

Often, ‘afsd -shutdown’ is called right after ‘umount’.
Both commands hold the glock before calling ‘afs_shutdown’.
However, one of the functions called by 'afs_shutdown', namely,
‘afs_FlushVCBs’, might drop the glock when the global
'afs_shuttingdown' is still equal to 0. As a result, a scenario
with two shutdown sequences proceeding in parallel is possible.

To fix the problem, the global ‘afs_shuttingdown’ is used as an
enumerated type to make sure that the second thread will not run
‘afs_shutdown’ while the first one is stuck inside ‘afs_FlushVCBs’.

Change-Id: Iffa89d82278b0df5fb90fc35608af66d8e8db29e
Tested-by: BuildBot <>
Reviewed-by: Chas Williams <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

5 years agoredhat: Correct permissions on systemd unit files 74/12174/3
Brian Torbich [Thu, 21 Jan 2016 15:08:27 +0000]
redhat: Correct permissions on systemd unit files

Change the systemd unit file permissions created via
openafs.spec to be 0644 instead of 0755.  Having the
systemd unit files be executable will trigger a systemd

FIXES 132662

Change-Id: I9f5111c855941528193aaabeb42bf1b732246a7e
Tested-by: BuildBot <>
Reviewed-by: Stephan Wiesand <>
Reviewed-by: Benjamin Kaduk <>

5 years agoredhat: Avoid bogus dependencies when building the srpm 03/11903/2
Stephan Wiesand [Mon, 22 Jun 2015 08:44:11 +0000]
redhat: Avoid bogus dependencies when building the srpm

By default the spec defines that both userland and kernel module
packages should be built. This results in a dependency of the form
"kernel-devel-`uname -m` = `uname -r`" being added to the source
package created by, which is bogus because the uname
values are from the system on which the srpm is built and needn't
apply to the system where it is used. While rpm and rpmbuild ignore
such dependencies of source packages, other tools don't and may fail.

Some versions of rpmbuild will also enforce those requirements when
building the srpm itself, which is pointless too.

Avoid both problems by pretending not to attempt building modules
and ignoring any dependencies when invokes rpmbuild -bs.

Change-Id: I0134e1936638c7d9c3fd9ff0ccf1cba36710d0d3
Tested-by: BuildBot <>
Reviewed-by: Stephan Wiesand <>
Tested-by: Stephan Wiesand <>
Reviewed-by: Benjamin Kaduk <>

5 years agoTweak grammar in README 56/12156/2
Benjamin Kaduk [Tue, 29 Dec 2015 00:30:30 +0000]
Tweak grammar in README

So as to get a trivial change as confirmation that an updated
gerrit is functioning correctly.

Change-Id: I04eb12cab982a3f1b5ecc92d60c455e7a0d2242c
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

5 years agoRemove blank line from README 53/12153/5
Benjamin Kaduk [Mon, 28 Dec 2015 22:11:17 +0000]
Remove blank line from README

There's no reason for this file to start with a blank line.

Change-Id: I175390d3c9796fc10ef8086a5b179f4fc27362b5
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

5 years agoUpdate extra-iput configure argument description
Benjamin Kaduk [Wed, 4 Feb 2015 15:11:29 +0000]
Update extra-iput configure argument description

Commit 15260c7fdc5ac8fe9fb1797c8e383c665e9e0ccd did not function
as advertised, since the conditional which attempted to make
the configure option --(en|dis)able-linux-d_splice_alias-extra-iput
mandatory on linux checked a variable for the system type which
was not set at the time the check ran.

Subsequent discussion of this behavior produced a consensus that
there is not a need to make the configure option mandatory,
due to the narrow range of kernels affected by the bug in question,
so this follow-up commit just fixes the documentation and removes
the ineffective code.

Change-Id: I36d1f8801d355f33c3132fcab166ea76faab8e87
Reviewed-by: Anders Kaseorg <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agocompile_et: Don't overflow input file buffer
Simon Wilkinson [Mon, 4 Mar 2013 16:15:37 +0000]
compile_et: Don't overflow input file buffer

Don't overlow the buffer that's used for the input filename by
copying in to much with sprintf. Use asprintf to dynamically
allocate a buffer instead.

Link roken for rk_asprintf where needed.

Build compile_et with libtool, to ensure that it is linked statically,
as is needed for build tools such as compile_et.  (This requires
a preceding change to set a buildtool_roken make variable.)

Caught by coverity (#985907)

Change-Id: I207dd2c49bcae3f04fa41c826b08a0a615d5f422
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoProvide a buildtool_roken make variable
Benjamin Kaduk [Wed, 25 Feb 2015 23:46:28 +0000]
Provide a buildtool_roken make variable

When using roken in build tools, i.e., binaries which must be
executed during the build stage, the roken library must be usable
prior to the 'install' stage.  In particular, if the internal
rokenafs is used, the shared library will not be installed and
the runtime linker will not be able to find it, causing execution
of the build tool to fail.  To avoid this failure, librokenafs
must be linked statically into these build tools.

Unfortunately, the way we currently use libtool is not very
well aligned to libtools model of how it should be used.  As a result,
it does not seem feasible to cause libtool to link librokenafs
statically without breaking other parts of the build.

Libtool peeks at the compiler command-line arguments to affect its
behavior when invoked as a linker.  The flags -static, -all-static,
and -static-libtool-libs can affect whether dynamic or static linkage
is used for various libraries being linked into the executable.
Passing -all-static tells libtool to not do any dynamic linking at
all, but is silently a no-op if static linking is not possible (the
default situation on most modern Linuxen, OS X, and Solaris).
Passing -static causes libtool to not do any dynamic linking of
libtool libraries which have not been installed, and passing
-static-libtool-libs causes libtool to not do any dynamic linking
of libtool libraries at all.

In order to get libtool to actually link statically in all cases,
we should pass -all-static, not just -static.  However, because
too many platforms disallow static linking by default, this is
not a viable option.

If we retain the libtool archive in the linker search
path, libtool then records the library dependency of libafshcrypto on
librokenafs in its metadata and refuses to install to
any path other than the configured prefix.  This restriction of
libtool is incompatible with our use in 'make dest', and it is not
feasible to desupport 'make dest' before the 1.8 release.

The most appropriate workaround seems to be to just pass the
path to librokenafs.a on the linker command line when linking
build tools.  As such, provide a new make variable buildtool_roken
which is appropriate for linking roken into build tools -- this
variable will be set to the path to librokenafs.a when the internal
roken is used, and the normal -lrokenafs when an external roken
is used.

Change-Id: I079fc6de5d0aa6403eb1071f3d58a248b1777853
Tested-by: BuildBot <>
Reviewed-by: Chas Williams <>
Reviewed-by: Benjamin Kaduk <>

5 years agorxkad: Resolve warnings in ticket5.c
Anders Kaseorg [Fri, 31 Jul 2015 05:42:55 +0000]
rxkad: Resolve warnings in ticket5.c

Resolves these warnings:

ticket5.c: In function ‘tkt_MakeTicket5’:
ticket5.c:574:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncTicketPart’ differ in signedness [-Wpointer-sign]
     code = encode_EncTicketPart(encodebuf, allocsiz, &data, &encodelen);
In file included from ticket5.c:80:0:
v5gen-rewrite.h:43:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
 #define encode_EncTicketPart _rxkad_v5_encode_EncTicketPart
v5gen.c:1889:1: note: in expansion of macro ‘encode_EncTicketPart’
 encode_EncTicketPart(unsigned char *p, size_t len, const EncTicketPart * data, size_t * size)
ticket5.c:602:33: warning: pointer targets in passing argument 1 of ‘_rxkad_v5_encode_EncryptedData’ differ in signedness [-Wpointer-sign]
     code = encode_EncryptedData(ticket + *ticketLen - 1, *ticketLen, &encdata, &tl);
In file included from ticket5.c:80:0:
v5gen-rewrite.h:16:30: note: expected ‘unsigned char *’ but argument is of type ‘char *’
 #define encode_EncryptedData _rxkad_v5_encode_EncryptedData
v5gen.c:690:1: note: in expansion of macro ‘encode_EncryptedData’
 encode_EncryptedData(unsigned char *p, size_t len, const EncryptedData * data, size_t * size)
ticket5.c: In function ‘tkt_DecodeTicket5’:
ticket5.c:320:10: warning: ‘plainsiz’ may be used uninitialized in this function [-Wmaybe-uninitialized]
     code = decode_EncTicketPart((unsigned char *)plain, plainsiz, &decr_part, &siz);

Change-Id: Ic1b878f01cf82222dc258847747ce192ee5948fc
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

5 years agoAdd filepath entries for rxkad.keytab
Benjamin Kaduk [Tue, 3 Mar 2015 01:19:07 +0000]
Add filepath entries for rxkad.keytab

Even though master is not using it, we still want to be able to
find it.

Change-Id: I31fa39fe4d4bed5144c5169236b1106bd9f18501
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

5 years agoMake typedKey helpers more friendly to use
Benjamin Kaduk [Mon, 2 Mar 2015 21:05:51 +0000]
Make typedKey helpers more friendly to use

Make freeing a NULL key pointer a no-op.

Allow passing NULL to afsconf_typedKey_values() when not all
values are needed.

Change-Id: I3a4088747913e9e88be094da891cd2cca0cbb114
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

5 years agoviced: do not overwrite possible failure
Marcio Barbosa [Thu, 24 Dec 2015 20:23:23 +0000]
viced: do not overwrite possible failure

The function ‘hpr_Initialize’ overwrites the code
returned by ‘ubik_ClientInit’. As a result, ‘hpr_Initialize’
will not report any failure triggered by ‘ubik_ClientInit’.

To fix this problem, store the code returned by ‘rxs_Release’
in a new variable. Only return this code if the function
‘ubik_ClientInit’ worked properly. Otherwise, return the code
provided by ‘ubik_ClientInit’.

Change-Id: I1820e3cbc2131daace01cec0464e56fd2982a783
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

5 years agoafs: incorrect types for AFSDB IPv4 addresses
Mark Vitale [Tue, 24 Nov 2015 02:03:57 +0000]
afs: incorrect types for AFSDB IPv4 addresses

DNS lookup results were being handled with int types.

Fortunately, this seems to be harmless, due to use of
memcpy when the types don't match, and assignment only
when both sides were int.

However, to avoid any future unpleasantness, change
them to afs_uint32.

No functional change should be incurred.

Change-Id: I31aeabb4ae3194a00b29a1fa767d05af167b4e4f
Tested-by: BuildBot <>
Reviewed-by: Chas Williams <>
Reviewed-by: Benjamin Kaduk <>

5 years agoafs: pioctl kernel memory overrun
Mark Vitale [Fri, 7 Aug 2015 15:56:16 +0000]
afs: pioctl kernel memory overrun

Any pioctl with an input buffer size (ViceIoctl->in_size)
exactly equal to AFS_LRALLOCSIZE (4096 bytes) will cause
a one-byte overwrite of its kernel memory working buffer.
This may crash the operating system or cause other
undefined behavior.

The attacking pioctl must be a valid AFS pioctl code.
However, it need not specify valid arguments (in the ViceIoctl),
since only rudimentary checking is done in afs_HandlePioctl.
Most argument validation occurs later in the individual
pioctl handlers.

Nor does the issuer need to be authenticated or authorized
in any way, since authorization checks also occur much later,
in the individual pioctl handlers.  An unauthorized user
may therefore trigger the overrun by either crafting his
own malicious pioctl, or by issuing a privileged
command, e.g. 'fs newalias', with appropriately sized but
otherwise arbitrary arguments.  In the latter case, the
attacker will see the expected error message:
 "fs: You do not have the required rights to do this operation"
but in either case the damage has been done.

Pioctls are not logged or audited in any way (except those
that cause loggable or auditable events as side effects).

root cause:
afs_HandlePioctli() calls afs_pd_alloc() to allocate two
two afs_pdata structs, one for input and one for output.
The memory for these buffers is based on the requested
size, plus at least one extra byte for the null terminator
to be set later:
  requested size allocated
  ================= =================================
  > AFS_LRALLOCSIZ osi_Alloc(size+1)

afs_HandlePioctl then adds a null terminator to each buffer,
one byte past the requested size.  This is safe in all cases
except one: if the requested in_size was _exactly_
AFS_LRALLOCSIZ (4096 bytes), this null is one byte beyond
the allocated storage, zeroing a byte of kernel memory.

Commit 6260cbecd0795c4795341bdcf98671de6b9a43fb introduced
the null terminators and they were correct at that time.
But the commit message warns:
 "note that this works because PIGGYSIZE is always less than

Commit f8ed1111d76bbf36a466036ff74b44e1425be8bd introduced
the bug by increasing the maximum size of the buffers but
failing to account correctly for the null terminator in
the case of input buffer size == AFS_LRALLOCSIZ.

Commit 592a99d6e693bc640e2bdfc2e7e5243fcedc8f93 (master
version of one of the fixes in the recent 1.6.13 security
release) is the fix that drew my attention to this new
bug.  Ironically, 592a99 (combined with this commit), will
make it possible to eliminate the "offending" null termination
line altogether since it will now be performed automatically by

[ adjust commit message for CVE number assignment,
reduce unneeded churn in the diff.]

Change-Id: I1a536b3a53ec4b6721fbd39a915207da4358720c

6 years agoviced: add missing new lines to log messages
Michael Meffie [Fri, 17 Apr 2015 00:03:21 +0000]
viced: add missing new lines to log messages

The server logger requires an explicit new line.

Change-Id: Iffbfcfee7499bfa745a63d1b5ccb8038ee06acd0
Tested-by: BuildBot <>
Reviewed-by: Mark Vitale <>
Reviewed-by: Benjamin Kaduk <>

6 years agovolser: range check acl header fields during dumps and restores
Michael Meffie [Fri, 30 Jan 2015 17:12:03 +0000]
volser: range check acl header fields during dumps and restores

Perform range checks on the acl header fields when reading an
acl from a dump stream and when writing an acl to a dump

Before this change, a bogus value in the total, positive, or
negative acl fields from a dump stream could cause an out of
bounds access of the acl entries table, crashing the volume

Change-Id: Ic7d7f615a37491835af8d92f3c5f1b6a667d9d01
Tested-by: BuildBot <>
Reviewed-by: Mark Vitale <>
Reviewed-by: Benjamin Kaduk <>

6 years agovolser: set error, not code, before rfail
Benjamin Kaduk [Sun, 22 Nov 2015 19:24:43 +0000]
volser: set error, not code, before rfail

The rfail cleanup handler overwrites 'code' ~unconditionally, but
does use an existing 'error' value if present.  Since the intent
is to return failure to the caller, preserve the code in the error
variable and do so.

FIXES 131897

Change-Id: I25db2f9ad75a5b856626d39d35f97a09f26bd7a9
Reviewed-by: Jeffrey Altman <>
Tested-by: BuildBot <>

6 years agoutil: add CloseLog routine to close the server log
Michael Meffie [Thu, 10 Sep 2015 01:24:04 +0000]
util: add CloseLog routine to close the server log

Add the missing complement to OpenLog().

Change-Id: I45e7e5d2da3241c163d2d4baa6b386167e90e582
Reviewed-by: Marcio Brito Barbosa <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agosalvager: redd up showlog global flag
Michael Meffie [Wed, 9 Sep 2015 17:22:26 +0000]
salvager: redd up showlog global flag

Clean up the show log flag so it is only set by the salvager and
is reset when spawning a child process.

Change-Id: I1702cf98faca583409594d1199a8215ffe08a75e
Reviewed-by: Mark Vitale <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agodafs: log to stderr when running salvageserver in client mode
Michael Meffie [Mon, 9 Feb 2015 15:14:41 +0000]
dafs: log to stderr when running salvageserver in client mode

When the -client option is given to the salvageserver, print
Log() messages to stderr instead of losing them.

Change-Id: I065e8136db9a8cc241639fbe34607db884751b95
Reviewed-by: Perry Ruiter <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agodafs: remove the salvageserver -showlog option
Michael Meffie [Mon, 30 Mar 2015 17:20:42 +0000]
dafs: remove the salvageserver -showlog option

Remove the salvagerserver option to print log messages to stdout.  This
was a carry over from the stand-alone salvager and is not appropriate for
a daemon.

Change-Id: I55b99112278cdabb3e9911948dbda6a628030951
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agogtx: use getmaxyx() with sensible fallbacks
Brandon S Allbery [Tue, 24 Nov 2015 21:39:02 +0000]
gtx: use getmaxyx() with sensible fallbacks

configure now checks for the standard getmaxyx() macro; failing that,
it looks for the older but pre-standardization getmaxx() and getmaxy(),
then falls back to the 4.2BSD curses _maxx and _maxy fields; if all
else fails, gtx building is disabled.

gtx now defines getmaxyx() itself if necessary, based on the above.

This also fixes a bug in gtx with all ncurses versions > 1.8.0 on
platforms other than NetBSD and OS X: gtx was using the _maxx and
_maxy fields, which starting with ncurses 1.8.1 were off by 1 from
the expected values. As such, behavior of scout and/or afsmonitor
may change on most ncurses-using platforms.

Change-Id: I49778e87adacef2598f0965e09538dfc3d840dcc
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Chas Williams <>
Reviewed-by: Benjamin Kaduk <>

6 years agoOpen syscall emulation file O_RDONLY
Chas Williams [Wed, 2 Dec 2015 15:38:42 +0000]
Open syscall emulation file O_RDONLY

As reported on the -info mailing list, docker is now exporting the
/proc filesystem as read only.  ioctl() doesn't need write permissions
to do its work, so change O_RDWR to O_RDONLY.

Change-Id: I2068888b13b6b5e31b1a2205bbcbe43f5f9fc55a
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>
Reviewed-by: Benjamin Kaduk <>

6 years agodafs: remove the salvageserver -datelogs option
Michael Meffie [Mon, 30 Mar 2015 17:17:25 +0000]
dafs: remove the salvageserver -datelogs option

Remove the undocumented -datelogs option from the salavageserver, which
was a carry over from the standalone salvager program, but is not
appropriate for a daemon.

Change-Id: Ia382d6550e0641edcba55a414e00323755487e18
Reviewed-by: Perry Ruiter <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agodoc: remove unimplemented -showsuid and -showmounts from the salvageserver man page
Michael Meffie [Tue, 31 Mar 2015 15:08:34 +0000]
doc: remove unimplemented -showsuid and -showmounts from the salvageserver man page

These options were copied from the salvager man page and are not implemented by
the salvageserver.

Change-Id: Ib6c5b3fd494f1662b958442863e5fbfc0755a0c2
Reviewed-by: Perry Ruiter <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agoFix ptserver -default_access parsing
Benjamin Kaduk [Sun, 22 Nov 2015 22:34:16 +0000]
Fix ptserver -default_access parsing

Commit 0b9986c8758c13a1de66b8bdae51b11abaea6cf3 converted ptserver
to use libcmd for parsing, but erroneously listed the
-default_access argument as CMD_SINGLE instead of CMD_LIST, since
two arguments are needed.  This made it impossible to use
-default_access at all, since libcmd would reject an extra argument
and the later argument processing would notice that the second
argument was missing.

FIXES 131731

Change-Id: Ib8241308d4f40f980d635513e2255aafa06c3d8a
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agodoc: add syslog options to salvageserver man page
Michael Meffie [Tue, 31 Mar 2015 15:04:26 +0000]
doc: add syslog options to salvageserver man page

Add the missing -syslog and -syslogfacility options to
the salvageserver man page.

Change-Id: I1cb057a8085c4aeda32bb003cc4cec5035d00407
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agotabular_output: allocate footer-line when set for the first time
Christof Hanke [Wed, 18 Nov 2015 13:02:50 +0000]
tabular_output: allocate footer-line when set for the first time

If the footer line is not allocated, programs segfault at runtime.
The printFooter functions should check if the footer
is allocated before printing them.

Change-Id: Ib4066a67ee104be918811e178c0b7d7d33d790b8
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agovlserver: VL_GetEntryByName* requests undercounted
Mark Vitale [Wed, 18 Nov 2015 20:09:37 +0000]
vlserver: VL_GetEntryByName* requests undercounted

Commit a14e791541bf19c6c377e68bc2f978fba34f94b1
refactored and corrected the counting of requests and aborts.
However, it inadvertently introduced a new undercount for
VL_GetEntryByName* requests, counting them only if
NameIsId(volname), e.g. volname="536870911".

Ensure that the normal case of a non-"numeric" volname is
also counted.

Discovered during review of pullup to 1.6.x.

Change-Id: Ic5dbc1a5871d0e0ff184dc4f3b11e92166c92f65
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agowriteconfig: emit error messages again in VerifyEntries
Stephan Wiesand [Tue, 17 Nov 2015 14:03:03 +0000]
writeconfig: emit error messages again in VerifyEntries

Before commit e4a8a7a38dbf29e89bc1a7b6b017447a6aa0c764 an error message
was printed if looking up a server hostname failed. Restore this, and
also print a message in the now detected case that the lookup returns
loopback addresses only.

Change-Id: Idf7c3133ab5c83e081335ba1dc8fcbddb7da7329
Tested-by: BuildBot <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Benjamin Kaduk <>

6 years agoCODING: permit --enable-checking with clang
Benjamin Kaduk [Fri, 28 Aug 2015 01:20:58 +0000]
CODING: permit --enable-checking with clang

Starting at 3.2, a mostly arbitrarily selected version.

Change-Id: I9f6a946e2571b939911cbf4b1b64e1d62e39e1a3
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agodoc: fix the salvageserver log file name
Michael Meffie [Mon, 9 Feb 2015 14:37:54 +0000]
doc: fix the salvageserver log file name

Fix capitialization of the salvageserver log file name.

Change-Id: If08dd191e35e7fb15db533a623b832154a6f9f17
Tested-by: BuildBot <>
Reviewed-by: Chas Williams <>
Reviewed-by: Stephan Wiesand <>
Reviewed-by: Benjamin Kaduk <>

6 years agobozo: create a syslog connection only if the -syslog option is given
Michael Meffie [Wed, 21 Jan 2015 19:42:47 +0000]
bozo: create a syslog connection only if the -syslog option is given

Fix a minor bug in which an unnecessary syslog connection is opened when
the BosLog is not present (typically, the first time the bosserver is
started) or when the BosLog is a named pipe, even if the -syslog option
was not given.

This bug was introduced in commit bdc7e43117706d0aa46d3b6435489e9d4c2b0888,
which added checks to avoid renaming logs when they are named pipes.

lstat() and S_ISFIFO are provided by libroken, so do not need to be hidden
behind conditional compilation.

Change-Id: I828534be69949fe017cc7dbed1b6798aa4c0ba17
Reviewed-by: Perry Ruiter <>
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agoprdb_check: fix out of bounds array access in continuation entries
Michael Meffie [Wed, 18 Feb 2015 02:54:46 +0000]
prdb_check: fix out of bounds array access in continuation entries

A continuation entry (struct contentry) contains 39 id elements, however
a regular entry (struct prentry) contains only 10 id elements.
Attempting to access more than 10 elements of a regular entry is
undefined behavior.

Use a stuct contentry when when processing continuation entries in
prdb_check.  This is done to safely traverse the id arrays of the
continuation entries.  Use the new pr_PrintContEntry to print
continuation entries.

The undefined behavior manfests as a segmentation violation in
WalkNextChain() when built with GCC 4.8 with optimization enabled.

Change-Id: I7613345ee6b7b232c5a0645f4f302c3eac0cdc15
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agoprdb_check: check for continuation entries in owner chains
Michael Meffie [Wed, 18 Feb 2015 01:58:27 +0000]
prdb_check: check for continuation entries in owner chains

Continuation entries may not be in owner chains. Fix the
comments in WalkOwnerChain (which were probably copied from
WalkNextChain) and add a check and error message for
continuation entries found on owner chains.

Change-Id: I8c49378478cf6a3d31317ff90a52fe1e74517dd3
Reviewed-by: Daria Phoebe Brashear <>
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agolibprot: add pr_PrintContEntry function
Michael Meffie [Wed, 18 Feb 2015 02:11:50 +0000]
libprot: add pr_PrintContEntry function

A continuation entry (struct contentry) contains 39 id elements, however
a regular entry (struct prentry) contains only 10 id elements. Attempting
to access more than 10 elements of a regular entry is undefined

Add a new function to safely print continuation entries and change
pr_PrintEntry to avoid accessing the entries array out of bounds.

The pr_PrintEntry function is at this time only used by the prdb_check
and ptclient debugging utilities.

Change-Id: Ie836983c8a5970a9495b87d0627ba6c05d117a9b
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agodoc: document the version subcommand
Michael Meffie [Wed, 21 May 2014 21:27:47 +0000]
doc: document the version subcommand

Document the built-in version sub-command which displays
the OpenAFS version string. This sub-command is provided
by the cmd library.

Document the switch style -version option provided by the cmd
library for the initcmd based commands: afsmonitor, scout,
xstat_fs_test, and xstat_cm_test.

Change-Id: Id421d2c68a5c49a2b1a5abb2f3e9ca64ea36cd3e
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agoafs: fix for return an error from afs_readdir when out of buffers
Michael Meffie [Tue, 13 Oct 2015 02:16:54 +0000]
afs: fix for return an error from afs_readdir when out of buffers

Commit 9b0d5f274fe79ccc5dd0e4bba86b3f52b27d3586 added a return code to
BlobScan to allow afs_readdir to return an error when afs_newslot failed
to allocate a buffer.  Unfortunately, that change introduced a false
EIO error.

Originally, BlobScan would return a blob number of 0 to indicate the end
of the file has been reached while traversing the directory blobs.
Restore that behavior by changing the cache manager's DRead function to
return ENOENT instead of the generic EIO error to indicate the page to
be read is out of bounds, and change BlobScan to return a blob of zero
to indicate to callers the last blob has been reached.  All callers
already check for a blob number of zero, which is out of range.

Change-Id: I5baae8e5377dd49dcca6765b7a4ddc89cca70738
Tested-by: BuildBot <>
Reviewed-by: Mark Vitale <>
Tested-by: Mark Vitale <>
Reviewed-by: Benjamin Kaduk <>

6 years agovos: reinstate the -localauth option for vos setaddrs
Michael Meffie [Fri, 6 Nov 2015 16:56:31 +0000]
vos: reinstate the -localauth option for vos setaddrs

Commit d1d411576cf39c4bc55918df0eb64327718d566c added the vos remaddrs
subcommand, but unfortunately stole the common parameters from
setaddrs.  Fix this bug and remove the extra blank line between
the subcommand syntax and the common params macro.

Change-Id: I1171bfadec08ac34679204f0a9245d76c468cafa
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agoMake libuafs safe for parallel make
Tim Creech [Mon, 2 Nov 2015 13:12:32 +0000]
Make libuafs safe for parallel make

In src/libuafs, "make" with a large number of jobs (e.g., "make -j16")
can fail because some of the LT_objs depend on make_h_tree having been
called already.

Make "h" (the libuafs header subdirectory) a dependency of all of

Change-Id: Ie005dbb1f1b0a794c703147062615808a45956dc
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agorx: OPENAFS-SA-2015-007 "Tattletale"
Jeffrey Altman [Fri, 9 Oct 2015 02:22:12 +0000]
rx: OPENAFS-SA-2015-007 "Tattletale"


The CMU/Transarc/IBM definition of rx_AckDataSize(nAcks) was mistakenly
computed from sizeof(struct rx_ackPacket) and inadvertently added three
octets to the computed ack data size due to C language alignment rules.
When constructing ack packets these three octets are not assigned a
value before writing them to the network.

Beginning with AFS 3.3, IBM extended the ACK packet with the "maxMTU" ack
trailer value which was appended to the packet according to the
rx_AckDataSize() computation.  As a result the three unassigned octets
were unintentionally cemented into the ACK packet format.

In OpenAFS commit 4916d4b4221213bb6950e76dbe464a09d7a51cc3 Nickolai
Zeldovich <> noticed that the size produced by the
rx_AckDataSize(nAcks) macro was dependent upon the compiler and processor
architecture.  The rx_AckDataSize() macro was altered to explicitly
expose the three octets that are included in the computation.
Unfortunately, the failure to initialize the three octets went unnoticed.

The Rx implementation maintains a pool of packet buffers that are reused
during the lifetime of the process.  When an ACK packet is constructed
three octets from a previously received or transmitted packets will be
leaked onto the network.  These octets can include data from a
received packet that was encrypted on the wire and then decrypted.

If the received encrypted packet is a duplicate or if it is outside the
valid window, the decrypted packet will be used immediately to construct
an ACK packet.


In OpenAFS commit c7f9307c35c0c89f7ec8ada315c81ebc47517f86 the ACK packet
was further extended in an attempt to detect the path MTU between two
peers.  When the ACK reason is RX_ACK_PING a variable number of octets is
appended to the ACK following the ACK trailers.

The implementation failed to initialize all of the padding region.
A variable amount of data from previous packets can be leaked onto the
network.  The padding region can include data from a received packet
that was encrypted on the wire and then decrypted.

OpenAFS 1.5.75 through 1.5.78 and all 1.6.x releases (including release
candidates) are vulnerable.


  Thanks to John Stumpo for identifying both vulnerabilities.

  Thanks to Simon Wilkinson for patch development.

  Thanks to Ben Kaduk for managing the security release cycle.

Change-Id: I29e47610e497c0ea94033450f434da11c367027c

6 years agoWindows: CM_ERROR_INEXACT_MATCH is not a fatal error
Jeffrey Altman [Mon, 12 Oct 2015 13:56:07 +0000]
Windows: CM_ERROR_INEXACT_MATCH is not a fatal error

cm_BPlusDirLookup() and cm_Lookup() can return CM_ERROR_INEXACT_MATCH
which is not a fatal error.  Instead it is an indication that the returned
cm_scache object was not a case sensitive match.  Do not fail the request
and do not leak the cm_scache reference.

Change-Id: Ieef3ce1ac96a8794859b5b9c530545d4fdd26bd5
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: cm_Lookup return ambiguous filename to caller
Jeffrey Altman [Wed, 8 Jul 2015 23:47:26 +0000]
Windows: cm_Lookup return ambiguous filename to caller

cm_Lookup() must not mask a CM_ERROR_AMBIGUOUS_FILENAME error by
converting it to CM_ERROR_BPLUS_NOMATCH.  Doing so results in the
redirector believing that the object does not exist instead of

Change-Id: Iaa84d50271c234a84e11dd58d78ef90f5d224032
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: fix RDR detection of ambiguous directory entries
Jeffrey Altman [Wed, 8 Jul 2015 20:49:38 +0000]
Windows: fix RDR detection of ambiguous directory entries

The redirector is supposed to reject access to file objects if there
is no case exact match and multiple entries match in a case insensitive
comparison.  The check was only present in the AFSLocateNameEntry()
function and not elsewhere.

Fix the AFSLocateNameEntry() call and addd the missing checks.

Change-Id: I15aba954179fa85e28b348989779bc05122c0037
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: rdr pioctl operations are opaque
Jeffrey Altman [Mon, 19 Oct 2015 00:32:06 +0000]
Windows: rdr pioctl operations are opaque

Although pioctl operations are delivered through the redirector the
contents of the operations are opaque to the redirector.  Therefore,
the cm_req must not be initialized as a redirector operation.  If they
are the necessary invalidation notifications for symlink and mount point
operations will not be delivered.

Change-Id: I48c2d89d2b2e0fc3f0ef56e7731108a8c51e1674
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: clientServiceProviderKeyExists use subkey
Jeffrey Altman [Thu, 15 Oct 2015 17:22:05 +0000]
Windows: clientServiceProviderKeyExists use subkey

clientServiceProviderKeyExists() must use AFSREG_CLT_SVC_PROVIDER_SUBKEY
since it is a relative path from HKEY_LOCAL_MACHINE.

Change-Id: I975d594bfe69c563f692978057c08b834d54b8b1
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: if no known IP addrs, query the addr list
Jeffrey Altman [Fri, 9 Oct 2015 14:20:41 +0000]
Windows: if no known IP addrs, query the addr list

If cm_noIPAddrs == 0, then no servers will be probed.   If
syscfg_GetIFInfo() fails then cm_noIPAddrs is set to 0.  Therefore,
also set cm_LanAdapterChangeDetected to non-zero if syscfg_GetIFInfo()
fails so that the interface info can be queried again prior to a server
probe attempt.

Change-Id: I6639441fa6266671cfb875256eb23c3b018e67c9
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: Only install Service NP if one exists
Jeffrey Altman [Wed, 7 Oct 2015 22:09:50 +0000]
Windows: Only install Service NP if one exists

Do not blindly install a network provider for the service since at
least one end user organization does not install the service's network

Change-Id: I15a528ff34ffd3e060fdbd93545af3857592c835
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>
Tested-by: Jeffrey Altman <>

6 years agoWindows: InstNetProvider do not leak key handle
Jeffrey Altman [Wed, 7 Oct 2015 22:09:17 +0000]
Windows: InstNetProvider do not leak key handle

If we open a handle, close it.

Change-Id: I1a5b2308a91f3c66791e65f76ca17ae52d34789f
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Jeffrey Altman <>
Tested-by: Jeffrey Altman <>

6 years agoWindows: multi ping do not leak ping count
Jeffrey Altman [Wed, 30 Sep 2015 17:23:36 +0000]
Windows: multi ping do not leak ping count

In cm_CheckServersMulti() if cm_ConnByServer() fails or if cm_noIPAddr is
zero then a cm_server.pingCount will be leaked.  This can result in
servers being marked down and never restored to an up state.

This change adds the necessary pingCount decrement and moves the
assignment of the cm_server_t pointer to serversp[] to make it clear
that the cm_server_t will not be in the array if a failure occurs.
Only objects in the array will have the pingCount decremented after
the RPCs are issued.

Change-Id: I18895c848039e4131a674d814019f236a1b0e5b5
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoSOLARIS: Minor whitespace corrections
Perry Ruiter [Sun, 13 Sep 2015 15:53:22 +0000]
SOLARIS: Minor whitespace corrections

Fix some incorrectly indented code in osi_file.c

Change-Id: I75a8ec18bfef13bb05a99f84b2cfbfae34fcd440
Tested-by: BuildBot <>
Reviewed-by: Marcio Brito Barbosa <>
Reviewed-by: Andrew Deason <>
Reviewed-by: Jeffrey Altman <>

6 years agovolser: update log formatting in dump and restore
Michael Meffie [Fri, 30 Jan 2015 16:53:58 +0000]
volser: update log formatting in dump and restore

Update the log messages to use modern formatting specifiers for
volume ids and inodes in the volume dump and restore code.

Change-Id: Ic2844e389e5951d2f2bbbc31a86c2342f2e8d848
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agoWindows: remove extraneous "pingCount" format param
Jeffrey Altman [Fri, 25 Sep 2015 22:12:24 +0000]
Windows: remove extraneous "pingCount" format param

In 0a0927497c8165aec11e718df01632da75fa4cdc an extra "pingCount"
format parameter was added in cm_DumpServers().  Remove it.

Change-Id: I79c2212c11319d7f94f963214d90b0530a978ab5
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: Replace CM_SERVERFLAG_PINGING with pingCount
Jeffrey Altman [Tue, 16 Jul 2013 13:10:04 +0000]
Windows: Replace CM_SERVERFLAG_PINGING with pingCount

Instead of replying upon a server flag use a pingCount interlocked
variable to track whether active ping operations are being performed
and whether or not to wait sleeping threads.

Change-Id: Ie967beee0debdb9c0963ca40b12737bd3fa88548
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: cm_GetCell_Gen rework cell prefix matching
Jeffrey Altman [Thu, 14 May 2015 22:29:45 +0000]
Windows: cm_GetCell_Gen rework cell prefix matching

The cm_GetCell_Gen() function permits cells to be searched for by
prefix.  The idea is to permit "" to be abbreviated "cs"
when at CMU.  There are two problems with the current behavior:

1. the existing match rules will accept "cs.c" and "cs.cmu.ed" as
   valid prefix matches.  By not restricting the prefix matching
   to full components the Freelance symlink list can become

2. the existing match rules will accept the first cell that
   matches even if there are more than one cells that would match.
   this can result in unpredictable behavior since the ordering
   of the cells is not guaranteed.

Instead, fail requests for cell prefixes that are not full component
matches or that would be ambiguous.

Change-Id: I59fb5ea9bba4cebdd71808fc9fae9662456943e0
Reviewed-by: Jeffrey Altman <>
Tested-by: BuildBot <>

6 years agoWindows: Network Provider registration at service start
Jeffrey Altman [Mon, 4 May 2015 17:25:04 +0000]
Windows: Network Provider registration at service start

Windows 8, 8.1 and pre-releases of 10 have a horrible bug as part
of the upgrade process.  All non-Microsoft network provider services
are removed from the NetworkProvider "Order" registry value.  For
OpenAFS this has the side effect of breaking integrated logon and
all drive letter mappings to \\AFS.

During service start add code to:

 1. Add "AFSRedirector" before "LanmanWorkstation" if not present
 2. Add "TransarcAFSDaemon" to the end of the list if not present

If the service is running in SMB mode

 3. Remove "AFSRedirector" if present

Change-Id: I14a703e44c6e0ee1bd36afd306f92a17dcc0d2a5
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: cm_Analyze mark server down for misc rx errors
Jeffrey Altman [Sun, 28 Jun 2015 19:06:34 +0000]
Windows: cm_Analyze mark server down for misc rx errors

In cm_Analyze() replace the token error retry logic for miscellaneous
rx errors and simply mark the server down.  The most common error
that will be seen in this category is RX_INVALID_OPERATION which would
be received if the Rx service id or security class is not recognized
by the peer.  This could happen if an AuriStor server is replaced by
an AFS3 server or if a packet is reflected.

A side effect of this change is that V* and CM_ERROR_* errors will
once again be retried.  This will permit proper failover to occur.

Change-Id: I77e6325eb05643ea6df1fc0bc877bd4ef496c974
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: avoid vldb lookup race with network stack
Jeffrey Altman [Sun, 28 Jun 2015 18:56:47 +0000]
Windows: avoid vldb lookup race with network stack

If a VLDB query attempt occurs when there is no current cell db server
list then the VLDB query won't actually occur but the last query time
would be set.  This prevents a query from taking place again on the volume
for 60 seconds.  If the volume in question is the root.cell volume then
the redirector will be forced to return device not ready for the share
(aka \\afs\cell).

Check for a failure of cm_UpdateCell() and only set the last update time
for the volume if there was success or if the VLDB responded with volume

Change-Id: Ic87d871feac3f2ea3d3db377854efa9dc9db3c00
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: cm_ApplyDir calls cm_SyncOpDone too soon
Jeffrey Altman [Sun, 28 Jun 2015 18:00:24 +0000]
Windows: cm_ApplyDir calls cm_SyncOpDone too soon

cm_ApplyDir() failed to maintain the synchronization state while the
GetBuffer() operation proceeded.

Change-Id: I616622e9aebbdb20a325826032991e5d5c5d9e24
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: cm_CheckNTDelete missing SyncDone call
Jeffrey Altman [Sun, 28 Jun 2015 17:59:28 +0000]
Windows: cm_CheckNTDelete missing SyncDone call

cm_CheckNTDelete() forgot to call cm_SyncDone() in one of the error
paths.  Fixup the call pattern and do not forget to call cm_SyncDone().

Change-Id: I9274b65c5a5f22ca71e0b10f860d57d7e567a56c
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: NPSupport Remote Name verification
Jeffrey Altman [Sun, 28 Jun 2015 17:51:40 +0000]
Windows: NPSupport Remote Name verification

When adding a connection verify that the server name and the share name
are valid.  If not return ERROR_BAD_NETWORK_NAME.

When getting connection information, if a pre-existing connection does
not exist and either the server name or the share name do not verify

Change-Id: Ib40a6b56318793d1c1b351ba895736beb616585d
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: NPGetResourceInformation return redir error
Jeffrey Altman [Sun, 28 Jun 2015 17:43:03 +0000]
Windows: NPGetResourceInformation return redir error

When the redirector ioctl fails in NPGetResourceInformation() return the
actual error to the caller.   Do not hide all errors as WN_BAD_NETNAME.

Change-Id: Ie02ca5331aa34aef4476c99045048871c6c25de0
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: NP RemoteName Length checks
Jeffrey Altman [Sun, 28 Jun 2015 17:39:32 +0000]
Windows: NP RemoteName Length checks

Ensure that RemoteName paths have at least two characters before
attempting to access character [1].

Change-Id: I75487056686dccf2bf57b22e7c99e9d8210eaaf3
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: AFSParseName() uniFullName.MaximumLength
Jeffrey Altman [Sun, 28 Jun 2015 17:27:03 +0000]
Windows: AFSParseName() uniFullName.MaximumLength

The uniFullName.MaximumLength in AFSParseName() is not properly
modified and can end up being extended beyond the actual memory
allocation due to a missing decrement.

Change-Id: I070ee33acd32849d05bbc83c6e7cfaf55e6a0997
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: remove dead network provider code
Jeffrey Altman [Sun, 28 Jun 2015 17:24:13 +0000]
Windows: remove dead network provider code

Remove all #if 0 code blocks.

Change-Id: I981d7a178c0ae1be7b3ca9f546a7a1aab8f5a48c
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: npdll do not retrieve auth id
Jeffrey Altman [Sun, 28 Jun 2015 17:21:35 +0000]
Windows: npdll do not retrieve auth id

The authentication id for the process will always be obtained in kernel
so no longer try to fetch it in userland.

Change-Id: I8d35af1349e137b8a3d7d8299b16e443710c6482
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: Always fetch auth id in kernel
Jeffrey Altman [Sun, 28 Jun 2015 17:18:01 +0000]
Windows: Always fetch auth id in kernel

When processing network provider requests in afsredirlib.sys always
obtain the auth id using the SYSTEM worker thread.   Do not trust
the values provided by userland.

Change-Id: I9786b0c836cf967074035a7595c38c8700cb7589
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: Move GetAuthenticationId to Worker Thread
Jeffrey Altman [Sun, 28 Jun 2015 17:12:13 +0000]
Windows: Move GetAuthenticationId to Worker Thread

When PsReferenceImpersonationToken(), PsReferencePrimaryToken(), and
SeQueryInformationToken() are called in the kernel from a user process
thread the restrictions on the userland process still apply.  Since we do
not want to be restricted we must obtain the token and query the token
information from a SYSTEM thread.

This change restructures the AFSGetAuthenticationId() process to queue a
synchronous task to the worker thread.

This should address the problem that has been seen during system boot when
the Group Policy Service attempts to query, remove or create a drive
letter mapping.

Change-Id: Ib8772e185aa1e4e52979ec847bbc18a9878bcaca
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: AFSRetrieveFileAttributes no parent path
Jeffrey Altman [Tue, 9 Jun 2015 12:55:44 +0000]
Windows: AFSRetrieveFileAttributes no parent path

Modify AFSRetrieveFileAttributes() to handle the case of a ParentPathName
with a Length == 0.   In such a case the introduction of a path separator
would result in the construction of an absolute path when a relative path
is required.

Change-Id: I2e633b22992b0aee914927a451bb146fc57110e8
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: AFSRetrieveParentPath handle no parent
Jeffrey Altman [Tue, 9 Jun 2015 12:44:43 +0000]
Windows: AFSRetrieveParentPath handle no parent

AFSRetrieveParentPath() when presented with a relative path that has no
parent will walk off the front of the FullFileName buffer.  Add checks to
ensure that Length never becomes less than zero.

Change-Id: I7d619dc569d6c002b1d236a9340921414c51647f
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: AFSGetConnectionInfo partial match validation
Jeffrey Altman [Wed, 13 May 2015 03:15:50 +0000]
Windows: AFSGetConnectionInfo partial match validation

AFSGetConnectionInfo() is called to respond to NPGetResourceInformation
and NPGetConnectionPerformance WNet API requests.  The former permits
the requestor to provide a path containing components that are not
processed by the file system represented by the called network provider.
As such partial matches are permitted BUT they must consist of full
components.  In other words, \\afs\sh is not a valid partial match for
\\afs\share but \\afs\share is a valid partial match for \\afs\share\dir.

This change adds validation checks to enforce full component comparisons.
It also cleans up some of the associated comparisons and trace output.

Change-Id: Ia736030f554f9770b201227c4dce26d7d45fe0d2
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: NetrShareGetInfo do not return registry errors
Jeffrey Altman [Mon, 6 Jul 2015 05:39:41 +0000]
Windows: NetrShareGetInfo do not return registry errors

In NetrShareGetInfo() when registry api calls fail do not leak the
error codes to the caller.  Instead, set the error to CM_ERROR_NOSUCHPATH
so that NERR_NetNameNotFound can be returned.

Change-Id: I2c8f12573ca604385176ebb18d92ff2f7023a27e
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agoWindows: NetrShareGetInfo no return buffer on error
Jeffrey Altman [Mon, 6 Jul 2015 05:38:01 +0000]
Windows: NetrShareGetInfo no return buffer on error

When processing the NetrShareGetInfo() pipe service rpc do not
allocate memory for the return buffer is the path cannot be found.

Change-Id: I782df44de4d6b7a4664234ae0f8295294b889469
Tested-by: BuildBot <>
Reviewed-by: Jeffrey Altman <>

6 years agolibafs: remove linux conditionals for md5 inode number calculation
Michael Meffie [Wed, 29 Apr 2015 15:54:45 +0000]
libafs: remove linux conditionals for md5 inode number calculation

Remove the conditionals which hide the md5 digest calculation for inode
numbers on non-linux platforms.  This feature was originally added to
support sites running on linux, but is generally useful and the
implementation is not specific to linux.

Change-Id: I7f406f9492780c1893dc1a2892db253b05036120
Tested-by: BuildBot <>
Reviewed-by: Perry Ruiter <>
Reviewed-by: Benjamin Kaduk <>

6 years agoopenafs: add a contributor code of conduct
Daria Phoebe Brashear [Thu, 20 Aug 2015 17:20:38 +0000]
openafs: add a contributor code of conduct

In the interest of fostering a friendly, welcoming environment
for contributors, institute a code of conduct for the project.

Adapted from the Contributor Covenant.


Change-Id: I4eb3b8a84981ef04f02e7d60ec46873305888147
Tested-by: BuildBot <>
Reviewed-by: Jonathan A. Kollasch <>
Reviewed-by: Thomas L. Kula <>
Reviewed-by: Nathaniel Filardo <>
Reviewed-by: Michael Meffie <>
Reviewed-by: Marcio Brito Barbosa <>
Reviewed-by: Stephan Wiesand <>
Reviewed-by: Benjamin Kaduk <>

6 years agoLINUX: Fix oops during negative dentry caching
Michael Meffie [Tue, 15 Sep 2015 17:33:12 +0000]
LINUX: Fix oops during negative dentry caching

Commit 2e9dcc069904aaa434787eec53c6f9821911cbab reinstated negative
dentry caching, but introduced an oops when fakestat is in use.  Be sure
the GLOCK is held when looking up the parent vcache dv when the parent
is a mount point and fakestat is in use, since the calls to do the
lookup require the GLOCK to be held.

Change-Id: I6c47fbf53280400bf40271b1ff2837bd7c6dc69e
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agodoc: remove supergroup caution from pt_util
Chas Williams (CONTRACTOR) [Fri, 16 Jan 2015 01:27:04 +0000]
doc: remove supergroup caution from pt_util

Supergroup information is explicitly present in -members

Change-Id: I25527c093858bc0b029417cbf2bb07717c50bb32
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agobozo: move more initialization code to functions
Michael Meffie [Thu, 22 Jan 2015 03:44:32 +0000]
bozo: move more initialization code to functions

Move the code to create the initial "localcell" configuration and the
code to get the rx bind address out of main and into separate

Replace the global array of host addresses used to get the rx bind
address with a local variable.

Replace the call to rx_getAllAddr() with rx_getAllAddrMaskMtu(). The
former is not safe to call before rx_InitHost().

Initialize the cell info structure to zeros when creating the initial
"localcell" configuration.

Change-Id: I756aef86018d0cdd499afa58fdea99a7ac0d99df
Tested-by: BuildBot <>
Reviewed-by: Perry Ruiter <>
Reviewed-by: Benjamin Kaduk <>

6 years agoopr: export softsig functions
Michael Meffie [Thu, 3 Sep 2015 20:07:32 +0000]
opr: export softsig functions

Add the softsig functions to the exported symbols list.

Change-Id: I9378297ae035111459e597ae211fe832e93b63e3
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agoafs: Increase vcache and dcache hash table sizes
Benjamin Kaduk [Thu, 15 Jan 2015 16:54:30 +0000]
afs: Increase vcache and dcache hash table sizes

Now that we are using a real hash function, larger hash tables
will be more useful.

The vcache hash tables are statically sized, and this increase will
add about a megabyte to the kernel module's memory footprint.

Update the algorithm used to dynamically size the dcache hash tables,
keeping the old behavior for small numbers of dcaches, but growing
the hash table's size to keep the average chain length near two
for a range of dcache numbers.  Cap the dcache hash tables at 32k
entries to avoid excessive resource usage.

This involves code from opr, namely opr/ffs.h, which is acceptable
in the kernel module because that header is a standalone header
like jhash.h, with no dependencies on the system.

Change-Id: I7cdb3e993b1c2ad177a46ecc06bfa2be52e619e5
Reviewed-by: Benjamin Kaduk <>
Tested-by: Benjamin Kaduk <>

6 years agoreturn an error from afs_readdir when out of buffers
Michael Meffie [Fri, 18 Dec 2009 13:20:24 +0000]
return an error from afs_readdir when out of buffers

Instead of silently failing, return EIO from readdir when the
cache manager is unable to allocate a buffer in afs_newslot,
(afs: all buffers locked).

Change-Id: I3d5a5d73ce78db216400cab45a651fd8a49ea0c3
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agoLINUX: Restore negative dentry caching
Andrew Deason [Mon, 25 Aug 2014 04:01:16 +0000]
LINUX: Restore negative dentry caching

One of the changes in commit 652f3bd9cb7a5d7833a760ba50ef7c2c67214bba
effectively disabled negative caching for dentries, by always
invalidating a negative dentry in afs_linux_dentry_revalidate. This
was because various temporary errors could result in ENOENT errors
being returned to afs_lookup, which created incorrect negative dentry
cache entries.

These incorrect ENOENT errors were rectified in change
Ib01e4309e44b532f843d53c8de2eae613e397bf6 . So, negative dentry cache
entries should work now, so remove the code to unconditionally
invalidate these negative entries.

Change-Id: Ic027147fd1f733beaa0fafbbabfa8c09f5656d34
Reviewed-by: Benjamin Kaduk <>
Reviewed-by: Daria Brashear <>
Tested-by: BuildBot <>

6 years agoMake compile_et output usable out-of-tree
Benjamin Kaduk [Tue, 27 Jan 2015 21:33:25 +0000]
Make compile_et output usable out-of-tree

Prior to this commit, the output C files from compile_et would
emit #includes of <afsconfig.h> and <afs/param.h>.  These files
are not installed, and are only available in an OpenAFS build tree.
The output C files also emit #includes of <afs/error_table.h>, which
is an installed file, and is therefore expected to be available on
a system with OpenAFS installed.  Removing the first two headers will
allow OpenAFS's compile_et binary to be used to compile error tables
which are not part of OpenAFS, on systems where OpenAFS is installed.

The inclusion of afsconfig.h was added in commit
972a4072827fb2ec680354d5adebc2c5cca06939 to ensure that it was included
prior to afs/param.h; however, the inclusion of afs/param.h in
compile_et.c stems from the original IBM import and seems of minimal
value.  The only changes needed to build without param.h are to use
int instead of afs_int32 in a couple places (int is 32 bits on
all platforms currently supported) and to include <sys/types.h>
for size_t.

Change-Id: I1ee969eec92b139d265a7494e13ddfc69c05f238
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>

6 years agoafs: use jenkins hash for dcache, vcache tables
Benjamin Kaduk [Wed, 14 Jan 2015 01:22:59 +0000]
afs: use jenkins hash for dcache, vcache tables

Switch the four dcache and vcache hash tables to use the jenkins
hash from opr.

This requires making DCHash into a full-weight function in order
to properly hash all three inputs; convert all four symbols to
full functions for consistency.  Just pull in <opr/jhash.h> via
afs.h so all consumers (e.g., of VCSIZE) can use it without

This is the first use of src/opr/ in src/afs/ (outside UKERNEL),
but it is permissible because opr/jhash.h is a standalone
header and there are no C files needed for its implementation which
would require anything from the system.

Change-Id: Ic7f31e7dc548ff2cf13ac087a9e4bbb2b874e03a
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>

6 years agorx: Tidy up rxi_CheckCall()'s mtuout handling
Benjamin Kaduk [Sun, 14 Dec 2014 21:13:39 +0000]
rx: Tidy up rxi_CheckCall()'s mtuout handling

We don't actually do anything that matters if lastPacketSizeSeq
is set and lastPacketSize is zero, so zero both when we're cleaning

lastPacketSize and lastPacketSizeSeq are set together in
rxi_SendPacket (and rxi_SendPacketList), when we are sending a packet
larger than the current estimate of the peer's maxPacketSize.

The two fields are checked together during ack processing, but
rxi_CheckCall() only checks lastPacketSize, ignoring lastPacketSizeSeq.

Change-Id: I4e52bed0900b5551859200699f114f5d5a61581c
Reviewed-by: Benjamin Kaduk <>
Tested-by: BuildBot <>
Reviewed-by: Daria Brashear <>

6 years agoDocument KeyFileExt(5)
Benjamin Kaduk [Fri, 27 Feb 2015 23:20:19 +0000]
Document KeyFileExt(5)

Add a manual page for the KeyFileExt file.

Add cross-references from all places which currently reference
KeyFile(5), and update their body text accordingly.

Change-Id: Iab56847fcb59dda0c8a344a626ddb0ff35b98b26
Tested-by: BuildBot <>
Reviewed-by: Benjamin Kaduk <>