Anders Kaseorg [Sun, 4 May 2014 09:30:25 +0000 (05:30 -0400)]
Fix buffer length validation in ktc_GetToken and knfs
The signed int tktLen is checked against a maximum size, then passed
as the unsigned size_t argument to memcpy. So we need to make sure it
isn’t negative.
This doesn’t appear to be exploitable: tktLen comes from the kernel,
which should have previously validated the length within the SETTOK
pioctl.
This bug was found with STACK <http://css.csail.mit.edu/stack/>.
Signed-off-by: Anders Kaseorg <andersk@mit.edu>
Reviewed-on: http://gerrit.openafs.org/11109 Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 9c10c202f1f2e516dde8b70c3a3b69a73d163070)
Change-Id: Id8dacdc00fd686d4f2ff234ffd6c8f5346d9e7b0
Reviewed-on: http://gerrit.openafs.org/11112 Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Anders Kaseorg <andersk@mit.edu> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
- That commit makes the RPC fail in situations where it did not
before. But even if we cannot calculate the checksum, we can still
return other information about the key, so this is undesirable.
- It masks the previous 'code' value, returned from stat(). The
return code of stat() is now effectively ignored, except for the
purposes of setting st_mtime, whereas previously a failure caused
the RPC to fail. This is a behavior change.
So, effectively revert c04de52da4e89e15b211b4a19a3d9bc4d612b209.
Explicitly cast the return value of ka_KeyCheckSum to void, to make it
clear that we are intentionally ignoring the return value, so
hopefully this will not be flagged as a warning by code analysis tools
such as coverity.
Reviewed-on: http://gerrit.openafs.org/11194 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit 1673764ea091a3f02a64a3d853c3e14f758cdda7)
Change-Id: Ibb05f0afc68db5fa66e1dc55f7dae190d8057232
Reviewed-on: http://gerrit.openafs.org/11206 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Stephan Wiesand [Mon, 2 Jun 2014 14:15:15 +0000 (16:15 +0200)]
fstrace: Don't read uninitialised data on other platforms either
Commit 908105fe8d51551e45692de4e145022138a0356c fixed an off-by-one
error potentially causing a buffer overread in CheckTypes, but only
in the IRIX/AIX version of the function. Apply the same fix to the
code for the other platforms.
Spotted by Andrew Deason.
Reviewed-on: http://gerrit.openafs.org/11185 Reviewed-by: D Brashear <shadow@your-file-system.com> Tested-by: BuildBot <buildbot@rampaginggeek.com>
(cherry picked from commit 3ab546395536a3c93613dfd53363d380e2c05081)
Change-Id: Iea700be24803d8156f07d1cf0617ba7aa2aa03d5
Reviewed-on: http://gerrit.openafs.org/11196 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Marc Dionne [Wed, 28 May 2014 13:53:58 +0000 (09:53 -0400)]
vol: Fix gcc 4.9 warnings
gcc 4.9 complains here because the trailing 0 in these macros
has no effect, the value having already been set to NULL.
Just remove the offending 0s, nothing uses the return value
anyway, even if there were platforms where 0 != NULL.
Reviewed-on: http://gerrit.openafs.org/11176 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit c0683441a0121433d772bfb36e1e9a1c020a5dcb)
Change-Id: Ia63f56d3254b87e7d56752ec11729d82e0293792
Reviewed-on: http://gerrit.openafs.org/11204 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
This does not change the current (normal) behaviour, but allows
logrotation via "copy and truncate" as offered by logrotate.
Otherwise the processes will remember the offset of the last write
and a truncated file is filled with '\0' until the current offset.
The mrafsStyleLogs are untouched, since they can be rotated by a
kill -HUP and are deprecated anyway.
Reviewed-on: http://gerrit.openafs.org/11092 Reviewed-by: Nathaniel Filardo <nwfilardo@gmail.com> Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: D Brashear <shadow@your-file-system.com> Tested-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit b71a041364d28d6a56905a770cd20d1497ee26ec)
Change-Id: Idc148f4d6b9302d25cbf32763d08ee24eaf7378b
Reviewed-on: http://gerrit.openafs.org/11193 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
"Briefly, 'host' structures are allocated without clearing all of the
contents to '0'. Only part of the structure is cleared, according to the
HOST_TO_ZERO macro. Unfortunately I put the new tmay_ fields right below
the 'index' field for some reason, so this means they aren't zeroed and
can contain garbage. This means we can easily segfault in the fileserver
when we try to access the pointers in there.
"We access uninitialized memory for every 'host' that is allocated. So
the chance of us corrupting memory is the chance that a particular
pointer-sized area of memory from 'malloc' is not already NULL.
"That seems pretty likely, but it's not so frequent as to have the
fileserver effectively "constantly" crashing at the site that noticed.
So it has not been a fire drill, but it has been noticeable (we heard
about it I think yesterday, and got details today when it happened
again). The noticing incident was a segfault, but an abort or sigbus are
probably also likely.
"Of course, the chances of noticing go way up with more clients. I expect
the chances dramatically increase if you have more than 512 client hosts
hit the box, since the first block of 512 are allocated before we really
do anything. For the next 512, it seems much more likely that 'malloc'
will give us back non-zeroed data. But this is just theory.
"With the incident I know about, the crash happened semi-quickly after
the server started (a few minutes). But it seems likely to occur after
the server has been up for a long time, if/when you cross the next line
of 512 hosts.
"I am also concerned that this can easily be corrupting memory without
being noticed via a crash (or it takes a while to crash), since we are
potentially free'ing invalid pointers, or stomping over someone else's
memory, etc etc."
The amount of space allocated for use by the pioctl call to
obtain the ACL for the source directory in the "up" command
is not large enough and the call fails when access lists get
sufficiently large.
This change increases the size of the space provided to
pioctl to the maximum possible. This allows for much larger
access lists and is consistent with a similar call in the
"fs listacl" command).
Stephan Wiesand [Wed, 7 May 2014 13:20:51 +0000 (15:20 +0200)]
redhat: Use the right path to depmod
As of Fedora 17 and RHEL 7, depmod has moved from /sbin to /usr/sbin.
The full path to depmod is used in package scripts and as a dependency.
This hasn't caused problems in most cases because on an installed
system a link /sbin -> /usr/sbin is present and during ordinary package
installations yum/rpm correctly then figure out that /sbin/depmod is
actually provided. But in other situations, the dependency check is not
that clever and (incorrectly) fails.
Add a macro to the spec defining the full path to depmod, use the macro
rather than plain /sbin/depmod throughout the spec, and also pass it to
kmodtool when required to generate the kmod package scripts and
requirements.
FIXES 131860
Reviewed-on: http://gerrit.openafs.org/11128 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit c20c01185ed748b2bc823369a8f28cf004b7d1c9)
Change-Id: Ib9e0bfe586de668ddba6fb82ff1ea2a081277150
Reviewed-on: http://gerrit.openafs.org/11171 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Ken Dreyer <ktdreyer@ktdreyer.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
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.
Andrew Deason [Tue, 15 Apr 2014 17:30:19 +0000 (12:30 -0500)]
doc: Clarify some BosConfig.new text
It is not always clear to users whether BosConfig.new is noticed
during an automatic restart, or if it requires stopping and starting
the bosserver. Slightly reword the relevant text and add a small note
that a "general restart" does cause BosConfig.new to be noticed, so
this is explicitly clear.
Reviewed-on: http://gerrit.openafs.org/11076 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 3946b50a7ecdfd34681ab471863929b2f82aff4b)
Change-Id: Ia630aec6ef5259fc3c3fd531fdf8fda8a4152c54
Reviewed-on: http://gerrit.openafs.org/11216 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Ken Dreyer <ktdreyer@ktdreyer.com> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Andrew Deason [Wed, 27 Mar 2013 23:12:41 +0000 (18:12 -0500)]
afs: Raise fake free space reporting
We report 'fake' values for free space, free file nodes, etc for the
'AFS' filesystem, since these values are not meaningful for AFS
itself. Currently we report about 9G of free space for most platforms,
and a few different values for a few others. Raise all of these to
2^32-1, so that trying to copy over 9G of data into AFS does not fail
for those applications that check the destination free space with
statfs(2). Note that one such application is KDE 4.8.x.
Consolidate all places that do this, and put the 'fake' value in one
place, AFS_VFS_FAKEFREE, along with the relevant comments.
Related issues reported by Lars Schimmer, Richard Brittain, and
others.
Reviewed-on: http://gerrit.openafs.org/9688 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Markus Koeberl <markus.koeberl@tugraz.at> Tested-by: Markus Koeberl <markus.koeberl@tugraz.at> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit e385571ba37fa6d43fecca17e9e5d60a22a414cd)
Change-Id: Id9b5fa92f48bc83b84bb4f50bc0ae25b028694b0
Reviewed-on: http://gerrit.openafs.org/10984 Tested-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: D Brashear <shadow@your-file-system.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Stephan Wiesand [Thu, 10 Apr 2014 15:59:24 +0000 (17:59 +0200)]
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.
Commit 54eb2485b59550ba42569ed3a8d76211a3a35019 removed the
implementation of bnode_Deactivate(), which had been #ifdef'd out
for a long time, but left the prototype in place. Remove the
obsolete declaration in bosprototypes.h as well.
Reviewed-on: http://gerrit.openafs.org/10868 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Perry Ruiter <pruiter@sinenomine.net> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit 01a7c64e472c241798306e7f8137de28efdef37e)
Change-Id: Ic1ca95ef455705a14cf68f0801d73789cc7bd4b5
Reviewed-on: http://gerrit.openafs.org/11192 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Stephan Wiesand [Fri, 14 Mar 2014 13:44:54 +0000 (14:44 +0100)]
bos: Change the remaining use of ktc_to_charptr to char cast
This is a 1.6 only change to complement the preceding commit.
On the master branch, this modification is part of commit a9b8fd81bf14c04d33113e9e4df4e1ca7573e49d which otherwise comprises
changes not foreseen to be applied to 1.6.
Change-Id: I44678c2fb15f136d70f2d38a3996ad3d7d0455d5
Reviewed-on: http://gerrit.openafs.org/10891 Reviewed-by: D Brashear <shadow@your-file-system.com> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Ken Dreyer [Wed, 6 Mar 2013 20:53:29 +0000 (13:53 -0700)]
doc: recommend cleanup steps in "vos convertROtoRW" man page
vos convertROtoRW leaves the older RW copy on the original fileserver,
although it is no longer in the VLDB. Provide the user with some hints
regarding clean up.
Reviewed-on: http://gerrit.openafs.org/9408 Reviewed-by: Ken Dreyer <ktdreyer@ktdreyer.com> Tested-by: Ken Dreyer <ktdreyer@ktdreyer.com>
(cherry picked from commit 279345c231d0a2d9f6e8c2f76a5347bafd40e70b)
Change-Id: Id273ab86bfe68f89ef629f0c4d839882852e09ab
Reviewed-on: http://gerrit.openafs.org/11126 Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Reviewed-by: Ken Dreyer <ktdreyer@ktdreyer.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de> Tested-by: Stephan Wiesand <stephan.wiesand@desy.de>
Simon Wilkinson [Mon, 4 Mar 2013 16:34:20 +0000 (16:34 +0000)]
butc: Init volheader before using it for hton
When converting a volume header from host to network order, make
sure that any unused fields in the structure are zero'd, so we don't
end up filling them with stack garbage in the network version of
the structure.
Simon Wilkinson [Sat, 2 Mar 2013 13:01:14 +0000 (13:01 +0000)]
auth: Don't overflow buffer in CompFindUser
The fullname buffer in CompFindUser is theoretically big enough
to take the data usually supplied to it. However, play it safe by
using strlcat and strlcpy to catch buffer overflows.
Simon Wilkinson [Sat, 2 Mar 2013 12:38:49 +0000 (12:38 +0000)]
audit: Fix overflow in file backend
If the filename passed to open_file was larger than MAXPATHLEN-5,
then we'd overflow the oldName buffer when creating the backup
filename. Fix the overflow by using a malloc'd buffer instead.
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 [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 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.
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: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.
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...
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.
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.
Michael Meffie [Tue, 30 Apr 2013 15:30:15 +0000 (11:30 -0400)]
pt_util: fix group line check for input files
Fix the check for requiring group lines before any membership lines. Do
not clear flag indicating the presence of a group after reading each
line. (This error was caught by the pt_util-t unit test.)
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.
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.
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.
Reviewed-on: http://gerrit.openafs.org/9273 Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit daed548f70a2187c679232e4f79d703389419c4e)
Change-Id: I834fab3b65859cb73ece3fc52d9272d0b7d452ed
Reviewed-on: http://gerrit.openafs.org/11012 Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Simon Wilkinson [Thu, 21 Feb 2013 20:36:19 +0000 (20:36 +0000)]
libadmin: Fix a lot of dead assignments in vsprocs
Tidy up a lot of places where we initialise a variable, then
immediately assign a proper value to it, or store a return value
that we don't actually care about.
Benjamin Kaduk [Fri, 24 Jan 2014 17:00:20 +0000 (12:00 -0500)]
FBSD: catch up to 1997 and include if_var.h with if.h
The commit message for upstream's r257244 change includes:
- Make the prophecy from 1997 happen and remove if_var.h inclusion
from if.h.
Despite the clear public posting, we were caught unawares. We made
it down to the cellar despite the missing stairs, but "Beware of
the Leopard" caused us to turn back, apparently.
Since if.h is included in many places and if_var.h is not present
on all OSes, pull the if.h inclusion into the common kernel headers
for afs/ and rx/ , and add in if_var.h (as well as the sys/socket.h
prerequisite).
Michael Meffie [Sat, 15 Feb 2014 17:03:43 +0000 (12:03 -0500)]
viced: fix get-statistics64 buffer overflow
Range check the statsVersion argument of the GetStatisitics64 RPC to
avoid a buffer overflow in the fileserver, or a huge memory allocation,
by a rogue client.
Andrew Deason [Fri, 21 Feb 2014 21:30:49 +0000 (15:30 -0600)]
rx: Avoid rxi_Delay on RXS_CheckResponse failure
Currently we rxi_Delay whenever RXS_CheckResponse fails for any
reason. This can result in disastrous performance degradations if a
client keeps sending "bad" responses, since rxi_Delay'ing here will
delay the Rx listener thread. This means we cannot receive any packets
for about a second, which can easily cause us to drop a lot of
incoming packets.
Instead, send the abort after 1 second by scheduling an event. This
will retain existing behavior from the point of view of the client
(it will get the abort after 1 second), but avoids hanging the Rx
listener thread.
Andrew Deason [Fri, 21 Feb 2014 21:26:35 +0000 (15:26 -0600)]
rx: Split out rxi_SendConnectionAbortLater
Take the functionality in rxi_SendConnectionAbort that schedules a
delayed abort, and split it out into a new function,
rxi_SendConnectionAbortLater. This allows callers an easy interface to
send such a delayed abort with their own delay.
This commit should incur no change in behavior; it is just code
reorganization.
Client host too busy while handling request from host %s:%d viceid %d fid %lu.%lu.%lu, failing request
Cannot get CPS for client while handling request [...], failing request
Cannot reconnect to ptserver while handling request [...], failing request
While the new messages are more informative, and (in my opinion)
better describe what is happening in those situations, they do look
very different from the old messages. This can break scripts that try
to parse these logs, but in general it is also not clear to
administrators that these messages still refer to the same events.
So instead, put these messages back the way they were. Still include
the extra information, of course, but revert the language to look more
like the old messages. Now we log:
CallPreamble: Couldn't get client while handling request from host %s:%d viceid %d fid %lu.%lu.%lu, failing request
CallPreamble: Couldn't get CPS while handling request [...], failing request
CallPreamble: couldn't reconnect to ptserver while handling request [...], failing request
Thanks to Ben Kaduk for bringing this up.
Reviewed-on: http://gerrit.openafs.org/10857 Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: D Brashear <shadow@your-file-system.com> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit 0e9bb718ce231ffd73fe11810d8dc1d3902e4b2d)
Change-Id: I35c8369a7efba0c08c000a24e14385209082cfe0
Reviewed-on: http://gerrit.openafs.org/10953 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Chas Williams - CONTRACTOR <chas@cmf.nrl.navy.mil> Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Andrew Deason [Fri, 18 Oct 2013 00:22:48 +0000 (20:22 -0400)]
viced: Improve client error log messages
Commit 6c41b1f740e16b5b9adfe9026630595be6f0699e improved a few log
messages to include the client ip and port of the request triggering
that log message. Include the viceid and fid (if applicable), too, so
an administrator may more easily identify the cause.
This creates the function LogClientError, so we can use a common
function for logging very similar information. This also modifies
h_FindClient_r to give the viceid to the caller, even in the case of
error. In addition, this modifies CallPreamble to accept a fid and
modifies all callers to accomodate.
Stephan Wiesand [Wed, 12 Mar 2014 09:47:17 +0000 (10:47 +0100)]
doc: bos setrestricted -mode 0 does make sense
Commit 070230ab76e1df338db3f2a7971111ca976a0c1a added documentation of
the mode parameter to bos setrestricted, claiming that the value 0 is
useless, and commit eee0bf5871944d919951cc8b7b4908ee909c3b62 added
documentation of the restrictmode entry in BosConfig, claiming that it
can only be set back to 0 with an editor. Both claims are wrong, since
bos setrestricted -mode 0 will do exactly that (if it succeeds, which
it only can if the server is running in unrestricted mode, which can
be achieved by sending it the FPE signal). Fix the man pages
accordingly.
Reviewed-on: http://gerrit.openafs.org/10885 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Marc Dionne <marc.c.dionne@gmail.com> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit da549eea21941681c075796512a27a830259c835)
Change-Id: Iea8f252829ba6192176da0ceba464cbc41aad53c
Reviewed-on: http://gerrit.openafs.org/10955 Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: Stephan Wiesand <stephan.wiesand@desy.de> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Stephan Wiesand [Fri, 7 Mar 2014 10:03:36 +0000 (11:03 +0100)]
doc: improve man pages related to bos restricted mode
Mention the restrictmode entry and the commands for setting and
querying it in the BosConfig man page, and add/fix cross references
between the BosConfig, bos, bos_getrestricted and bos_setrestricted
ones.
Reviewed-on: http://gerrit.openafs.org/10874 Reviewed-by: Ken Dreyer <ktdreyer@ktdreyer.com> Reviewed-by: Benjamin Kaduk <kaduk@mit.edu> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com> Tested-by: Jeffrey Altman <jaltman@your-file-system.com>
(cherry picked from commit eee0bf5871944d919951cc8b7b4908ee909c3b62)
Change-Id: I25d2f23d75a9074ae478f86296bb13c1b2dda95b
Reviewed-on: http://gerrit.openafs.org/10883 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: Michael Meffie <mmeffie@sinenomine.net> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
Michael Meffie [Fri, 7 Feb 2014 14:55:31 +0000 (06:55 -0800)]
fs: display cell not available on ESRCH
The cache manager pioctls abuse ESRCH to represent errors due to
unavailable cell information. Give a more sensible error message to
the user when a pioctl returns an ESRCH error, instead of "no such
process", which is the conventional meaning of ESRCH.
The new error message is consistent with the Windows implementation
of fs.
For example, on a host with a misconfigured ThisCell and/or CellServDB.
$ fs wscell
fs: No such process
becomes:
$ fs wscell
fs: Cell name not recognized.
Reviewed-on: http://gerrit.openafs.org/10824 Tested-by: BuildBot <buildbot@rampaginggeek.com> Reviewed-by: D Brashear <shadow@your-file-system.com>
(cherry picked from commit 8beba712d95b637225f215534a759961ff4d80a9)
Change-Id: I0cf6f6e0939a1075332049361153bc8a0b0958ce
Reviewed-on: http://gerrit.openafs.org/10949 Reviewed-by: D Brashear <shadow@your-file-system.com> Reviewed-by: Andrew Deason <adeason@sinenomine.net> Tested-by: Andrew Deason <adeason@sinenomine.net> Reviewed-by: Jeffrey Altman <jaltman@your-file-system.com> Reviewed-by: Stephan Wiesand <stephan.wiesand@desy.de>
projects / packages / o / openafs.git / log