Jeffrey Altman [Thu, 7 Mar 2013 22:39:05 +0000 (17:39 -0500)]
Windows: AFSLocateNameEntry Backup Volume Change
When AFSLocateNameEntry() calls AFSBackupEntry() in the name array it is
possible that the DirectoryCB returned belongs to a different VolumeCB.
If so, pCurrentVolume must be updated and reference counts must be
adjusted.
pete scott [Tue, 5 Mar 2013 20:21:41 +0000 (13:21 -0700)]
Windows: RDR SymbolicLink create support
Permit the redirector to handle Microsoft's IO_REPARSE_TAG_MOUNT_POINT
and IO_REPARSE_TAG_SYMLINK requests. The IO_REPARSE_TAG_SYMLINK request
is issued as a result of a CreateSymbolicLink Win32 API.
Creating a symlink in Windows is not equivalent to the way a symlink is
created in AFS or UNIX. Instead of creating a symlink object whose data
string represents the target and mode bits indicate that the stream should
be treated as a link, on Windows it is a two step process.
To create a symlink to a directory, create an empty directory and then
assign the reparse tag data to the directory object. To create a symlink
to anything else, create an empty file and assign the reparse tag data to
the file. Deleting a reparse point simply removes the reparse tag data
and not the underlying directory or file.
The way this will work for AFS is that assigning reparse data to an
existing directory or file will require that the object be deleted from
the directory and a new symlink object be created in its place. This is
why upon successful completion of the upcall to the service the directory
object information has the AFS_OBJECT_FLAGS_DIRECTORY_ENUMERATED flag
cleared.
This patchset permits symlink creation but does not do anything to support
symlink removal.
Jeffrey Altman [Tue, 5 Mar 2013 12:52:37 +0000 (07:52 -0500)]
Windows: Avoid race during cm_FreeServerList
cm_FreeServerList obtains cm_serverLock exclusively and in some
circumstances will call cm_FreeServer(). cm_FreeServer() will
drop the cm_serverLock if the cm_server_t.refCount is zero in order to
avoid a lock order violation when calling cm_GCConnections() since
cm_connLock is higher in the lock hierarchy.
The call to cm_FreeServer is performed after the cm_serverRef_t
to be deleted is identified but before it is removed from the list.
There is the potential for two threads calling cm_FreeServerList()
to race and for more than one thread to attempt to delete the same
cm_serverRef_t twice.
Fix this by:
1. maintain a private copy of the cm_server_t pointer, delete the
cm_serverRef_t and update the list pointers before calling cm_FreeServer().
2. obtain and release a refcnt on the next cm_serverRef_t to ensure
that it is not deleted out from underneath the thread in case the
cm_serverLock is dropped.
Simon Wilkinson [Sat, 2 Mar 2013 12:15:22 +0000 (12:15 +0000)]
aklog: Protect against overflows from cmdline
The cell, realm and path arrays are populated based on the user's
command line, and xlog_path is populated from their passwd map
entry. Protect against all of these overflowing, by making suitable
use of strlcpy and strlcat.
Simon Wilkinson [Sat, 2 Mar 2013 12:04:46 +0000 (12:04 +0000)]
aklog: Fix overflows in auth_to_path
In the auth_to_path routine, don't use strcpy and strcat when
working with the fixed length pathtocheck buffer. Instead, use
strlcpy and strlcat to ensure that all string operations fit within
the buffer limits.
Simon Wilkinson [Sat, 2 Mar 2013 10:27:47 +0000 (10:27 +0000)]
util: Avoid overflow in GetNameByINet
We copy the results of gethostbyaddr into a fixed length buffer
without checking whether they fit. Add a length check, and use
strlcpy to do the copy to make sure we can't overflow.
Simon Wilkinson [Sat, 2 Mar 2013 09:35:01 +0000 (09:35 +0000)]
kauth: Don't overflow stack when building username
knfs constructs the userName by combining the clientName.name
and clientName.instance arrays, along with a dot separator. Make
sure that the userName array is big enough to hold these, and
use strlcpy and strlcat just to make sure.
Simon Wilkinson [Fri, 1 Mar 2013 12:08:46 +0000 (12:08 +0000)]
fs: Fix improper use of readlink
readlink returns a non-NUL terminated buffer. If we are going to
terminate its response, we need to make sure that there's space to
do so. So the length passed to readlink should be one less than the
real length of the buffer.
Simon Wilkinson [Sat, 2 Mar 2013 11:49:13 +0000 (11:49 +0000)]
fstrace: Don't read uninitialised data
The pftix variable points to the next free element in the
printfTypes array, so when we iterate through that array to
read that data back, we should stop when our iterator equals
pftix, not when it is greater than it.
Simon Wilkinson [Sat, 2 Mar 2013 11:44:02 +0000 (11:44 +0000)]
readpwd: Make sure user supplies a passwdfile
If the user supplies enough command line arguments, but doesn't
provide a passwdfile, then we can end up trying to open whatever
garbage is on the stack.
Once we've finished parsing the command line arguments, make sure
that a filename was supplied.
Simon Wilkinson [Sat, 2 Mar 2013 11:44:02 +0000 (11:44 +0000)]
readgroup: Make sure user supplies a groupfile
If the user supplies enough command line arguments, but doesn't
provide a groupfile, then we can end up trying to open whatever
garbage is on the stack.
Once we've finished parsing the command line arguments, make sure
that a filename was supplied.
Simon Wilkinson [Sat, 2 Mar 2013 11:36:31 +0000 (11:36 +0000)]
libadmin: Don't pass garbage to pts_UserCreate
The libadmin pts_UserCreate function uses the value passed to
it in newUserId to control whether the user is being created
with a user supplied ID or not.
Initialise this value in the caller, so we don't end up creating
users with corrupt ids.
Simon Wilkinson [Sat, 2 Mar 2013 11:36:31 +0000 (11:36 +0000)]
libadmin: Don't pass garbage to pts_GroupCreate
The libadmin pts_GroupCreate function uses the value passed to
it in newGroupId to control whether the group is being created
with a user supplied ID or not.
Initialise this value in the caller, so we don't end up creating
groups with corrupt ids.
Simon Wilkinson [Sat, 2 Mar 2013 11:09:08 +0000 (11:09 +0000)]
libadmin: Don't double check for required param
The server, process and stat_type parameters are declared as
required when the command syntax is set up - so they must be
present when the command handler is called. So, don't bother
checking for them.
Simon Wilkinson [Sat, 2 Mar 2013 10:15:10 +0000 (10:15 +0000)]
rmtsys: Don't overflow pathname buffer
When we're constructing a homedirectory path to look for the
.AFSSERVER file in, we copy the HOME environment variable into a
static buffer, with a risk of overflowing that buffer.
Instead of using a static buffer, just allocate one with asprintf.
Simon Wilkinson [Sat, 2 Mar 2013 09:59:20 +0000 (09:59 +0000)]
auth: Don't overflow hostName array
afsconf_cell's hostName structure is a fixed length. Don't overflow
it by writing whatever comes back from gethostbyaddr into it. Use
strlcpy to catch an overflow, and if one occurs, just use
"UNKNOWNHOST", rather than a truncated host name.
Simon Wilkinson [Sat, 2 Mar 2013 09:47:53 +0000 (09:47 +0000)]
libadmin: Don't overflow volume name
The maximum volume name length in the VLDB RPCs is VL_MAXNAMELEN
(65), not 64 as used as a hardcoded value in vsprocs. Switch to
using the defined value, and also use strlcat to check that we
don't overflow this.
Simon Wilkinson [Sat, 2 Mar 2013 09:47:53 +0000 (09:47 +0000)]
volser: Don't overflow volume name
The maximum volume name length in the VLDB RPCs is VL_MAXNAMELEN
(65), not 64 as used as a hardcoded value in vsprocs. Switch to
using the defined value, and also use strlcat to check that we
don't overflow this.
Simon Wilkinson [Sat, 2 Mar 2013 09:26:05 +0000 (09:26 +0000)]
auth: Don't overflow cell string
If the kernel gives us bogus data back from the VIOCGETTOK pioctl,
we might overflow the cell string when copying in to it. Use
strlcpy to avoid this (unlikely) occurrence.
Andrew Deason [Sat, 23 Feb 2013 04:46:12 +0000 (22:46 -0600)]
viced: Improve CallPreamble error messages
These messages are not very useful right now. At least try to say what
host we sent an error to, so we know which host may be experiencing
some troubles as a result.
Andrew Deason [Wed, 19 Dec 2012 23:11:40 +0000 (17:11 -0600)]
ihandle: Remove ih_sync_thread
ih_sync_thread currently syncs files flagged as needing
synchronization in the background every 10 seconds. This practice has
caused severe data corruption on more than one occasion over the past
few years (124359, 131530). It has also been argued repeatedly that it
provides no meaningful additional on-disk consistency, so there is no
reason for it to exist even if it were error-free.
Syncing files in the background provides no guarantee on the
consistency of the file contents, since the files are not synced in
any order with respect to each other, or with respect to what
filesystem operations may be occurring in the application.
Additionally, journalling filesystems common on fileserver backends
will typically ensure some consistency after a certain amount of time
(by default, 5 seconds on ZFS and ext3+), so doing this sync ourselves
is often redundant or even counterproductive.
So, to avoid current and future issues with ih_sync_thread interacting
with other ihandle users, just get rid of it. Files flagged as needing
sync are still synced (not in the background) during IH_REALLYCLOSE.
Modify AFSPopulateNameArrayFromRelatedArray so that the DirectoryCB
parameter can safely be NULL. A NULL DirectoryCB input is required
to copy the entire NameArray.
src/rxosd/Makefile.in: avoid infinite recursion in CC variable
src/rxosd/Makefile sets CC=$(MT_CC).
src/config/Makefile.config sets MT_CC=$(CC).
Since both are recursive-expansion variables, this causes an infinite
loop, and make complains about it:
afscp: Fix check for bare root.cell dirs in dynroot mode
A previous fix (git commit fb1d7491fbe2e90300b23284f213cac2bdcd56ac)
added a check for '!p' in gettoproot(), but p is always a non-NULL pointer
(in part since it's dereferenced just above in the code), so the check
is always false.
Instead, I suspect the original author intended to check for '!*p',
which this patch does.
Simon Wilkinson [Fri, 1 Mar 2013 12:12:07 +0000 (12:12 +0000)]
up: Fix improper use of readlink
readlink returns a non-NUL terminated string. If the string must be
terminated, we need to have space in the buffer for it. So, the
buffer passed to readlink must be 1 less than the real length of
the buffer.
Simon Wilkinson [Fri, 1 Mar 2013 12:01:19 +0000 (12:01 +0000)]
util: Fix overflows in address parsing
The extractAddr function (which turns a dotted quad into an IP
address), has a number of overflows when one or more elements of
the quad are more than 31 characters in length.
The array allocated for each portion is 32 bytes long, but we only
stop writing into the array when the indexing pointer reaches 32,
which doesn't leave us with space for the trailing NULL.
Rework this so we always allow space for the NULL, and use a #define
for the array length to make it more clear whats going on.
Caught by coverity (#985591, #985592, #985593, #985594)
Simon Wilkinson [Fri, 1 Mar 2013 11:47:03 +0000 (11:47 +0000)]
ubik: Don't overflow server's addr array
We're checking to see if we've overflowed the array _after_ we've
looked up an element within it - so on the final iteration, we
always read past the end of the array.
Fix this by swapping the order of the tests in the for statemen
Simon Wilkinson [Fri, 1 Mar 2013 11:35:05 +0000 (11:35 +0000)]
rxgen: Don't overflow PackageIndex
PackageIndex++ returns the pre-index value of PackageIndex, so the
error statement isn't run when PackageIndex == MAX_PACKAGES. This
means we go on to overflow all of the arrays that are MAX_PACKAGES
in size.
Caught by coverity (#985583, #985584, #985585, #985586,
#985587, #985588, #985589)
Simon Wilkinson [Fri, 1 Mar 2013 11:31:31 +0000 (11:31 +0000)]
unlog: Don't overflow cells array
cells has a maximum size of MAXCELLS. Doing cells[MAXCELLS] overflows
that array. Clamp our maximum number of cells at one below this to
avoid the overflow.
Simon Wilkinson [Fri, 1 Mar 2013 11:22:26 +0000 (11:22 +0000)]
kauth: Don't overflow cellinfo hostAddrs array
The hostaddrs array has MAXHOSTSPERCELL (8) available elements.
The ubik connections list has MAXSERVERS (20) elements - when copying
from the ubik list into the cellinfo hostaddrs list, be careful not
to overflow it.
Simon Wilkinson [Fri, 1 Mar 2013 11:09:04 +0000 (11:09 +0000)]
bucoord: Remove theoretical overflow of ubik array
The ubik connections array is NULL terminated, so we have to
ensure that there is enough space for the trailing NULL. As the
array is MAXSERVERS elements long, this means that we can only
store MAXSERVERS-1 connections in it.
This problem will never be encountered by the correct code, as
the number of hosts returned from afsconf_Open is capped at
MAXHOSTSPERCELL (currently 8). MAXSERVERS is currently 20. However,
fix the bug in case we increase MAXHOSTSPERCELL at some point in
the future.
Marc Dionne [Sat, 2 Mar 2013 15:06:47 +0000 (10:06 -0500)]
crypto: make krb5_enomem a static inline function
With the recent update to the imported heimdal code, krb5_enomem
is used in a few places as a simple statement that doesn't make
use of the value. With the current definition, this triggers
compiler warnings because the statement has no effect.
Replace the definition with a static inline function that returns
the expected value.
Jeffrey Altman [Thu, 28 Feb 2013 22:11:57 +0000 (17:11 -0500)]
Windows: afslogon !KA_USERAUTH_AUTHENT_LOGON
Patchset 305133cde60fec7fb1050caf60a4319cdcf88a27 halted the practice
of calling ka_UserAuthenticateGeneral2() with the alternate smbName.
It should have halted the practice of passing flag
KA_USERAUTH_AUTHENT_LOGON which indicates that the smbName parameter
has been provided. Passing KA_USERAUTH_AUTHENT_LOGON without the smbName
field results in a KTC_INVAL error.
This error only impacts sites that still use kaserver or Kerberos v4 for
authentication.
Simon Wilkinson [Thu, 28 Feb 2013 22:07:12 +0000 (22:07 +0000)]
libadmin: Don't free garbage
Make sure that we initialise the nbulkentries structure to 0 before
we start work, so that if the failure handler is called, it doesn't
try to free garbage.
If an array is n elements long, accessing element array[n] is an
overflow. Fix various places where we apply loop bounds incorrectly
using the NUM_CM_STAT_ENTRIES constant.
If an array is n elements long, accessing element array[n] is an
overflow. Fix various places where we apply loop bounds incorrectly
using the NUM_FS_STAT_ENTRIES constant.
Simon Wilkinson [Thu, 28 Feb 2013 17:07:31 +0000 (17:07 +0000)]
afsmonitor: Add missing items to fsOpNames array
The Lookup and Residency fs stats counters were missing from the
fsOpNames array. Add them in - Lookup has been missing since the
IBM release, Residency was missed when the MR-AFS code was merged.
This is still rather fragile, as there's no guarantee that
AFS_STATS_NUM_FS_RPC_OPS matches the number of elements in this
array. However, this is now correct until someone breaks it again...
Simon Wilkinson [Thu, 28 Feb 2013 15:26:15 +0000 (15:26 +0000)]
Unix CM: Fix hash table overflow in dnlc code
In GetMeAnEntry, we can end up overflowing the nameHash array by one
element if the stars are particularly badly aligned.
nameptr is a static across function calls, so nameptr and j are not
equal. If nameptr is increment to NHSIZE in the same loop iteration
as j reaches NHSIZE + 2, the loop will terminate. We'll then
lookup nameHash[NHSIZE], which is 1 element passed the end of the
array.
Add an if statement which loops nameptr outside the loop (in the
same way as the if statement in the loop)
Simon Wilkinson [Thu, 28 Feb 2013 13:45:00 +0000 (13:45 +0000)]
Unix CM: Don't overflow ICL logs array
When checking whether a user supplied index into the ICL logs array
is out of bounds, we need to check whether it is greater or equal
to the maxmimum number of elements. Otherwise we can access one
more than the number of elements in the array.
in order to remove new lines from cmdbuf. Coverity thinks there's
a danger of strlen(cmdbuf) being 0, and thus the strlen being negative.
That shouldn't happen, but if fgets hits EOF midway through a line, we
might get a string that doesn't have a trailing '\n', and end up
removing the wrong character. Tidy this up by checking that the string
isn't 0 length, and that the character we're zapping is a newline.
Jeffrey Altman [Wed, 27 Feb 2013 20:19:41 +0000 (15:19 -0500)]
Windows: afsio reduce library link list
When linking afsio many of the libraries specified in the EXELIB3
list were unnecessary. The only lib in the list that is required
is libcmd.lib. Rename EXELIB3 to LIBCMD and prune the rest.
Jeffrey Altman [Wed, 27 Feb 2013 20:07:57 +0000 (15:07 -0500)]
Windows: Remove RXAFS* from afsrpc.dll
afsrpc.dll is supposed to be the multi-threaded dynamic loadable
version of the RX package. It has also included the RXAFS and
RXAFSCB client functions from src/fsint. Including the RXAFS* is
nice in theory but in practice the afsd_service.exe must link to
afsint.lib anyway in order to obtain access to RXAFSCB_ExecuteRequest()
which is part of the server portion. Linking to both results in
multiple instances of the same symbols in which case afsint.lib
must be linked to first and the dynamic version of the functions
are ignored.
The only other module that used RXAFS* functions from afsrpc.dll
is the adminutil library from the libadmin package. adminutil
doesn't implement the server functions but it can just as easily
link against afsint.lib.
Removing RXAFS* from afsrpc.dll results in a library that only
contains RX, RXSTATS, core xdr and rxkad.
This patchset also fixes some minor bugs:
1. $(UTILOBJS) were not being linked to the library due to
a typo.
2. Always use multi-thread safe versions of afsutil.lib
even though a dedicated afspthread.dll existed and all pthreaded
modules must link to afspthread.dll and not all link to
afsauthent.dll. Move these functions and the global mutex variable
to afspthread.dll to ensure that only one instance of the variable
is present in any binary.
Also remove from src/libafsauthent/NTMakefile the local building
of src/util object files and link to mtafsutil.lib.
Simon Wilkinson [Wed, 27 Feb 2013 10:34:59 +0000 (10:34 +0000)]
ubik: Avoid unlinking garbage
If SDISK_SendFail fails before the pathname to the temporary file
has been constructed, then the failure handler will unlink stack
garbage, with variable results.
Initialise the string, and check to see if it has contents before
calling unlink
Simon Wilkinson [Wed, 27 Feb 2013 10:28:05 +0000 (10:28 +0000)]
Unix CM: Don't free cell, then release lock on it
If afs_NewCell fails, then we can end up releasing a lock on a
section of memory that we have already freed. As this only happens
if the memory we're operating on is newly allocated and not yet
visible to anyone else, it is safe to release the lock before
starting to tidy things up.
Simon Wilkinson [Wed, 27 Feb 2013 10:11:21 +0000 (10:11 +0000)]
libafscp: Can't unlock something we've freed
When we call _StatCleanup on a stored statent structure, it
deletes the mutex, and frees the structure itself. This means it
can't be called with a locked structure as the mutex deletion
will fail, and then we'll try to reference freed memory when we
later unlock that mutex.
Fix this by unlocking the mutex before calling _StatCleanup. This
is safe because the only reference to the structure visible to other
threads must have been deleted by the time we reach this point.
Simon Wilkinson [Wed, 27 Feb 2013 09:23:07 +0000 (09:23 +0000)]
pt_util: Protect against corrupt input files
If we have an input file which contains a group membership line
(with a leading space) before any group definitions occur, pt_util
would use stack garbage as the group to create these members in.
Avoid this by requiring the presence of a group line before any
membership lines.
Simon Wilkinson [Tue, 26 Feb 2013 22:27:25 +0000 (22:27 +0000)]
auth: Fix buffer overflow in afsconf_Open
If we fallback to the .AFSCONF file in the user's homedirectory,
the results of getenv("HOME") are copied into a fixed length string,
without checking for overflows.
Instead of risking this, just use asprintf to dynamically construct
a string, and free it when we are done.
Simon Wilkinson [Tue, 26 Feb 2013 21:30:20 +0000 (21:30 +0000)]
bos_util: Fix buffer overflow
Get rid of a buffer overflow in the bos_util utility, by just
printing the key from the 'tbuffer' string, rather than copying
it into 'x' which is too small for it.
Simon Wilkinson [Tue, 26 Feb 2013 21:28:52 +0000 (21:28 +0000)]
volser: Fix bad readlink usage
readlink fills the buffer passed to it with a non-terminated string.
It can legitimately fill the whole of this buffer. So, if we require
a string to be NUL terminated, we must give readlink one less than
the string length so that the termination character can be safely
appended.
ptserver/testpt.c: remove dead code in ListUsedIds
A part of the current ListUsedIds code is:
range = abs(startId - maxId);
if (range < 0)
range = -range;
The only way abs() could return a negative value would be if its
argument was INT_MIN (-2147483648) to begin with, because -INT_MIN
cannot be represented in two's complement. However, calling
abs(INT_MIN) is undefined behavior in C [see C99 7.20.6.1], and for
that matter, so would be computing -range (-INT_MIN) in that case,
so we could still be left with a negative range value.
Luckily, (startId - maxId) can never be INT_MIN. If startId < 0,
then maxId <= startId, so in the worst case, when maxId = INT_MIN
and startId = -1, (startId-maxId)=INT_MAX. If startId >= 0, then
maxId >= startId, so in the worst case, when maxId = INT_MAX and
startId = 0, (startId-maxId)=-INT_MAX=INT_MIN+1.
Simon Wilkinson [Tue, 26 Feb 2013 12:30:00 +0000 (12:30 +0000)]
ptserver: Zero ubik header before writing to it
When using pt_util's ubik shim, if we're creating a new ubik label
make sure to zero the whole structure before writing it out to disk.
Otherwise we get a shorts worth of stack garbage in the resulting
file.
Mark Vitale [Tue, 26 Feb 2013 14:31:43 +0000 (09:31 -0500)]
vos: Unrecognized or ambiguous switch '-c'
The documented abbreviation '-c' for '-cell' no longer works since
the -config option was added. Add a parm alias '-c' to restore
compatibility with previous syntax.
Simon Wilkinson [Tue, 26 Feb 2013 12:26:36 +0000 (12:26 +0000)]
ptserver: Rename ubik.c as ptubik.c
Coverity gets confused between ubik/ubik.c and ptserver/ubik.c,
and produces a load of false positives. Rename the ptserver ubik
shim (which is only used by pt_util) in order to reduce this
confusion.
Simon Wilkinson [Tue, 26 Feb 2013 12:21:59 +0000 (12:21 +0000)]
ubik: Zero header before writing to disk
The ubik disk header contains a padding field, which is never
zeroed before being written out to disk. This means that there is
a shorts worth of stack garbage in every ubik label!
Just zero the whole structure with memset before we populate it.
Simon Wilkinson [Tue, 26 Feb 2013 12:17:29 +0000 (12:17 +0000)]
viced: Don't write out garbage when extending file
When we extend a file, we write a single byte of data to it - tlen
is used as a source for this single byte. However, in the current
code, tlen is used uninitialised. Set it to 0, so we don't write a
byte of stack garbage.
Simon Wilkinson [Mon, 25 Feb 2013 23:13:47 +0000 (23:13 +0000)]
Add krb5_enomem for config parser & kernel crypto
The imported code form Heimdal that forms our config file parser
and kernel rfc3961 library now makes use of krb5_enomem. Provide
an implementation so we build again ...
Kumar Thangavelu (1):
unning "kinit --fast-armor-cache=xxx" against a Win2K3 domain resulted in a crash with the attached backtrace. FAST is not supported with RC4 keys which are used in Win2K3. The code already handles this but the error code is not propagated up the stack.
Love Hornquist Astrand (1):
add socket_set_nonblocking
Roland C. Dowdeswell (3):
Optimise _krb5_n_fold() a bit.
Use krb5_enomem() more consistently in lib/krb5.
Further improvements to lib/krb5/n-fold.c:
Simon Wilkinson [Wed, 20 Feb 2013 14:54:55 +0000 (14:54 +0000)]
volser: Make it clear what's copied from partList
XVolListPartitions uses partList to maintain a list of partition
IDs - it does not use the partFlags field. Make it clear when we
populate the partEntries field that we are only copying the
partition list.
projects / packages / o / openafs.git / log